-
Notifications
You must be signed in to change notification settings - Fork 1
#2461695 Support paragraphs field translations #1
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
Open
derhasi
wants to merge
30
commits into
8.x-1.x
Choose a base branch
from
2461695-field-translation
base: 8.x-1.x
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Patch: https://www.drupal.org/files/issues/paragraphs_field-translation-2461695.patch I worked out a solution for my use case. And the most simple solution is don't translate the paragraph entities. Wat i want is that the field of the host handles the translations and not the paragraph entities. When translating the host entity the paragraph entities are duplicated with the values of the original language instead of translated. // If there is no paragraph entity in this language create one and duplicate the fields. $target_langcode = $this->getCurrentLangcode($form_state, $items); if ($paragraphs_entity->language()->getId() != $target_langcode) { $values = $paragraphs_entity->toArray(); $entity_type = $entity_manager->getDefinition($target_type); $bundle_key = $entity_type->getKey('bundle'); $new_values = array( $bundle_key => $paragraphs_entity->bundle(), 'langcode' => $target_langcode, ); foreach ($paragraphs_entity->getFieldDefinitions() as $key => $value) { if (get_class($value) == 'Drupal\field\Entity\FieldConfig') { $new_values[$key] = $values[$key][0]; } } $paragraphs_entity = $entity_manager->getStorage($target_type)->create($new_values); } vs // If target translation is not yet available, populate it with data from the original paragraph. $target_langcode = $this->getCurrentLangcode($form_state, $items); if ($paragraphs_entity->language()->getId() != $target_langcode && !$paragraphs_entity->hasTranslation($target_langcode)) { $paragraphs_entity->addTranslation($target_langcode, $paragraphs_entity->toArray()); } // Initiate the paragraph with the correct translation. if ($paragraphs_entity->hasTranslation($this->getCurrentLangcode($form_state, $items))) { $paragraphs_entity = $paragraphs_entity->getTranslation($this->getCurrentLangcode($form_state, $items)); } else { $paragraphs_entity = $paragraphs_entity->addTranslation($this->getCurrentLangcode($form_state, $items)); } In this case the the paragraphs items can co-exist without conflicts. I don't there should be an obvious link between the two items. It is provided by the host field. In this way we can add delete and modify paragraph entities without it having an effect on the translation. If the host translation is deleted the paragraph items are deleted as wel. This is not the case with the translations in the current version. The patch attached will implement this method of translation. But it destroys the other method when you want the host field same for all languages and translate the paragraph fields instead. I think we need check the language settings for the host-field. If it is set to be translated. Create different entities. If not provide an option if the paragraph fields should be translated.
Patch: https://www.drupal.org/files/issues/paragraphs_multilingual-2461695-15.patch Attached is a re-roll of the patch from #11 which fixes the issue described in #13. It also copies all values for multi-valued nested paragraph fields, instead of just the first item. I agree that the ideal solution to this translation strategy ("translate the primary host ERR field; never translate fields on paragraph entities") is to extend the InlineParagraphsWidget...once the work in #268239 is stable and committed.
Patch: https://www.drupal.org/files/issues/paragraphs-meta_support-2461695-34.patch @miro_dietiker I agree with you that what is being discussed is two different things, translation and localization. To your point we should be careful with how we mix those two. Like others in this thread I have run into both of these situations with clients. Some would like purely translated content (same content different language) and others want localized content (different content for different languages). Each has a content structure that would greatly benefit from using paragraphs. Thanks in part to the way you have structured the module, I think the Paragraphs logic could be augmented to support localization in addition to translation with little need of altering the UI or worry about confusing the user. The following is my theory (as well as my patch) for how to simplistically support both use cases. Scenario 1: Translated Paragraphs ( currently implemented) In this Scenario the user does not translated the host (entity_reference) field, but instead translates the fields in the Paragraph Entity. For the host entity this is a 1:1 translation. When the user creates a translation of the Entity, the paragraphs are translated as well. The user cannot alter the paragraphs in the translated Entity without altering them for all Entity translations (as in adding or reorganizing the paragraphs). Scenario 2: Localized Paragraphs In this Scenario the user does not translate the fields in the Paragraph Entity, instead they translate the host entity field (entity_reference field). When the user creates/edits a translation of an Entity, each paragraph is duplicated as a new paragraph entity in the translated language. The user then has the ability to change the value in the entity Reference field (add, remove, edit, re-order the paragraphs) without effecting the other translations. Allowing them to 'localize' their paragraphs. ---- Setting up Paragraphs to work in this method requires no need for additional UI work as it allows paragraphs to follow the expected workflow when marking a field as translatable. If a field is marked as translatable you expect to be able to change the field value per translation. If a field is not translatable you expect it to contain the same value for all translations. My attached patch add this intrinsic functionality. When translating a paragraph entity, then host field is checked. If the host field is marked as translatable the logic then assumes the user is looking to localize the paragraphs and creates new paragraph entities in that language and assigns their value to the field. When editing a translated entity the logic also checks if the host field is translatable, and if so (since it is assume the content is 'localized') enables access to manipulate and add more paragraphs for that translation. This patch was very much influenced by the patch provided by @krlucas in comment #15, is almost basically a re-roll with some additional logic added. Hope this adds positively to this feature and discussion.
Patch: https://www.drupal.org/files/issues/paragraphs_localised-2461695.patch Rework patch #34 to the latest version.
Patch: https://www.drupal.org/files/issues/recursive_cloning_localised_paragraphs_field_collections-2461695-59.patch This patch is based on #44 with the addition that it goes into the paragraphs being cloned and clones any paragraphs or field collections in them. I don't really like that we had to hard-code the entities we support for cloning although the code doing the cloning itself is pretty generic (but I had to do so, because it's very often not very desirable to clone other entity types referenced in the paragraph --e.g. users, taxonomy terms and probably nodes-- ) ... In fact, whether an entity should be cloned or just referenced (when we are speaking about asymmetric translation) largely depends on whether the widget shown on that form allows the entity to be edited (but that's not a very practical condition, because a site builder can always change the widget of such an entity back and forth, which would leave the DB totally inconsistent) thoughts specifically about this are highly appreciated :)
Patch: https://www.drupal.org/files/issues/paragraphs-recursive_cloning_localised_paragraphs_field_collections-2461695-65.patch Okay, I recreated the patch with all the code style stuff and some extra syntactical enhancements :) Is it important that paragraphs are to be localized? What if the parent is translatable and the paragraph is not? Perhaps the paragraph does not need to be translatable (= localized). Different paragraph entities does not need to be translatable, as far as I can see. yes, if a paragraphs (entity revision) field is marked as translatable, this means that in different languages it will (be able to) reference different paragraph entities ... what that really means is: it doesn't really matter whether the paragraph itself is translatable or not, we'll always pre-populate the widget and make it ready to create a new paragraph entity that will be only referenced from this host translation (even if it has the exact same content). I prefer doing it like this because it's much easier to understand from an editor perspective (it's more confusing as a content editor to have some paragraphs that I can change per translation and some that would change everywhere if changed in one translation) It's unusual to pass the entity (type?) manager to the method. Does it change from one call to another? The entity_manager is being passed just because it was already instantiated in a pervious point in the code before actually calling this function, it doesn't change, it comes from the IDE refactoring, I've moved this to a property on the widget object to be shareable between methods. Is Drupal's entity reference protected against circular referencing? Otherwise we may need to check for this. (follow-up issue) I think it's not, although I believe it's not exactly possible from the admin UI, yet I don't think it's impossible to create a circular reference programmatically, which really makes this issue interesting to investigate.
Patch: https://www.drupal.org/files/issues/paragraphs_multilingual-2461695-72.patch The code that prepares the paragraph entity as part of InlineParagraphsWidget::formElement has grown in this patch. I think it is good to break this out into its own ::prepareEntity() method.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-81.patch Patch no longer applied. Created re-roll.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-82.patch This is the right patch.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-94-test-only.patch Started working on a test.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-96.patch There have been some commits into dev of Paragraphs since #94. I am attaching re-rolled patch against latest (today) dev.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-99.patch Attached a working patch against latest dev with the following extra's: #2778409: Incorrect closing <li> tag on paragraph add buttons was reverted in the patch in #2461695-96: [META] Support translatable paragraph entity reference revision field, I added it back in Removed all the warning messages about paragraph fields not being translatable Removed the tests to check if those warning messages were displayed I've also attached a patch against the latest stable release: meta_support-2461695-99-stable8.1.0.patch. (We prefer patches against stables) It contains the exact same functionality except #2778409: Incorrect closing <li> tag on paragraph add buttons because that was not released in 8.1.0.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-against-dev-120.patch I was researching the localized options for Paragraphs, mostly for the reasons like explained above. Our current project requires that the editor should be able to add/remove a paragraph per translated node. Tried to roll the #106 patch against the current dev version, but that failed so I rolled the failed parts of the patch by hand. Hope I got them all. (long live pull requests instead of patches) See Anyhow, attached you that patch, hopefully it works. And for what it is worth, hopes this will be the defacto behaviour for Paragraphs or else at least will continues to live as a separate community project as a separate widget because most of the 'magic' happens in there. The form overrides could be de-overridden in a .module.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-against-dev-128.patch I've re-rolled the patch id 120 from comment #121 since it failed on the DEV version. Further I've implemented the following fixes: In the scenario where you create a node in language 1 and then translate it, it is not possible to add more paragraphs into the translation. This is because the add buttons are not shown when translating a node. I've created an exception to check if the field is set as "translatable" before showing the add buttons. This way the add buttons won't show up on a paragraphs field that is not set as translatable. The comment from #127 referencing to #105 has also been applied. I don't know what this does, can someone check if this does not break anything in nested paragraphs ?
Patch: https://www.drupal.org/files/issues/meta_support-2461695-143.patch Made a small changes to #142 - Added if (!empty($form_state->get('content_translation'))) { - Allowed add/remove buttons
Patch: https://www.drupal.org/files/issues/meta_support-2461695-162.patch Patch no longer applied since 8.x-1.1 was released. Created a reroll.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-164.patch Patches #142 and #162 failed to apply towards the latest dev version. Created a new one.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-168-test-only.patch Another attempt to write a test for this issue.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-177.patch I have merged the patches from #168 to a single patch, and I have also written some more tests myself: Translate node and confirm different paragraphs. Update the paragraphs on each translation. Add different number of paragraphs on each translation. Remove paragraphs from each translation. Reorder the paragraphs on each translation. I tested in a clean installation of: D8.2.7 Paragraphs: latest dev (ae21d1a) ERR: latest dev (5e30fd0) All tests seem to pass, and the new feature seems to work fine.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-178.patch My tests seem to fail on PHP7 because of the regex I used. Updating the tests with a more simplified regex, that seems to work.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-184.patch I submit a new patch that: Adds a test for nested paragraphs (one level of nesting) Adds a test for deleting a translated node Removes some trailing spaces that were introduced by my previous patch (this can be seen at the interdiff) Please note that an important change was implemented, in order to test for nested paragraphs: The paragraph entity, and it's paragraph reference field had to be set as translatable. I left the previous configuration (that was setting them as non-translatable) commented on purpose.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-194.patch Having reviewed the patch in #189 I believe this is what @PavelKiryanov was hoping to contribute.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-202.patch I took patch #198 and add Regnoy’s fix. This solution works for me.
Patch: https://www.drupal.org/files/issues/meta_support-2461695-206.patch Confirm that patch #202 does not apply towards the latest dev version. Created a new one.
Patch: https://www.drupal.org/files/issues/paragraphs-8-1-DEV-d15b2e4----2461695-213.patch Updated the patch to work with drupal/paragraphs: 1.x-dev#d15b2e44281e5d85f9dfde164df61ccc8a764e9a
Patch: https://www.drupal.org/files/issues/2461695-220.patch Re-rolled patch from #212 against current head (1.x-dev#a0aa28bcfc0529e2b7a333009a214f30995a3208).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This Pseudo MR tries to keep track of changes in https://www.drupal.org/node/2461695
The history so far was generated with https://github.com/derhasi/drupal-org-issue-patch-history.