Problem/Motivation

Experience builder fields should be translatable

We need 2 types of translation

Symmetric Translation

This means all the components on an entity are the same across all languages, but each component can different values per language. This is useful for sites that want the same layout and look across languages

Asymmetric Translation

The means each entity would have totally different components per language.

Steps to reproduce

Proposed resolution

We should be able to set the properties of the component_tree field to be translatable to achieve this

Symmetric Translation

For this tree property should not be translatable because it store which components are on the page
The props property would be translatable

Asymmetric Translation

For this both the tree and props properties should be translatable.

Remaining tasks

User interface changes

API changes

Data model changes

Hopefully none as the Content Translation module should allow this functionality

Issue fork experience_builder-3454257

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

tedbow created an issue. See original summary.

tedbow opened merge request !52

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Active » Needs work
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » Unassigned
Status: Needs work » Needs review
Related issues: +#3456000: If Experience Builder field is not translatable the dynamic source properties do not use the translated entity field value

All passing

Created follow-up for #3456000: If Experience Builder field is not translatable the dynamic source properties do not use the translated entity field value

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented

I figure out the problem with viewing entity as a translation and having the XB field being untranslated and showing the title of the default translation.

I figured out why this happens.

  1. \Drupal\Core\Entity\ContentEntityBase::get() calls \Drupal\Core\Entity\ContentEntityBase::getTranslatedField
  2. getTranslatedField has 
    if (!$default && !$definition->isTranslatable()) {
        if (!isset($this->fields[$name][LanguageInterface::LANGCODE_DEFAULT])) {
          $this->fields[$name][LanguageInterface::LANGCODE_DEFAULT] = $this->getTranslatedField($name, LanguageInterface::LANGCODE_DEFAULT);
        }
        $this->fields[$name][$langcode] = &$this->fields[$name][LanguageInterface::LANGCODE_DEFAULT];
      }
      else {

    So in the case the XB field is not translated it do a recursive call to getTranslatedField() but his time using LanguageInterface::LANGCODE_DEFAULT

  3. In the recursive call to getTranslatedField() then this is called
    $field = \Drupal::service('plugin.manager.field.field_type')->createFieldItemList($this->getTranslation($langcode), $name, $value);

    So $this->getTranslation($langcode) means the field item list will be created with the default, LanguageInterface::LANGCODE_DEFAULT translation, even though the entity that had the original call to \Drupal\Core\Entity\ContentEntityBase::get() in step above could have been the translation of another language.

The change was made in #2513094: ContentEntityBase::getTranslatedField and ContentEntityBase::__clone break field reference to parent entity

$field = \Drupal::service('plugin.manager.field.field_type')->createFieldItemList($this, $name, $value);

was changed to 

$field = \Drupal::service('plugin.manager.field.field_type')->createFieldItemList($this->getTranslation($langcode), $name, $value);

I will read up that issue. But presumably there was a good reason to need the translation instead of the original entity object.

I was able to use hook_entity_prepare_view to set the fields entity parent to the correct translation

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented

I think maybe we can close #2513094: ContentEntityBase::getTranslatedField and ContentEntityBase::__clone break field reference to parent entity as I think the use of hook_entity_prepare_view is an okay solution here. But I will research #2513094: ContentEntityBase::getTranslatedField and ContentEntityBase::__clone break field reference to parent entity a little more to be sure.

but I think hook_entity_prepare_view is a least a good solution for now and we could leave #2513094: ContentEntityBase::getTranslatedField and ContentEntityBase::__clone break field reference to parent entity open if we want to ask for some core change

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

@larowlan could we merge this and open a follow-up for the feedback? @tedbow is currently on PTO and it would be great to merge this if we think it's close enough 😇

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Will take a look early next week @lauriii

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » wim leers

Back from vacation. I reviewed all other issues that you worked on, @tedbow, because they seemed to be where your current attention is at.

Will review on Monday, unless @larowlan beats me to it.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » Unassigned
Status: Needs review » Needs work

Didn't get to it last week, but @tedbow was already working on the more urgent #3455728: FieldType: Support storing component *trees* instead of *lists*, which is now in 👍

Updated the MR to pass on the latest 0.x.

Questions on the MR.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » tedbow
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » wim leers
Status: Needs work » Needs review
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: wim leers » tedbow
Status: Needs review » Needs work

fixing phpcs

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » wim leers
Status: Needs work » Needs review

Fixed the phpcs problem expect for 1 in themes/engines/semi_coupled/semi_coupled.engine which was not changed. I guess there is new rule in core?
Anyways I could fix this other file too if we want to get green

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented

fixed the other file, can revert if needed

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

#3456024: Lift most logic out of ComponentTreeItem::preSave() and into a new validation constraint landed and conflicted, merged in upstream + resolved conflicts.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » tedbow
Status: Needs review » Needs work

Almost there: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » wim leers
Status: Needs work » Needs review

See MR comment

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » tedbow
Status: Needs review » Needs work

That MR comment is great — thank you! Pushed a commit that captures it for future readers.

As I was making a final pass, I realized that this MR is not actually using/updating the translation: symmetric|asymmetric setting yet 😅 See https://git.drupalcode.org/project/experience_builder/-/merge_requests/5....

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » wim leers
Status: Needs work » Needs review

Let me know if you agree with this and I can implement it https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...

Wim Leers credited effulgentsia.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » tedbow
Status: Needs review » Needs work

Discussed with @tedbow and @effulgentsia — we agreed that a setting might offer nicer UX, but that's for some very distant day in the future, that is nowhere near urgent. It doesn't even merit a follow-up issue. Because the current UX for configuring this also might be sufficient 👍

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » wim leers
Status: Needs work » Needs review
  1. Re #22 removed the setting
  2. Merged in 0.x and fixed tests(just merged in 2 more commits, think tests will pass🤞🏻)
  3. Assigning to @Wim Leers for review. Wim in the Gitlab CI I see

    The change requests must be completed or resolved.

    I made the changes I think you asked for and resolved the threads. Not sure where in the UI to mark this as resolved

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » tedbow
Status: Needs review » Needs work

Tantalizingly close — there's one thing I don't understand 😭

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » wim leers
Status: Needs work » Needs review
berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Not caught up on the whole discussion or experience builder storagae in general. I do want to add that the real complexity around translations starts to show when you add pending revisions, aka content_moderation/workflows to the mix. Because that's when you need to start to merge revision data on a per-field or even field-property (in case of ERR/paragraphs, in this case on whatever partial thingies you store/define here).

See also https://berdir.github.io/entities-explained/#/7/4 and slides before and after that.

Mostly an issue for symmetric translations. As an example, assume you have a thing that has a translatable text field, a partially translated image field (file id is synced, alt/title is per-translation) and an untranslatable list/reference field. You might create 3 draft translations in parallel for different languages and at the same time also change the untranslatable list field in the default translation. Then you publish all those in order and the resulting revision should then contain all translations and the untranslatable value from the default translations.

You don't need to and very likely don't want to support that complexity in this initial issue, but it needs to be on the radar, because this is is how content on multilingual sites works (you need to prepare changes on multiple languages and publish them all basically together). Paragraphs/ERR supports this fairly well, getting as far as we are was a large amount of work but people are still struggling with a number of scenarios (for example #3007233: Draft translations should be based on the latest revision of the source language, not the published version and there are several open issues in ERR that I'm struggling to understand).

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » Unassigned
Status: Needs review » Reviewed & tested by the community

Thank you so much for that, @Berdir!

This issue represents a big step forward, but I acknowledge it does not guarantee that everything works.

So:

  1. merging this MR
  2. extracted #26 into a new critical issue: #3469379: Test (a)symmetric XB field translations with revisions, pending revisions and content_moderation/workflows

  • Wim Leers committed 0e5b23e0 on 0.x authored by tedbow 
    Issue #3454257 by tedbow, Wim Leers, effulgentsia: Allow Experience...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Fixed
Related issues: +#3469379: Test (a)symmetric XB field translations with revisions, pending revisions and content_moderation/workflows

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.