From #1023208: [i18n] Improve language switcher usability (D7):

As we can see here, D7 language switcher has a big UX problem, which was already present in D6: one can enable the language switcher block without performing the needed language negotiation settings. In this case the language switcher won't appear.

i18n fixed this for D7 by introducing a massage warning the user that she needs to configure language negotiation to have the language switcher actually displayed.

Do we want to straightforwardly port this to D8? UX team needed here.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

klonos’s picture

...coming from #729146: Language switcher block (User interface text language detection) doesn't appear.

I usually use i18n, so I never came across this one until I needed to help testing in #1060246: Translation alters language switch links even if node translation is disabled and I was hit bit this wtf. I know, I know... one needs to visit the 'Detection and selection' page and enable either one of the 'URL' or 'Session' methods in order for the block to actually show up, but...

I am re-opening this as a bug (IMHO it is at the very least a UX issue). I believe we should be checking if any of these two methods are enabled and if not do a couple of things...

1. Display a message to the user linking to the 'Detection and selection' page:

The Language switcher block is now enabled. In order to be displayed though either the URL and/or the Session language detection methods should be enabled. You can enable and configure them here[link to the 'Detection and selection' page].

2. In addition, there should be a section in the block's config page pointing to the 'Detection and selection' page and explaining what needs to be done. Ideally, instead of making the user go to that page, enable and configure the detection methods, then come back again to the block to configure it, we could have a radio box right there giving the user the following three options:

The Language switcher block is now enabled. In order to be displayed though either the URL and/or the Session language detection methods should be enabled.

( ) Enable the URL detection method
( ) Enable the Session detection method
( ) Enable both

[ Save ]

Finally, there should be a help text explaining to them that this 'shortcut' is provided for their convenience and that they should/might need to actually go to that config page later on to separately configure/tweak these methods in order to function as desired.

plach’s picture

LoMo’s picture

Issue tags: +D8MI

It's true. Work has been completed in Drupal 8 for the "other direction" (#1738374: Provide a cue to enable the language switcher when adding a language), but there still needs to be a message of some kind when enabling the Language Switcher block, and perhaps a link to "Help" and/or links to various language configuration pages ... or something that makes it a bit more clear that before this block is useful, there are some steps necessary for adding languages (and possibly enabling "translation" of content types). Anyway, I can confirm that if a new user starts by going to the Blocks page, before enabling multiple languages, they might be confused about the next steps they need to take for the Language switcher to actually show up or be useful.

Carsten Müller’s picture

Assigned: Unassigned » Carsten Müller
Carsten Müller’s picture

Status: Active » Needs review
FileSize
3.38 KB

Hi,

i agree with LoMo. So i started to search a solution. There is no hook or another possibility to set a custom message if a block gets enabled. That is why i added a ne hook "hook_block_admin_display_form_submit".
this is called directly after the update statement of the block settings, and provides the $block to the hook.
So each module can inteact on his or another block and can provide a message.

There is also a statement added which adds the old status of the block, so the module can interact if the block is enabled.

I don't know if this is a really good solution, but it is the only solution i have found at the moment.
And it provides the possibility to interact on any block actions in the admin interface by each module.

Please let me know if this is a solution which is worth to work on. Then we can extend this solution.

leschekfm’s picture

Status: Needs review » Needs work

I think it would be better to use form validation to check whether all necessary settings have been made for the language_switcher. If not there should be a validation error so that the block cannot be enabled and instructions what else needs to be set up.

Another fancy feature could be to introduce requirements for blocks. If those aren't met the block element should be disabled in the ui.

jair’s picture

Issue tags: +Needs reroll
alansaviolobo’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: block_language_switcher_enabling_message_108290_5.patch, failed testing.

mitsuroseba’s picture

Assigned: Carsten Müller » mitsuroseba
Issue summary: View changes
mitsuroseba’s picture

Assigned: mitsuroseba » Unassigned
DuttonMa’s picture

Assigned: Unassigned » DuttonMa
DuttonMa’s picture

Status: Needs work » Needs review
FileSize
1.94 KB

This is my first patch so be gentle with me :-)

The codebase had moved on so much that this is basically a re-write inspired by the initial patch.

I added 'invokeAll' for the new hook 'hook_block_admin_display_form_submit' into BlockForm::submit. (If anyone can think of a better place to add it then please let me know.)

I then implemented the new hook in the Language module. The thinking in here is that if the language switcher is being enabled and we only have one language defined on the system then display a message directing them to the language administration page.

Note that if more than one language is already defined then we should be OK. Further up this thread there was mention of checking whether a detection method had been set, but I notice that the current interface defaults to 'language from the url' so if multiple languages have been defined, something must be set.

Note also that I do not check to see whether the block was 'not enabled' before and is now 'enabled' as the only time that this hook will be called is when the 'System / Language Switcher' block is selected from the 'Place Blocks' area of the blocks admin page. I also intentionally did not check to see whether a region has been selected (although I could do) as I felt that the message should be displayed when selecting the block for the first time even if it isn't actually allocated to a region yet.

Carsten Müller’s picture

Hi DuttonMa,

nice work, i took your patch and extended it.

The old status is not needed, now the region is checked. If the block is assigned to a region the message is displayed. If no region is set, the message will nocht be shown.

