Skip to content
New issue

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

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

Already on GitHub? # to your account

EZP-29990: As an Administrator I want to translate ezselection Field Definition options #277

Merged

Conversation

ViniTou
Copy link
Contributor

@ViniTou ViniTou commented Jan 31, 2019

Question Answer
Tickets https://jira.ez.no/browse/EZP-29990
Bug fix? no
New feature? yes
BC breaks? no
Tests pass? yes
Doc needed? yes
License GPL-2.0

Kernel:
ezsystems/ezpublish-kernel#2532
Admin-UI:
ezsystems/ezplatform-admin-ui#824

Checklist:

  • Coding standards ($ composer fix-cs)
  • Ready for Code Review

@ezrobot

This comment has been minimized.

Copy link
Contributor

@webhdx webhdx left a comment

Choose a reason for hiding this comment

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

There is potential BC break here. SelectionFormMapper will now add options or multilingualOptions form type which means it will break templates if there is form.options call.

if ($baseLanguage && $language) {
$names[$language->languageCode] = $fieldDef->getName($baseLanguage->languageCode);
$descriptions[$language->languageCode] = $fieldDef->getDescription($baseLanguage->languageCode);

if (isset($fieldDef->fieldSettings['multilingualOptions'][$baseLanguage->languageCode])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it's field specific it should be placed here. You could create an extension point here but I'm not sure if this should happen inside ContentTypeDraftMapper.

@ezrobot

This comment has been minimized.

@ViniTou ViniTou force-pushed the EZP-29990-translate-ezselection-options branch from 69dd3d1 to 50d6f64 Compare February 20, 2019 14:01
@ViniTou ViniTou requested review from alongosz and webhdx February 22, 2019 10:22
Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

From my side this looks good, but could use a few readability improvements, if you have time:

@ViniTou ViniTou force-pushed the EZP-29990-translate-ezselection-options branch from 058696f to 8cedc46 Compare February 25, 2019 09:20
@lserwatka lserwatka merged commit 05f857e into ezsystems:master Feb 25, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Development

Successfully merging this pull request may close these issues.

6 participants