Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Enums in Choice fields #5740

Closed
wants to merge 3 commits into from

Conversation

javiereguiluz
Copy link
Collaborator

No description provided.

abozhinov and others added 2 commits May 1, 2023 15:24
Update FieldFactory.php

Update ChoiceConfigurator.php

Update ChoiceConfigurator.php

Update ChoiceConfigurator.php

Update ChoiceConfigurator.php

Update ChoiceConfigurator.php

Update Actions.php

Update Actions.php

Update ChoiceConfigurator.php
@javiereguiluz
Copy link
Collaborator Author

I've fixed a couple of tests, but there's a test that still fails.

To be honest, I don't even understand what I'm doing. I don't work much with Enums, so this is still confusing to me. Hopefully @abozhinov or @ksn135 can take a look here and suggest some fixes. Thanks.

@ksn135
Copy link
Contributor

ksn135 commented May 29, 2023

Hi @javiereguiluz !
Sorry for the delay in replying.
I applied the patch to the current master 4.x branch and everything works fine in my projects as follows:
In lists and in view mode, the translated value is shown. In edit or create mode, a selection item appears with the same correctly translated caption. It seems to me it is possible to merge with the main branch.
Screenshot 2023-05-29 at 19 06 16
Screenshot 2023-05-29 at 19 05 57
Screenshot 2023-05-29 at 19 11 58
New code WITH this patch:

        yield Fields\ChoiceField::new('priority', 'admin_label.doc.field.priority')->setRequired(true)
            ->setFormTypeOption('choice_label', fn ($choice, $key, $value) => 'admin_label.enum.docPriority.'. $value);

Current version of VERY ugly BUT working code:

        yield Fields\ChoiceField::new('priority', 'admin_label.doc.field.priority')->setRequired(true)
            ->setFormTypeOption('placeholder', false)
            ->setFormType(MyEnumType::class) // use transformer to convert enum to string and back
            ->setFormTypeOption('class', DocPriority::class) // needed for MyEnumType
            ->setChoices( // to correctly show translated labels
                        \in_array($pageName, [Crud::PAGE_INDEX, Crud::PAGE_DETAIL], true)
                                    ? DocPriority::choices() : DocPriority::cases()); 

P.S.: I don't really understand why that test crashes either, unfortunately. I'll try to figure it out a little later.
P.P.S.: Please review my old PR #5495 and give me feedback about it.

@ksn135
Copy link
Contributor

ksn135 commented May 29, 2023

Hi @javiereguiluz ! There is no FAILING test. Just run it locally.
Screenshot 2023-05-29 at 19 41 55

@abozhinov
Copy link
Contributor

Hi @javiereguiluz and @ksn135,
I wanted to discuss the goals of this pull request (PR) with you:

  1. The first goal is to enable the hiding of fields from the Configurator.
  2. The second goal is to transform ChoiceField to EnumField when an Enum is being used.
    Let me know if you have any questions or suggestions.

@abozhinov
Copy link
Contributor

Without changes in the ChoiceConfigurator has following error:
Symfony\Component\Form\Extension\Core\Type\EnumType::Symfony\Component\Form\Extension\Core\Type\{closure}(): Argument #1 ($choice) must be of type ?BackedEnum, string given, called in /var/www/html/dreamshop/vendor/symfony/form/ChoiceList/ArrayChoiceList.php on line 181

yield ChoiceField::new('type', new TranslatableMessage('product.attribute.type.label')) ->setChoices($isInfoPage ? ProductAttributeType::getAsArray() : ProductAttributeType::cases()) ->setFormType(EnumType::class) ->setFormTypeOption('class', ProductAttributeType::class) ->setFormTypeOption('choice_label', function (ProductAttributeType $enum) { return 'product.attribute.type.'.$enum->value; }) ->setCustomOption(ChoiceField::OPTION_USE_TRANSLATABLE_CHOICES, $isInfoPage) ->setDisabled() ->setVirtual(true);

@abozhinov
Copy link
Contributor

abozhinov commented Jun 26, 2023

yield ChoiceField::new('type', new TranslatableMessage('package.type.label')) ->setFormTypeOption('choice_label', function (PackageType $enum) { return 'package.type.'.$enum->value; }) ->setCustomOption(ChoiceField::OPTION_USE_TRANSLATABLE_CHOICES, $isInfoPage);

Screenshot 2023-06-26 at 18 40 27

foreach ((array) $fieldValue as $selectedValue) {
if (null !== $selectedLabel = $flippedChoices[$selectedValue] ?? null) {
if ($selectedValue instanceof TranslatableInterface) {
$choiceMessage = $selectedValue;
} else {
if (\is_object($selectedLabel) && enum_exists($selectedLabel::class)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should also add && !$field->getFormTypeOption('choice_label')

@abozhinov
Copy link
Contributor

Will anyone merge this changes to main branch?

@abozhinov
Copy link
Contributor

You can close this PR because I create new #5867 with all tests Passed.

@javiereguiluz
Copy link
Collaborator Author

Closing in favor of #5944. Hopefully it fixes problems for good 🙏 Thanks!

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.

3 participants