This is in-line with what's being done in core and also makes sense in general as it allows to get rid of the pesky if ($action == 'edit') and similar checks.

CommentFileSizeAuthor
#11 2111087-11.patch38.42 KBGábor Hojtsy
#11 interdiff.txt1.99 KBGábor Hojtsy
#9 2111087-9.patch36.43 KBGábor Hojtsy
#9 interdiff.txt2.02 KBGábor Hojtsy
#8 2111087-8.patch36.03 KBGábor Hojtsy
#5 2111087-5.patch13.8 KBtstoeckler
#5 2111087-4-5-interdiff.txt1.07 KBtstoeckler
#4 2111087-4.patch13.62 KBtstoeckler
#4 2111087-2-4-interdiff.txt1.8 KBtstoeckler
#2 2111087-4.patch13.46 KBtstoeckler
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Btw, including #2111013: Site information is displayed in the wrong language on the site information translation page in this one, as the same code will be touched.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Active » Needs review
FileSize
2111087-4.patch13.46 KB

Here we go.

Status: Needs review » Needs work

The last submitted patch, 2111087-4.patch, failed testing.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs work » Needs review
FileSize
2111087-2-4-interdiff.txt1.8 KB
2111087-4.patch13.62 KB

Well, that was only minimally smart of me... :-(

Oops, just realized the weird filename in #2. Interdiff and new patch should still be fine, though (and correctly named).

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
FileSize
2111087-4-5-interdiff.txt1.07 KB
2111087-5.patch13.8 KB

Is this my brain telling me to go to bed?

Weird, though, that this didn't fail locally.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented 
+++ b/lib/Drupal/config_translation/Form/ConfigTranslationFormBase.php
@@ -136,6 +150,11 @@ class ConfigTranslationManageForm extends FormBase {
+    // Leave the language context so that configuration accessed later in the
+    // requested is displayed in the correct language.

later in the *request*

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Nice, so this passed. I won't reroll for #6, though.
Since this still includes #2111013: Site information is displayed in the wrong language on the site information translation page it will need to be rerolled anyway once that is in.
@Gábor Hojtsy: Or if you want to commit the two together (i.e. this patch directly) maybe you can fix the comment pre-commit?!
I don't really care.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Issue tags: +D8MI, +language-config
FileSize
2111087-8.patch36.03 KB

Rerolled after #2111013: Site information is displayed in the wrong language on the site information translation page landed.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
FileSize
interdiff.txt2.02 KB
2111087-9.patch36.43 KB

Rolled in separate messages as discussed in #2110491-19: Modernize and clean-up controller and forms.

Status: Needs review » Needs work

The last submitted patch, 2111087-9.patch, failed testing.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs work » Needs review
FileSize
interdiff.txt1.99 KB
2111087-11.patch38.42 KB

With fix for tests, now testing separate messages as well.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs review » Reviewed & tested by the community

Awesome, I totally forgot about that!
The interdiffs are RTBC if this comes back green. Since this isn't core, marking the whole patch RTBC as well :-)

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Reviewed & tested by the community » Fixed

Yay, committed! Thanks a lot again!

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

BTW hook_menu() for routing was recently removed in #2106709: Remove legacy router backward compatibility layer, so now its only used for other things, and we only use it for contextual links, since there is no replacement yet (#2084463: Convert contextual links to a plugin system similar to local tasks/actions for that). So I'm not sure we are limited in terms of deepness in menus anymore(?). Re the hook_menu() limitation mentioned in the commit :)

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Well the contextual links are the ones that hit the parent limit, right? I.e. the contextual links on some fields or something if I remember correctly. So I think we need to wait on #2084463: Convert contextual links to a plugin system similar to local tasks/actions. But I didn't actually try it out, it might be that this actually works already. :-)

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Yeah maybe. Did not try it either :)

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