I also added the new hook hook_block_admin_display_form_submit() in the BlockListBuilder.php form, because the region can also be changed in the blocks overview page of the theme and in the configuration form of the block. So both forms have to handle this.

open question
I'm just thinking about if the solution of leschekfm is maybe better than adding a new hook.

Carsten Müller’s picture

Alternative handling possiblity: When the block entity is saved the language module checks if there is only 1 language and if a region is set.
Advantage: Only one position where this is handled within the language module.

Maybe usage of hook_ENTITY_TYPE_update() and hook_ENTITY_TYPE_insert() in the language.module

Carsten Müller’s picture

new patch implementing hook_entity_type_insert() and hook_entity_type_update().
Much less code than before and without a new hook, much better i think.

Carsten Müller’s picture

Assigned: Carsten Müller » Unassigned
Gábor Hojtsy’s picture

Gábor Hojtsy’s picture

Issue tags: +SprintWeekend2015Queue
Kristen Pol’s picture

I keep getting An error occurred while patching the project. when trying to test with simplytest.me so I'm going to run the patch through tests again.

Status: Needs review » Needs work

The last submitted patch, 16: block_language_switcher_enabling_message_108290_16.patch, failed testing.

Gábor Hojtsy’s picture

Issue tags: +Needs reroll
ravi.khetri’s picture

Assigned: Unassigned » ravi.khetri
Status: Needs work » Needs review
Issue tags: +SprintWeekend2015
FileSize
2.85 KB

Re-rolled.

ravi.khetri’s picture

Assigned: ravi.khetri » Unassigned
DevElCuy’s picture

Issue tags: -SprintWeekend2015Queue
DevElCuy’s picture

Issue tags: +SprintWeekend2015Queue

Removed SprintWeekend2015Queue by mistake.

Kristen Pol’s picture

Thanks for the patch in #24.

I was confused about how to test because the issue summary says this is when the language negotiation settings are missing that text should be shown but this patch shows text if there is only one language.

I did manage to get the text to show up somewhere with just one language but then I added another language and then deleted the language and I can't figure out where the text showed up before. From what I can tell from the patch, it should show the text upon submit. Seems like it would be better if it happened when the user opened the configuration form for the language switcher rather than after submitting it. So...

1) Is the patch checking what it's supposed to be checking? i.e. there is only one language added. If so, then the issue summary needs to be updated to match.

2) Shouldn't we show text upon opening the language switcher form (before submitting)? I'll have to test again with a fresh install but I believe this wasn't the case.

Besides these more fundamental questions, there are some formatting things in the patch that would need work if this indeed is the right approach:

  1. +++ b/core/modules/block/src/BlockListBuilder.php
    @@ -398,6 +398,7 @@ public function validateForm(array &$form, FormStateInterface $form_state) {
       public function submitForm(array &$form, FormStateInterface $form_state) {
    +      ¶
         $entities = $this->storage->loadMultiple(array_keys($form_state->getValue('blocks')));
    

    Don't take out new line.

  2. +++ b/core/modules/block/src/BlockListBuilder.php
    @@ -410,6 +411,16 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
    +      ¶
    +      // let other modules interact if a block is changed
    

    Comments should be full sentences with punctuation.

  3. +++ b/core/modules/block/src/BlockListBuilder.php
    @@ -410,6 +411,16 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
    +      $messages = \Drupal::moduleHandler()->invokeAll('block_admin_display_form_submit', array($entity));
    

    Actually... is this the right approach to making sure messages are displayed? Seems odd to me but I don't look at D8 code often.

  4. +++ b/core/modules/block/src/BlockListBuilder.php
    @@ -410,6 +411,16 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
    +        foreach($messages as $message) {
    +          drupal_set_message($message);
    +        }
    

    Space needed after foreach.

  5. +++ b/core/modules/language/language.module
    @@ -508,6 +509,24 @@ function language_field_info_alter(&$info) {
    +  // If the language switcher block has been enabled, check to see whether
    +  // any languages have been added (if they haven't it won't work)
    

    Comments should be full sentences with punctuation. Try to avoid contractions and parentheses.

  6. +++ b/core/modules/language/language.module
    @@ -508,6 +509,24 @@ function language_field_info_alter(&$info) {
    +      $messages['advice'] = t('With the language switcher block you can switch the languages of the interface,
    +                                to do this you will need to add a language on the <a href="@language_admin">language administration page</a>',
    +                                array('@language_admin' => '/admin/config/regional/language'));
    

    Needs punctuation.

Kristen Pol’s picture

From #18, the issue summary for this is indeed a duplicate of #2019511: Explain why the language switcher would not show under some configurations. So... since the patch is really *not* a fix to what the issue summary says (see above).

For consistency, the same approach as #2019511: Explain why the language switcher would not show under some configurations would be best for handling the case where there is only one language. In fact, that code isn't differentiating between when it's the language negotiation settings vs when it's only one language. It's just checking if there are no links and *assuming* that it's because of the language negotiation settings which might not be the case.

I think this issue should be closed and #2019511: Explain why the language switcher would not show under some configurations should handle both of these cases.

manningpete’s picture

Issue tags: -Needs reroll

Last patch applies.

Gábor Hojtsy’s picture

Status: Needs review » Closed (duplicate)
Issue tags: +language-ui

I agree think it makes sense to not have two slightly similar issues.