Problem/Motivation

When translating, depending on the settings we might get all the Paragraphs open, even the ones that do not have translatable fields. This clutters the UI with fields that are not supposed to be translated.

Proposed resolution

Close paragraphs that do not have translatable fields while translating.

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yongt9412 created an issue. See original summary.

johnchque’s picture

Status: Active » Needs review
FileSize
7.7 KB

This should work. :)

Status: Needs review » Needs work

The last submitted patch, 2: close_paragraphs_translating-3008687-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

miro_dietiker’s picture

+++ b/src/Plugin/Field/FieldWidget/ParagraphsWidget.php
@@ -464,6 +464,28 @@ class ParagraphsWidget extends WidgetBase {
+      foreach (array_keys($display->get('content')) as $field) {
...
+      if ($this->isTranslating && $translating_closed && !isset($widget_state['paragraphs'][$delta]['mode'])) {

Really only do this loop and all this complexity if we are translating and it is until now considered open.
If we have such a force-closed Paragraph, we should maybe also hide the edit button as there's nothing you can do anyway.
If i'm wrong, we can always revert this direction...

Maybe we should look at this combined with the other requirement to always expand Paragraphs when adding a translation.
#2954921: Expand all Paragraphs recursively when adding a translation

There will be more to consider as unpublished paragraph translations could indicate that a translation is not yet ready, so expansion still makes sense on later edits. However it would be not if the semantic of an unpublished paragraph translation means it should persistently be hidden for that language. That's why we need to define how to deal with this.

Realizing that combining is boosting complexity and slowing us down and we maybe should go step by step.
So let's stay limited here, but include the hiding of the edit button.

Then we can do a UX assessment on real content instances to figure out if it creates frictions.

miro_dietiker’s picture

miro_dietiker’s picture

There's one thing we shouldn't forget:
If you don't have content moderation enabled, you can easily have untranslatable fields displayed when translating.
If someone prefers this flavor of editing everyting that is visible on the translation rendering on one page, we can not force close and hide.
Not sure where core goes, but we should only force collapse if fields are configured to be hidden.

So we need to check the setting at admin/config/regional/content-language for the content type in question: "Hide non translatable fields on translation forms"

johnchque’s picture

Thanks for the feedback. About only doing this when open, I think that we need to check in every widget state not that we are hiding the buttons, because we can have an already closed paragraph, that has no translatable fields and that has that content moderation setting enabled, then we would be able to open it and then get nothing.

Adding more tests. :)

Status: Needs review » Needs work

The last submitted patch, 7: close_paragraphs_translating-3008687-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

miro_dietiker’s picture

+++ b/src/Plugin/Field/FieldWidget/ParagraphsWidget.php
@@ -464,6 +464,36 @@ class ParagraphsWidget extends WidgetBase {
+      $translating_open = TRUE;
...
+        $translating_open = FALSE;

@@ -600,7 +630,7 @@ class ParagraphsWidget extends WidgetBase {
+              '#access' => $paragraphs_entity->access('update') && $translating_open,

That's a super strange name and IMHO the logic is inverted.

We want a variable $translating_force_close = FALSE -- maiby just $force_close -- initially that is set to TRUE if expanding is not allowed anymore.

Then below its && !$force_close;

It's simply weird otherwise if we have a variable "translating_open" that is set to TRUE if we are not translating at all...

Soo i see we have a bunch of fails? ;-)

johnchque’s picture

Right! Changed the name of the variable, tests are passing locally. Let's see what is happening here. :)

Status: Needs review » Needs work

The last submitted patch, 10: close_paragraphs_translating-3008687-10.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

johnchque’s picture

Oops, this should fix the tests fails.

Status: Needs review » Needs work

The last submitted patch, 12: close_paragraphs_translating-3008687-12.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

johnchque’s picture

Status: Needs work » Needs review
FileSize
10.67 KB
889 bytes

hmm my bad here.

miro_dietiker’s picture

Looks awesome. Some nitpick below..

+++ b/src/Plugin/Field/FieldWidget/ParagraphsWidget.php
@@ -464,6 +464,38 @@ class ParagraphsWidget extends WidgetBase {
+                  $translating_force_close = FALSE;
...
+                $translating_force_close = FALSE;

We can quit the loop with break on the first match here.
Rewriting a bit blindly.

miro_dietiker’s picture

Status: Needs review » Fixed

Awesome, committing this. :-)

Berdir’s picture

This is nice, but it might not be 100% obvious to users *why* they can't do anything with those paragraphs. Maybe we could use the free space to show some sort of notice that this paragraph can not be expanded because there's nothing to translate?

miro_dietiker’s picture

Status: Fixed » Closed (fixed)

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