OEL-1167: Theme the content language switcher
Created by: GilNovacomm
Merge request reports
Activity
105 $plugin_block = $block_manager->createInstance('oe_multilingual_content_language_switcher', $config); 106 $render = $plugin_block->build(); 107 $html = (string) $this->container->get('renderer')->renderRoot($render); 108 $crawler = new Crawler($html); 109 110 // Make sure that content language switcher block is present. 111 $actual = $crawler->filter('#dropdown-languages'); 112 $this->assertCount(1, $actual); 113 114 // Warning message doesn't contain the unavailable language, the translation 115 // will have it. 116 $this->assertUnavailableLanguage($crawler, 'This page is not available in български.'); 117 118 // Make sure that available languages are properly rendered. 119 $this->assertTranslationLinks($crawler, ['español', 'English']); 120 } Created by: drishu
why not continue with https://github.com/openeuropa/oe_theme/blob/a33ffc0b726fd3be5985c6c8ccd7033350c06e44/tests/src/Kernel/ContentLanguageSwitcherTest.php#L84 ?
14 */ 15 class ContentLanguageSwitcherTest extends KernelTestBase { 16 /** 17 * {@inheritdoc} 18 */ 19 public static $modules = [ 20 'block', 21 'content_translation', 22 'language', 23 'locale', 24 'node', 25 'oe_bootstrap_theme_helper', 26 'oe_multilingual', 27 'oe_whitelabel_helper', 28 'oe_whitelabel_multilingual', 29 'system', 5 namespace Drupal\Tests\oe_whitelabel\Kernel; 6 7 use Drupal\node\Entity\Node; 8 use Drupal\KernelTests\KernelTestBase; 9 use Symfony\Component\DomCrawler\Crawler; 10 use Symfony\Component\HttpFoundation\Request; 11 12 /** 13 * Test content language switcher rendering. 14 */ 15 class ContentLanguageSwitcherTest extends KernelTestBase { 16 /** 17 * {@inheritdoc} 18 */ 19 public static $modules = [ 20 'block', 130 $html = (string) $this->container->get('renderer')->renderRoot($render); 131 $crawler = new Crawler($html); 132 133 // Verify that the requested language is set as unavailable. 134 $this->assertUnavailableLanguage($crawler, 'This page is not available in español.'); 135 136 // Verify that the content has been rendered in the fallback language. 137 $this->assertSelectedLanguage($crawler, 'English'); 138 139 // Re-render the block assuming a request to the base language English 140 // version of the node. 141 $this->setCurrentRequest('/en/node/' . $node->id()); 142 143 $html = (string) $this->container->get('renderer')->renderRoot($render); 144 $crawler = new Crawler($html); 145 $render = $plugin_block->build(); 129 130 $html = (string) $this->container->get('renderer')->renderRoot($render); 131 $crawler = new Crawler($html); 132 133 // Verify that the requested language is set as unavailable. 134 $this->assertUnavailableLanguage($crawler, 'This page is not available in español.'); 135 136 // Verify that the content has been rendered in the fallback language. 137 $this->assertSelectedLanguage($crawler, 'English'); 138 139 // Simulate a request to the canonical route of the node base language. 140 $this->setCurrentRequest('/node/' . $node->id()); 141 142 $html = (string) $this->container->get('renderer')->renderRoot($render); 143 $crawler = new Crawler($html); 144 8 {% set _message = { 9 message: "This page is not available in @language."|t({'@language': unavailable_language}), 10 path: bcl_icon_path, 11 variant: "warning", 12 }%} 13 {% set _expandable = { 14 label: "Choose another language"|t, 15 icon: { 16 name: "caret-down-fill", 17 path: bcl_icon_path, 18 }, 19 outline: "true", 20 } %} 21 22 {% set _languages = languages|default([]) %} 23 {% set _expandable_attributes = _expandable.attributes ?: create_attribute() %} 5 @todo Call include once BCL 0.20 is available. 6 #} 7 {% set _id = "dropdown-languages" %} 8 {% set _message = { 9 message: "This page is not available in @language."|t({'@language': unavailable_language}), 10 path: bcl_icon_path, 11 variant: "warning", 12 }%} 13 {% set _expandable = { 14 label: "Choose another language"|t, 15 icon: { 16 name: "caret-down-fill", 17 path: bcl_icon_path, 18 }, 19 outline: "true", 20 } %} 21 22 {% set _languages = languages|default([]) %} 23 {% set _expandable_attributes = _expandable.attributes ?: create_attribute() %} 24 {% set _expandable_attributes = _expandable_attributes 25 .setAttribute('data-bs-toggle', 'collapse') 26 .setAttribute('autocomplete', 'off') 27 .setAttribute('aria-expanded', 'false') 28 .setAttribute('aria-controls', _id) 29 .setAttribute('data-bs-target', '#' ~ _id) 30 %} 31 32 {% include '@oe-bcl/bcl-alert/alert.html.twig' with _message only %} 33 <div class="mb-3"> 34 {% include '@oe-bcl/bcl-button/button.html.twig' with _expandable|merge({ 35 attributes: _expandable_attributes 36 }) only %} Created by: donquixote
Can we we drop the variable and use an inline value? (could be just my personal preference) E.g.
{# Dropdown-like button to hide/reveal the language list. #} {% include '@oe-bcl/bcl-button/button.html.twig' with { label: "Choose another language"|t, icon: { name: "caret-down-fill", path: bcl_icon_path, }, outline: "true", attributes: create_attribute() .setAttribute('data-bs-toggle', 'collapse') .setAttribute('autocomplete', 'off') .setAttribute('aria-expanded', 'false') .setAttribute('aria-controls', _id) .setAttribute('data-bs-target', '#' ~ _id), } only %}
27 .setAttribute('aria-expanded', 'false') 28 .setAttribute('aria-controls', _id) 29 .setAttribute('data-bs-target', '#' ~ _id) 30 %} 31 32 {% include '@oe-bcl/bcl-alert/alert.html.twig' with _message only %} 33 <div class="mb-3"> 34 {% include '@oe-bcl/bcl-button/button.html.twig' with _expandable|merge({ 35 attributes: _expandable_attributes 36 }) only %} 37 38 <div class="collapse mt-3" id="{{ _id }}"> 39 <div 40 class="d-md-grid" 41 style="grid-auto-flow: column; grid-template-rows: repeat(4, 1fr)" 42 > 7 {% set _id = "dropdown-languages" %} 8 {% set _message = { 9 message: "This page is not available in @language."|t({'@language': unavailable_language}), 10 path: bcl_icon_path, 11 variant: "warning", 12 }%} 13 {% set _expandable = { 14 label: "Choose another language"|t, 15 icon: { 16 name: "caret-down-fill", 17 path: bcl_icon_path, 18 }, 19 outline: "true", 20 } %} 21 22 {% set _languages = languages|default([]) %} 17 path: bcl_icon_path, 18 }, 19 outline: "true", 20 } %} 21 22 {% set _languages = languages|default([]) %} 23 {% set _expandable_attributes = _expandable.attributes ?: create_attribute() %} 24 {% set _expandable_attributes = _expandable_attributes 25 .setAttribute('data-bs-toggle', 'collapse') 26 .setAttribute('autocomplete', 'off') 27 .setAttribute('aria-expanded', 'false') 28 .setAttribute('aria-controls', _id) 29 .setAttribute('data-bs-target', '#' ~ _id) 30 %} 31 32 {% include '@oe-bcl/bcl-alert/alert.html.twig' with _message only %} Created by: donquixote
Again I would prefer inline parameters instead of a variable. But I am ok if the goal is to be aligned with the bcl template (snapshot at 67447742)
E.g.
{% include '@oe-bcl/bcl-alert/alert.html.twig' with { message: "This page is not available in @language."|t({'@language': unavailable_language}), path: bcl_icon_path, variant: "warning", } only %}
40 /** @var \Drupal\Core\Entity\EntityInterface $translation */ 41 $translation = $multilingual_helper->getCurrentLanguageEntityTranslation($entity); 42 $variables['languages'][] = [ 43 'path' => $translation->toUrl()->setAbsolute(TRUE)->toString(), 44 'hreflang' => $translation->language()->getId(), 45 'label' => $languages[$translation->language()->getId()]->getName(), 46 'current' => TRUE, 47 'icon_position' => 'before', 48 'remove_icon_spacers' => FALSE, 49 'icon' => [ 50 'name' => 'check-lg', 51 'path' => $variables['bcl_icon_path'], 52 'size' => 'xs', 53 '#attributes' => ['class' => ['me-2']], 54 ], 55 ]; Created by: donquixote
I wonder, why does
ContentLanguageBlock
fromoe_multilingual
not provide this link?Also, when we show this link, as a visitor I think I would want to see something like: "✓ English (shown instead of Polish)" instead of just "✓ English", if Polish is the language where we don't have a translation.
But it seems this has been decided in BCL and we are not going to change it here.
Created by: donquixote
I have some general feedback about UX for the design in BCL. This is not really the place to fix this, but I am going to mention it anyway:
- The open/close button looks like a radio element, but it does not behave as such, defying user expectations.
- The open/close icon always points down. It should point up when the list is opened.
Not a blocker for this PR.
43 38 - block.block.oe_whitelabel_branding 39 - block.block.oe_whitelabel_breadcrumbs 40 - block.block.oe_whitelabel_ec_corporate_footer 41 - block.block.oe_whitelabel_eu_corporate_footer 42 - block.block.oe_whitelabel_eulogin 43 - block.block.oe_whitelabel_language_switcher 44 44 - block.block.oe_whitelabel_local_actions 45 45 - block.block.oe_whitelabel_local_tasks 46 - block.block.oe_whitelabel_main_navigation 47 - block.block.oe_whitelabel_main_page_content 46 48 - block.block.oe_whitelabel_messages 47 - block.block.oe_whitelabel_page_title 48 - block.block.whitelabelsearchblock 49 - cas.settings.yml 49 - block.block.oe_whitelabel_neutral_footer 50 - block.block.oe_whitelabel_search_form Created by: donquixote
I found that these entries are already wrong in 1.x. Also in two other files:
- oe_whitelabel_starter_event.info.yml (wrong order)
- oe_whitelabel_starter_news.info.yml (".yml" suffix)
Imo we should have a separate branch to fix all of the entries, and then we rebase. But I can accept if we want to do this here to move forward.
46 'current' => TRUE, 47 'icon_position' => 'before', 48 'remove_icon_spacers' => FALSE, 49 'icon' => [ 50 'name' => 'check-lg', 51 'path' => $variables['bcl_icon_path'], 52 'size' => 'xs', 53 '#attributes' => ['class' => ['me-2']], 54 ], 55 ]; 56 } 57 58 /** 59 * Implements hook_preprocess_links__language_block(). 60 */ 61 function oe_whitelabel_multilingual_preprocess_links__language_block(&$variables) { Created by: donquixote
TBD: We could follow the pattern where we implement the preprocess hook on behalf of oe_whitelabel theme, instead of the module. This would prevent the hook from running if oe_whitelabel is not the active theme, e.g. during a test when we only test the structure and not the theme. I think it won't matter in this case, but we want to establish best practices that are safe to use elsewhere.
72 // @see CurrentUserContext::getRuntimeContexts(). 73 // @see EntityConverter::convert(). 74 module_load_include('install', 'user'); 75 user_install(); 76 77 \Drupal::service('kernel')->rebuildContainer(); 78 } 79 80 /** 81 * Tests the rendering of the language switcher block. 82 */ 83 public function testMultilingualLanguageSwitcherBlockRendering(): void { 84 $node = Node::create([ 85 'title' => 'Hello, world!', 86 'type' => 'oe_demo_translatable_page', 87 ]); 87 ]); 88 /** @var \Drupal\Core\Entity\EntityInterface $translation */ 89 $node->addTranslation('es', ['title' => '¡Hola mundo!'])->save(); 90 91 // Simulate a request to the canonical route of the node with Bulgarian 92 // language prefix. 93 $this->setCurrentRequest('/bg/node/' . $node->id()); 94 95 // Setup and render language switcher block. 96 $block_manager = \Drupal::service('plugin.manager.block'); 97 $config = [ 98 'id' => 'oe_multilingual_content_language_switcher', 99 'label' => 'Content language switcher', 100 'provider' => 'oe_multilingual', 101 'label_display' => '0', 102 ]; 109 109 $crawler = new Crawler($html); 110 110 111 111 // Make sure that content language switcher block is present. 112 $actual = $crawler->filter('#dropdown-languages'); 112 $actual = $crawler->filter('div.collapse.mt-3'); 57 59 ], 58 60 ]; 59 61 } 62 63 // Generate required ids. 64 $variables['expandable_id'] = Html::getUniqueId('bcl-expandable'); 44 if ($translation->language()->getId() !== 'und') { 45 $variables['languages'][] = [ 46 'path' => $translation->toUrl()->setAbsolute(TRUE)->toString(), 47 'hreflang' => $translation->language()->getId(), 48 'label' => $languages[$translation->language()->getId()]->getName(), 49 'current' => TRUE, 50 'icon_position' => 'before', 51 'remove_icon_spacers' => FALSE, 52 'icon' => [ 53 'name' => 'check-lg', 54 'path' => $variables['bcl_icon_path'], 55 'size' => 'xs', 56 '#attributes' => ['class' => ['me-2']], 57 ], 58 ]; 59 } Created by: donquixote
Btw I was considering to suggest early return pattern here (invert the if to get rid of indentation). But then we would have to drop the early return when we add the
$variables['expandable_id']
. This is why I prefer to use early return only in functions with a single responsibility - which is often not the case for preprocess. So ok to keep it this way.Created by: donquixote
Also if language is 'und', will the message "This page is not available in ***." make sense for all cases?
I think we should settle for something that works, we have no time to investigate all possible scenarios. If you say you want to keep the current version, let's go with it.
47 'icon_position' => 'before', 48 'remove_icon_spacers' => FALSE, 49 'icon' => [ 50 'name' => 'check-lg', 51 'path' => $variables['bcl_icon_path'], 52 'size' => 'xs', 53 '#attributes' => ['class' => ['me-2']], 54 ], 55 ]; 42 // If we don't have a language id defined yet, the current translation wasn't 43 // saved, so we don't add it to the list. 44 if ($translation->language()->getId() !== 'und') { 45 $variables['languages'][] = [ 46 'path' => $translation->toUrl()->setAbsolute(TRUE)->toString(), 47 'hreflang' => $translation->language()->getId(), 48 'label' => $languages[$translation->language()->getId()]->getName(), Created by: donquixote
We could introduce a local var for
$translation->language()->getId()
, to avoid the repeated call, e.g.$current_content_language_id
.We could also check whether
$languages[$current_content_language_id]
exists. Would be strange if it does not. Perhaps if a site language is deleted, renamed or disabled, while translations still exist? I don't think it will happen.
43 'path' => $translation->toUrl()->setAbsolute(TRUE)->toString(), 44 'hreflang' => $translation->language()->getId(), 45 'label' => $languages[$translation->language()->getId()]->getName(), 46 'current' => TRUE, 47 'icon_position' => 'before', 48 'remove_icon_spacers' => FALSE, 49 'icon' => [ 50 'name' => 'check-lg', 51 'path' => $variables['bcl_icon_path'], 52 'size' => 'xs', 53 '#attributes' => ['class' => ['me-2']], 54 ], 55 ]; 42 // If we don't have a language id defined yet, the current translation wasn't 43 // saved, so we don't add it to the list. 44 if ($translation->language()->getId() !== 'und') { Created by: donquixote
I notice you are using
'und'
string literal instead of a constant. In Drupal 7 I would have told you to useLANGUAGE_NONE
. Now this has changed, see LANGUAGE_NONE changed to LANGUAGE_NOT_SPECIFIED, LANGUAGE_NOT_APPLICABLE and LANGUAGE_MULTIPLE added Not sure if we also need to support other possible values. Perhaps safer to just checkisset($languages[$current_content_language_id])
?