Follow up for #1831530: Entity translation UI in core (part 2)

Problem/Motivation

a translation get marked outdated. edit the translation to update it. save it. notice forgot to uncheck the needs updating. edit the translation. expand the translation settings fieldset. uncheck box. save it. Ick.

Proposed resolution

when edit a translation that was marked outdated, auto expand the translation settings so dont forget to uncheck the needs update setting

Remaining tasks

User interface changes

see above. this is a ui issue.

API changes

no api changes expected.

Files: 
CommentFileSizeAuthor
#37 expand_translation-1833076-37.patch2.6 KBalansaviolobo
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 80,789 pass(es).
[ View ]
#37 after.png34.3 KBalansaviolobo
#37 before.png24.21 KBalansaviolobo
#34 expand_translation-1833076-1.patch2.61 KBalansaviolobo
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 80,759 pass(es), 16 fail(s), and 0 exception(s).
[ View ]
#33 content-translation-expand-outdated-1833076-33.patch2.95 KBherom
PASSED: [[SimpleTest]]: [MySQL] 58,118 pass(es).
[ View ]
#31 content-translation-expand-outdated-1833076-31.patch2.94 KBPancho
PASSED: [[SimpleTest]]: [MySQL] 58,016 pass(es).
[ View ]
#27 translation-entity-expand-outdated-1833076-27.patch2.91 KBherom
PASSED: [[SimpleTest]]: [MySQL] 57,268 pass(es).
[ View ]
#23 translation-entity-expand-outdated-1833076-23.patch2.93 KBpixelite
FAILED: [[SimpleTest]]: [MySQL] Repository checkout: failed to checkout from [git://git.drupal.org/project/drupal.git].
[ View ]
#20 expandoutdated-1833076-20-withtests.patch3.31 KBjessehs
FAILED: [[SimpleTest]]: [MySQL] 53,176 pass(es), 12 fail(s), and 0 exception(s).
[ View ]
#18 interdiff-6-14.txt1.57 KBherom
#18 interdiff-14-15.txt903 bytesherom
#15 expandoutdated-1833076-15-withtests.patch2.15 KBherom
PASSED: [[SimpleTest]]: [MySQL] 50,213 pass(es).
[ View ]
#14 expandoutdated-1833076-14-justtests.patch1.27 KBherom
FAILED: [[SimpleTest]]: [MySQL] 50,821 pass(es), 10 fail(s), and 0 exception(s).
[ View ]
#13 expandoutdated-1833076-13.patch837 bytesherom
PASSED: [[SimpleTest]]: [MySQL] 49,711 pass(es).
[ View ]
#6 expandoutdated-1833076-6-justtests.patch1.17 KBYesCT
FAILED: [[SimpleTest]]: [MySQL] 50,698 pass(es), 10 fail(s), and 0 exception(s).
[ View ]
#3 translation_interface_list_taxonony_outdated.png38.83 KB-enzo-
#3 translation_interface_taxonomy_term_fieldset_flag_other_as_outdated.png14.5 KB-enzo-
#3 translation_interface_taxonony_term_checkbox_remain_outdate.png14.8 KB-enzo-
#1 how_to_enable_translation_field_for_empty_field.png25.78 KB-enzo-
#1 how_to_enable_translation_field_for_field_with_data.png26.87 KB-enzo-
#1 transation_interface_vertical_tab_flag_others_as_ outdated.png32.27 KB-enzo-
#1 traslations_list_with_outdated_content.png34.43 KB-enzo-
#1 translation_interface_vertical_tab_remain_checked.png26.15 KB-enzo-

Comments

-enzo-’s picture

[edited to improve wording & add taxonomy sample]

Steps to reproduce with node entities.

1. Enable Entity Translation & Language Modules
2. Add a new language using page Administration » Configuration » Regional and language like Spanish for instance.
3. Edit a content type like Article at admin/structure/types/manage/article and Enable Entity Translation in Language Settings vertical tab
4. Edit a field and Enable the Entity Translation like Body field at admin/structure/types/manage/article/fields/body

The user interface will looks different if the entity has information or not information

Interface for empty entity

how_to_enable_translation_field_for_empty_field.png

Interface for entity with data

how_to_enable_translation_field_for_field_with_data.png

5. Add content and add translations
6. Add a translation content
7. Create new source version and flag other translations as out updated

transation_interface_vertical_tab_flag_others_as_ outdated.png

8. Now in translation tab you can see all translation outdated

traslations_list_with_outdated_content.png

8. Update your translation to follow the new version of source. If you save your content is still outdated, the only way to fix this is go to translation vertical tab and uncheck the checbox "This translation needs to be updated"

translation_interface_vertical_tab_remain_checked.png

9. Now after update your translation the content is still flag as outdated.

-enzo-’s picture

Assigned:Unassigned» -enzo-
-enzo-’s picture

Steps to reproduce with taxonomy entities.

1. Enable Entity Translation & Language Modules
2. Add a new language using page Administration » Configuration » Regional and language like Spanish for instance.
3. Edit a taxonomy like tags at admin/structure/taxonomy/tags/edit and Enable Translation in TERMS LANGUAGE fieldset
4. Edit a field and Enable the Entity Translation
5. Add terms and translations
6. Update original term field and flag others as outdated

translation_interface_taxonomy_term_fieldset_flag_other_as_outdated.png

7. Check the list of field terms outdated

translation_interface_list_taxonony_outdated.png

8. Edit a term and update the translation, at least you go and uncheck the checkbox "This fields need to be updated" the term will still remain as outdated

translation_interface_taxonony_term_checkbox_remain_outdate.png

-enzo-’s picture

Issue summary:View changes

Updated issue summary. Update remaning tasks to add link to steps to reproduce

-enzo-’s picture

Assigned:-enzo-» Unassigned
YesCT’s picture

@-enzo- has found the same as me.

For example in #3 step 8, the field set is collapsed by default and so I think many people will save it without expanding it and unchecking.

I suggest a fix of having in the edit form, when it is outdated, make the translations fieldset expanded by default.
(Keep the behavior of having that fieldset collapsed when it's not outdated.)

YesCT’s picture

StatusFileSize
new1.17 KB
FAILED: [[SimpleTest]]: [MySQL] 50,698 pass(es), 10 fail(s), and 0 exception(s).
[ View ]

Here is a start on tests. It fails when the translations setting is not expanded and it's outdated. I'm not sure if this is testing just Test Entities or those and nodes, terms, etc.

YesCT’s picture

The patch in #6 is only testing the test entity because it is in the file in the Test Entity directory. To test nodes, terms, etc have to go add similar code to their test files.

YesCT’s picture

Issue tags:+content translation, +D8MI

adding tags

plach’s picture

YesCT’s picture

I went back to look at the test I started in #6 to see how to add it to nodes, terms, etc and because they .. inherit the test so they are tested too.

YesCT’s picture

Status:Active» Needs review
Issue tags:+medium, +needs initial patch

the tests might need rerolled.

needs initial patch (the fix) to expand the field set when it's outdated.

Status:Needs review» Needs work

The last submitted patch, expandoutdated-1833076-6-justtests.patch, failed testing.

herom’s picture

Assigned:Unassigned» herom
Status:Needs work» Needs review
StatusFileSize
new837 bytes
PASSED: [[SimpleTest]]: [MySQL] 49,711 pass(es).
[ View ]

This patch seems to get the job done.

The ['#default_tab'] attribute is being used inside the '/include/form.inc' file, line 4018.

herom’s picture

Assigned:herom» Unassigned
StatusFileSize
new1.27 KB
FAILED: [[SimpleTest]]: [MySQL] 50,821 pass(es), 10 fail(s), and 0 exception(s).
[ View ]

Seeing into how the page gets rendered during the drupal webtests, it seems to me that the previous test patch is using a wrong assertion. Toggling the fieldset is done by adding an (open="open") attribute to the details tag, not by a "collapsed" class.
Here's what i think would be the correct test patch for this issue.

herom’s picture

StatusFileSize
new2.15 KB
PASSED: [[SimpleTest]]: [MySQL] 50,213 pass(es).
[ View ]

And here is my proposed patch to fix the issue, with the new tests.

herom’s picture

Issue tags:-needs initial patch

removing the "needs initial patch" tag.

YesCT’s picture

thanks. An interdiff would be handy (even if it might be bigger than the patch!), but I'm sure someone will review (the patch is small).

herom’s picture

StatusFileSize
new903 bytes
new1.57 KB

added missing interdiffs for the issue's patches.
(the patch on comment 13 was a partial patch, and is not included).

YesCT’s picture

jessehs’s picture

StatusFileSize
new3.31 KB
FAILED: [[SimpleTest]]: [MySQL] 53,176 pass(es), 12 fail(s), and 0 exception(s).
[ View ]

Re-rolling the patch in #15. It appears that the Translation fieldset is no longer a part of the vertical tabs, so the bit about it being the 'default tab' no longer applies. Also, the logic of the form-alter if statement in the patch in 15 has been incorporated in a different way (!$translate vs $translate).

Status:Needs review» Needs work

The last submitted patch, expandoutdated-1833076-20-withtests.patch, failed testing.

pixelite’s picture

StatusFileSize
new153.68 KB

I've applied the patch in #20 and followed the steps in #1. The fieldset for the translation status is open until the 'This translation needs to be updated' checkbox is unchecked (as expected).

Translation status fieldset

So it seems like the test for this in EntityTranslationUITest.php might be out-of-date.

pixelite’s picture

StatusFileSize
new2.93 KB
FAILED: [[SimpleTest]]: [MySQL] Repository checkout: failed to checkout from [git://git.drupal.org/project/drupal.git].
[ View ]

It looks like the id in the test is out-of-date. The id has changed from edit-translation to edit-translation-entity. Attaching a patch with the updated id in the test.

pixelite’s picture

Status:Needs work» Needs review

Forgot to change this to 'needs review'.

plach’s picture

Shouldn't the translation fieldset be a pane in the right sidebar?

Status:Needs review» Needs work

The last submitted patch, translation-entity-expand-outdated-1833076-23.patch, failed testing.

herom’s picture

Status:Needs work» Needs review
StatusFileSize
new2.91 KB
PASSED: [[SimpleTest]]: [MySQL] 57,268 pass(es).
[ View ]

well, the patch on #23 missed a case of changing 'edit-translation' to 'edit-translation-entity' in the test code.
this should pass the tests.

Status:Needs review» Needs work
Issue tags:-Usability, -D8MI, -language-content, -medium

The last submitted patch, translation-entity-expand-outdated-1833076-27.patch, failed testing.

YesCT’s picture

Status:Needs work» Needs review
Issue tags:+Usability, +D8MI, +language-content, +medium
Gábor Hojtsy’s picture

Component:translation_entity.module» content_translation.module
Pancho’s picture

StatusFileSize
new2.94 KB
PASSED: [[SimpleTest]]: [MySQL] 58,016 pass(es).
[ View ]
jair’s picture

Issue tags:+Needs reroll

Needs reroll

herom’s picture

Issue tags:-Needs reroll
StatusFileSize
new2.95 KB
PASSED: [[SimpleTest]]: [MySQL] 58,118 pass(es).
[ View ]

reroll done.

herom’s picture

Issue summary:View changes

Updated issue summary.

alansaviolobo’s picture

Issue summary:View changes
StatusFileSize
new2.61 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 80,759 pass(es), 16 fail(s), and 0 exception(s).
[ View ]

rerolled

YesCT’s picture

@alansaviolobo Thanks.

Please try the steps to reproduce with and without the patch and make a comment attaching screenshots (and embed them also in the summary).
Here are some instructions that might help:
https://drupal.org/contributor-tasks/add-screenshots

Status:Needs review» Needs work

The last submitted patch, 34: expand_translation-1833076-1.patch, failed testing.

alansaviolobo’s picture

Status:Needs work» Needs review
StatusFileSize
new24.21 KB
new34.3 KB
new2.6 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 80,789 pass(es).
[ View ]

updated the patch and also attached screenshots of the node edit RHS section before and after applying the patch.

balagan’s picture

Assigned:Unassigned» balagan
balagan’s picture

Patch applied I can see the changes according to the screenshots.

balagan’s picture

Assigned:balagan» Unassigned
Issue summary:View changes
Status:Needs review» Reviewed & tested by the community
plach’s picture

RTBC +1, thanks!

YesCT’s picture

@alansaviolobo or someone, please put the before and after screen shots embedded in the issue summary. (Attaching them is a good step in the right direction, but embedding them in the summary is the way to get people to see them)

Also, the steps to reproduce should go *in the summary*.

We don't want to make people search through comments for important information.

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

This issue is a prioritized change (usability) as per #2350615: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase? and it's benefits outweigh any disruption. Committed 6bf7909 and pushed to 8.0.x. Thanks!

  • alexpott committed 6bf7909 on 8.0.x
    Issue #1833076 by herom, alansaviolobo, Pancho, pixelite, jessehs, YesCT...
Bojhan’s picture

This help text is a bit lenghty, do we really need it?

Status:Fixed» Closed (fixed)

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