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.

CommentFileSizeAuthor
#37 expand_translation-1833076-37.patch2.6 KBalansaviolobo
#37 after.png34.3 KBalansaviolobo
#37 before.png24.21 KBalansaviolobo
#34 expand_translation-1833076-1.patch2.61 KBalansaviolobo
#33 content-translation-expand-outdated-1833076-33.patch2.95 KBherom
#31 content-translation-expand-outdated-1833076-31.patch2.94 KBpancho
#27 translation-entity-expand-outdated-1833076-27.patch2.91 KBherom
#23 translation-entity-expand-outdated-1833076-23.patch2.93 KBpixelite
#22 KZIALG-2013.5.24-13.16.png153.68 KBpixelite
#20 expandoutdated-1833076-20-withtests.patch3.31 KBjessehs
#18 interdiff-6-14.txt1.57 KBherom
#18 interdiff-14-15.txt903 bytesherom
#15 expandoutdated-1833076-15-withtests.patch2.15 KBherom
#14 expandoutdated-1833076-14-justtests.patch1.27 KBherom
#13 expandoutdated-1833076-13.patch837 bytesherom
#6 expandoutdated-1833076-6-justtests.patch1.17 KByesct
#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

-enzo- commented
Assigned: -enzo- » Unassigned
StatusFileSize
new translation_interface_vertical_tab_remain_checked.png26.15 KB
new traslations_list_with_outdated_content.png34.43 KB
new transation_interface_vertical_tab_flag_others_as_ outdated.png32.27 KB
new how_to_enable_translation_field_for_field_with_data.png26.87 KB
new how_to_enable_translation_field_for_empty_field.png25.78 KB

[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

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

-enzo- commented
Assigned: Unassigned » -enzo-
StatusFileSize
new translation_interface_taxonony_term_checkbox_remain_outdate.png14.8 KB
new translation_interface_taxonomy_term_fieldset_flag_other_as_outdated.png14.5 KB
new translation_interface_list_taxonony_outdated.png38.83 KB

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

-enzo- commented
Issue summary: View changes

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

-enzo-’s picture

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

yesct commented

@-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

yesct commented
StatusFileSize
new expandoutdated-1833076-6-justtests.patch1.17 KB

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

yesct commented

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

yesct commented
Issue tags: +content translation, +D8MI

adding tags

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Issue tags: -content translation +Usability, +language-content
yesct’s picture

yesct commented

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

yesct commented
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

herom commented
Assigned: Unassigned » herom
Status: Needs work » Needs review
StatusFileSize
new expandoutdated-1833076-13.patch837 bytes

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

herom commented
Assigned: herom » Unassigned
StatusFileSize
new expandoutdated-1833076-14-justtests.patch1.27 KB

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

herom commented
StatusFileSize
new expandoutdated-1833076-15-withtests.patch2.15 KB

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

herom’s picture

herom commented
Issue tags: -needs initial patch

removing the "needs initial patch" tag.

yesct’s picture

yesct commented

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

herom commented
StatusFileSize
new interdiff-14-15.txt903 bytes
new interdiff-6-14.txt1.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

yesct commented

#15: expandoutdated-1833076-15-withtests.patch queued for re-testing.

jessehs’s picture

jessehs
he/him
Primary language English
Location Taos, New Mexico
commented
StatusFileSize
new expandoutdated-1833076-20-withtests.patch3.31 KB

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

pixelite
Primary language English
Location Montreal
commented
StatusFileSize
new KZIALG-2013.5.24-13.16.png153.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

pixelite
Primary language English
Location Montreal
commented
StatusFileSize
new translation-entity-expand-outdated-1833076-23.patch2.93 KB

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

pixelite
Primary language English
Location Montreal
commented
Status: Needs work » Needs review

Forgot to change this to 'needs review'.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

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

herom commented
Status: Needs work » Needs review
StatusFileSize
new translation-entity-expand-outdated-1833076-27.patch2.91 KB

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

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

#27: translation-entity-expand-outdated-1833076-27.patch queued for re-testing.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Component: translation_entity.module » content_translation.module
pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented
StatusFileSize
new content-translation-expand-outdated-1833076-31.patch2.94 KB

Straight reroll of #27 after #2024867: Rename translation_entity to content_translation.

jair’s picture

jair commented
Issue tags: +Needs reroll

Needs reroll

herom’s picture

herom commented
Issue tags: -Needs reroll
StatusFileSize
new content-translation-expand-outdated-1833076-33.patch2.95 KB

reroll done.

herom’s picture

herom commented
Issue summary: View changes

Updated issue summary.

alansaviolobo’s picture

alansaviolobo commented
Issue summary: View changes
StatusFileSize
new expand_translation-1833076-1.patch2.61 KB

rerolled

yesct’s picture

yesct commented

@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

alansaviolobo commented
Status: Needs work » Needs review
StatusFileSize
new before.png24.21 KB
new after.png34.3 KB
new expand_translation-1833076-37.patch2.6 KB

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

balagan’s picture

balagan commented
Assigned: Unassigned » balagan
balagan’s picture

balagan commented

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

balagan’s picture

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

plach
he/him
Primary language Italian
Location Venezia
commented

RTBC +1, thanks!

yesct’s picture

yesct commented

@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

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
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

Bojhan commented

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.