Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OEL-1167: Theme the content language switcher #94

Merged
merged 36 commits into from
Mar 21, 2022
Merged

OEL-1167: Theme the content language switcher #94

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
8c6ef00
OEL-1167: Create oe_whitelabel_multilingual submodule.
Mar 10, 2022
54fb4b1
OEL-1167: Create the language switcher block template.
Mar 10, 2022
4633bca
OEL-1167: Preprocess variables for the language switcher template.
Mar 10, 2022
3ed54e2
OEL-1167: Refactor oe_whitelabel_multilingual.module.
Mar 14, 2022
ec1ab50
OEL-1167: Use placeholders on alert text and add a margin bottom to s…
Mar 14, 2022
d2d4536
OEL-1167: Create whitelabel multilingual kernel test.
Mar 14, 2022
a369c61
OEL-1167: Move template to avoid dependency on oe_whitelabel_multilin…
GilNovacomm Mar 16, 2022
af44289
OEL-1167: Update tests.
GilNovacomm Mar 16, 2022
ab65900
OEL-1167: Fix library commentary, and link icon definition.
GilNovacomm Mar 16, 2022
a58da02
OEL-1167: Remove unecessary registry empty statement.
GilNovacomm Mar 16, 2022
e812d79
OEL-1167: Assert block is not shown in test when viewing node on its …
GilNovacomm Mar 16, 2022
8e06e95
OEL-1167: Enable oe_whitelabel_multilingual during installation.
GilNovacomm Mar 16, 2022
7f61d64
OEL-1167: Sort test module dependencies and remove duplicated block r…
GilNovacomm Mar 16, 2022
52f27eb
OEL-1167: Code standards.
GilNovacomm Mar 16, 2022
862198b
OEL-1167: Update template comment and icon path.
GilNovacomm Mar 16, 2022
6bdc691
OEL-1167: Place the main page content block to ensure block order pla…
GilNovacomm Mar 16, 2022
b90af4f
OEL-1167: Update config dependency.
GilNovacomm Mar 16, 2022
d6111df
OEL-1167: Place the language switcher block on the page content region.
GilNovacomm Mar 17, 2022
d0341e0
OEL-1167: Move blocks to root module config/optional folder.
GilNovacomm Mar 17, 2022
44095f0
OEL-1167: Remove config dependency.
GilNovacomm Mar 17, 2022
c956a83
OEL-1167: Update dependencies and remove unexisting cas.settings.yml.
GilNovacomm Mar 17, 2022
bdeaf6a
OEL-1167: Configuration moved from oe_whitelabel_multiglingual.
GilNovacomm Mar 17, 2022
7eef2e1
OEL-1167: Rename multiligual lang swticher block and move to theme ro…
GilNovacomm Mar 17, 2022
7e6ce8a
OEL-1167: Rename content language switcher block.
drishu Mar 18, 2022
d7fc007
OEL-1167: Remove redundant variables.
drishu Mar 18, 2022
94185c9
OEL-1167: Fixup test.
drishu Mar 18, 2022
1b27d53
OEL-1167: Export block config.
drishu Mar 18, 2022
7bf25c4
OEL-1167: Rename content switcher in config_devel.
drishu Mar 18, 2022
1586043
OEL-1167: Remove content switcher block export.
drishu Mar 18, 2022
777b1ae
OEL-1167: Remove language_block preprocess from root.
drishu Mar 18, 2022
de3b993
OEL-1167: Fix typo in links block preprocess.
drishu Mar 18, 2022
5eaf632
OEL-1167: Add new module dependency to multilingual kernel test.
drishu Mar 18, 2022
bc274a4
OEL-1167: Account for language und.
drishu Mar 21, 2022
e5b2f37
OEL-1167: Generate expandable id.
drishu Mar 21, 2022
34998d5
OEL-1167: Fix config_devel settings.
drishu Mar 21, 2022
023a770
OEL-1167: Move var doc to multilingual helper service.
drishu Mar 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions modules/oe_whitelabel_multilingual/oe_whitelabel_multilingual.module
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

declare(strict_types = 1);

use Drupal\Component\Utility\Html;

/**
* Implements hook_preprocess_links().
*/
Expand Down Expand Up @@ -57,6 +59,9 @@ function oe_whitelabel_multilingual_preprocess_links__oe_multilingual_content_la
],
];
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the scenario where this happens?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// Generate required ids.
$variables['expandable_id'] = Html::getUniqueId('bcl-expandable');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if caching (render cache) could cause the same id to be used elsewhere on the same page.
But I think this would need a very convoluted scenario, which we should ignore for the time being.
So I am ok with it.

}

/**
Expand Down
2 changes: 1 addition & 1 deletion templates/overrides/navigation/links--oe-multilingual-content-language-block.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
_id, _message and _expandable are overwritten.
@todo Call include once BCL 0.20 is available.
#}
{% set _id = "dropdown-languages" %}
{% set _id = expandable_id %}
{% set _message = {
message: "This page is not available in @language."|t({'@language': unavailable_language}),
path: bcl_icon_path,
Expand Down
6 changes: 3 additions & 3 deletions tests/src/Kernel/ContentLanguageSwitcherTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public function testMultilingualLanguageSwitcherBlockRendering(): void {
$crawler = new Crawler($html);

// Make sure that content language switcher block is present.
$actual = $crawler->filter('#dropdown-languages');
$actual = $crawler->filter('div.collapse.mt-3');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like to see the .mt-3 here, but I suppose we have to for disambiguation?
Worst case we have to update this test when this class changes.

$this->assertCount(1, $actual);

// Warning message doesn't contain the unavailable language, the translation
Expand Down Expand Up @@ -154,7 +154,7 @@ public function testMultilingualLanguageSwitcherBlockRendering(): void {
*/
protected function assertSelectedLanguage(Crawler $crawler, string $expected): void {
// The selected language link will contain a svg, so we target that.
$actual = $crawler->filter('#dropdown-languages > div > a > svg')->parents()->first()->text();
$actual = $crawler->filter('div.collapse.mt-3 > div > a > svg')->parents()->first()->text();
$this->assertEquals($expected, trim($actual));
}
drishu marked this conversation as resolved.
Show resolved Hide resolved

Expand All @@ -180,7 +180,7 @@ protected function assertUnavailableLanguage(Crawler $crawler, string $expected)
* The labels of the translations that should be rendered as links.
*/
protected function assertTranslationLinks(Crawler $crawler, array $expected): void {
$elements = $crawler->filter('#dropdown-languages > div > a');
$elements = $crawler->filter('div.collapse.mt-3 > div > a');
// Filter out the selected language.
$elements = $elements->reduce(function (Crawler $crawler) {
return $crawler->filter('svg')->count() === 0;
Expand Down