Skip to content

Conversation

ericabouaf
Copy link
Contributor

Short description of what this feature will allow to do:

The FieldTraitAwareInterface was added to the repository to serve as the interface for fields that use FieldTrait. However, none of the fields that actually use FieldTrait implement this interface—they only implement FieldInterface. This makes FieldTraitAwareInterface effectively unused and defeats its intended purpose.

Example of how to use this feature

This change would enable us to create field factories that return FieldTraitAwareInterface instances, which could then be manipulated using the methods provided by FieldTrait. We have found this approach particularly valuable for defining fields outside of CRUD controllers, allowing them to be reused across different components.

Closes #7010

*/
public function setDefaultColumns(int|string $cols): self;

public function setIcon(?string $iconCssClass, string $invokingMethod = 'FormField::setIcon()'): self;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is only implemented by FormField, and is not part of FieldTrait

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing a method from an interface is a BC break and should only be done in major version

What you can do instead is providing a blank deprecated method setIcon in the trait

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VincentLanglet : alright, it's restored. However I do not really agree with your point :

Firstly, FieldTraitAwareInterface is not used anywhere in the codebase, and I doubt anybody uses it yet, since none of the existing EasyAdmin fields implements it. (That's the goal of this PR).

Secondly, the signature in this interface does not match the only implementation of it in FormField, so I had to change the signature in FormField.

Thirdly, the goal of this interface is to have a common contract implemented by ALL the fields implementing FieldTrait. I don't see the point of adding it for a single field (FormField.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should Fields using FieldTrait implement FieldTraitAwareInterface ?
2 participants