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
- DONE
write precise steps to reproduce with nodes #1833076-1: expand translation settings when editing outdated translation so remember to uncheck "needs updating" and for taxonomies #1833076-3: expand translation settings when editing outdated translation so remember to uncheck "needs updating" screenshot with collapsedscreenshot with expandedwrite code that makes it expanded when the box is checkedmanually testmake sure existing tests pass(does not need new tests http://drupal.org/core-gates#testing)
User interface changes
see above. this is a ui issue.
API changes
no api changes expected.
Comment | File | Size | Author |
---|---|---|---|
#37 | expand_translation-1833076-37.patch | 2.6 KB | alansaviolobo |
#37 | after.png | 34.3 KB | alansaviolobo |
#37 | before.png | 24.21 KB | alansaviolobo |
#34 | expand_translation-1833076-1.patch | 2.61 KB | alansaviolobo |
#33 | content-translation-expand-outdated-1833076-33.patch | 2.95 KB | herom |
Comments
Comment #1
-enzo- CreditAttribution: -enzo- commented[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
Interface for entity with data
5. Add content and add translations
6. Add a translation content
7. Create new source version and flag other translations as out updated
8. Now in translation tab you can see all translation outdated
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"
9. Now after update your translation the content is still flag as outdated.
Comment #2
-enzo- CreditAttribution: -enzo- commentedComment #3
-enzo- CreditAttribution: -enzo- commentedSteps 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
7. Check the list of field terms outdated
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
Comment #3.0
-enzo- CreditAttribution: -enzo- commentedUpdated issue summary. Update remaning tasks to add link to steps to reproduce
Comment #4
-enzo- CreditAttribution: -enzo- commentedComment #5
YesCT CreditAttribution: 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.)
Comment #6
YesCT CreditAttribution: YesCT commentedHere 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.
Comment #7
YesCT CreditAttribution: YesCT commentedThe 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.
Comment #8
YesCT CreditAttribution: YesCT commentedadding tags
Comment #9
plachComment #10
YesCT CreditAttribution: YesCT commentedI 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.
Comment #11
YesCT CreditAttribution: YesCT commentedthe tests might need rerolled.
needs initial patch (the fix) to expand the field set when it's outdated.
Comment #13
herom CreditAttribution: herom commentedThis patch seems to get the job done.
The
['#default_tab']
attribute is being used inside the '/include/form.inc' file, line 4018.Comment #14
herom CreditAttribution: herom commentedSeeing 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.
Comment #15
herom CreditAttribution: herom commentedAnd here is my proposed patch to fix the issue, with the new tests.
Comment #16
herom CreditAttribution: herom commentedremoving the "needs initial patch" tag.
Comment #17
YesCT CreditAttribution: YesCT commentedthanks. 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).
Comment #18
herom CreditAttribution: herom commentedadded missing interdiffs for the issue's patches.
(the patch on comment 13 was a partial patch, and is not included).
Comment #19
YesCT CreditAttribution: YesCT commented#15: expandoutdated-1833076-15-withtests.patch queued for re-testing.
Comment #20
jessehsRe-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).
Comment #22
pixeliteI'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).
So it seems like the test for this in EntityTranslationUITest.php might be out-of-date.
Comment #23
pixeliteIt 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.
Comment #24
pixeliteForgot to change this to 'needs review'.
Comment #25
plachShouldn't the translation fieldset be a pane in the right sidebar?
Comment #27
herom CreditAttribution: herom commentedwell, the patch on #23 missed a case of changing 'edit-translation' to 'edit-translation-entity' in the test code.
this should pass the tests.
Comment #29
YesCT CreditAttribution: YesCT commented#27: translation-entity-expand-outdated-1833076-27.patch queued for re-testing.
Comment #30
Gábor HojtsyComment #31
PanchoStraight reroll of #27 after #2024867: Rename translation_entity to content_translation.
Comment #32
jair CreditAttribution: jair commentedNeeds reroll
Comment #33
herom CreditAttribution: herom commentedreroll done.
Comment #33.0
herom CreditAttribution: herom commentedUpdated issue summary.
Comment #34
alansaviolobo CreditAttribution: alansaviolobo commentedrerolled
Comment #35
YesCT CreditAttribution: 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
Comment #37
alansaviolobo CreditAttribution: alansaviolobo commentedupdated the patch and also attached screenshots of the node edit RHS section before and after applying the patch.
Comment #38
balagan CreditAttribution: balagan commentedComment #39
balagan CreditAttribution: balagan commentedPatch applied I can see the changes according to the screenshots.
Comment #40
balagan CreditAttribution: balagan commentedComment #41
plachRTBC +1, thanks!
Comment #42
YesCT CreditAttribution: 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.
Comment #43
alexpottThis 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!
Comment #45
Bojhan CreditAttribution: Bojhan commentedThis help text is a bit lenghty, do we really need it?