? NPM Resource Changed ? ? Pending

User tests: Successful: Unsuccessful:

avatar drmenzelit
drmenzelit
17 May 2021

Replacing #31275.

Summary of Changes

Several changes in the code of mod_languages, based on the comments here: #31275 (comment)

Also changes in the CSS of the module itself and in Cassiopeia

Testing Instructions

You need a multilanguage Joomla installation. Create several language switcher modules using the different parameter combinations: with / without dropdown, with / without images... see #31275 (comment)

Install the patch and run npm ci

Actual result BEFORE applying this Pull Request

Sometimes missing alt description for the flags
Different select list (select list or dropdown) if flags are present or not
Shadow and arrow in the dropdown menu

Expected result AFTER applying this Pull Request

grafik

grafik

Documentation Changes Required

avatar drmenzelit drmenzelit - open - 17 May 2021
avatar drmenzelit drmenzelit - change - 17 May 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 17 May 2021
Category Repository NPM Change Language & Strings Modules Front End Templates (site)
avatar drmenzelit drmenzelit - change - 17 May 2021
Labels Added: ? NPM Resource Changed ? ?
avatar drmenzelit drmenzelit - change - 17 May 2021
Labels Added: ?
Removed: ?
avatar chmst
chmst - comment - 17 May 2021

The code looks good!

I don't like the layout of the options in the dropdown, but this is not related to this PR.

I am missing a focus on the language buttons or links. Do you others see a focus?

avatar drmenzelit drmenzelit - change - 17 May 2021
Labels Added: ?
Removed: ?
avatar drmenzelit drmenzelit - change - 17 May 2021
The description was changed
avatar drmenzelit drmenzelit - edited - 17 May 2021
avatar drmenzelit
drmenzelit - comment - 17 May 2021

I am missing a focus on the language buttons or links. Do you others see a focus?

Check the last commit, I have added focus to button and links (also updated the PR description)

avatar drmenzelit drmenzelit - change - 18 May 2021
Labels Added: ?
Removed: ?
avatar drmenzelit drmenzelit - change - 18 May 2021
Labels Added: ?
Removed: ?
avatar drmenzelit drmenzelit - change - 18 May 2021
Labels Added: ?
Removed: ?
avatar brianteeman
brianteeman - comment - 18 May 2021

Will review soon. Just a head s up that the aria-practices site is surprisingly not that good and is often heavily criticised by leading accessibility professionals.

avatar drmenzelit drmenzelit - change - 18 May 2021
Labels Added: ?
Removed: ?
avatar brianteeman
brianteeman - comment - 18 May 2021

Adding the a:focus css breaks the native browser focus.

With a:focus
b

Native without a:focus
final

avatar brianteeman
brianteeman - comment - 18 May 2021

Accessibility. the list box has no label or description. The hidden paragraph above the listbox is not connected to the listbox so will not be announced as intended as a description of the list.

avatar drmenzelit
drmenzelit - comment - 18 May 2021

Ĺabel added. I'm unsure about tabindex="-1" and aria-activedescendant="ID_REF" on the explanation here: https://www.w3.org/TR/wai-aria-practices/examples/listbox/listbox-collapsible.html
What do you think @brianteeman ?

avatar brianteeman
brianteeman - comment - 18 May 2021

you should not need either of those

avatar drmenzelit drmenzelit - change - 18 May 2021
The description was changed
avatar drmenzelit drmenzelit - edited - 18 May 2021
avatar drmenzelit drmenzelit - change - 18 May 2021
Labels Added: ?
Removed: ?
avatar brianteeman
brianteeman - comment - 18 May 2021

When tested with a screen reader it becomes obvious that when you are using the short name (eg EN, FR) it is not very helpful. I suggest when it is the short name being displayed there is an aria label of the long name

I am not convinced that aria-selected is the correct way to indicate the active language

avatar drmenzelit drmenzelit - change - 19 May 2021
Title
[4.0] [POC] [a11y] Changes in mod_languages
[4.0] [a11y] Changes in mod_languages
avatar drmenzelit drmenzelit - edited - 19 May 2021
avatar drmenzelit drmenzelit - change - 19 May 2021
Labels Added: ?
Removed: ?
avatar drmenzelit
drmenzelit - comment - 19 May 2021

With the last commit I added aria-label for the cases where full names are not being used. I tested all parameters combinations I could think about and also the case of a language not having an image. The test with screen reader on Chrome was ok.

Concerning aria-selected I can only point to the explanation of w3c
grafik
I will search for more information about that.

avatar brianteeman
brianteeman - comment - 19 May 2021

much better with the aria-label than the abbreviation

avatar brianteeman
brianteeman - comment - 19 May 2021

What is the purpose of
dir="<?php echo $app->getLanguage()->isRtl() ? 'rtl' : 'ltr'; ?>">

In my tests it did nothing

avatar drmenzelit drmenzelit - change - 19 May 2021
Labels Added: ?
Removed: ?
avatar drmenzelit
drmenzelit - comment - 19 May 2021

What is the purpose of
dir="<?php echo $app->getLanguage()->isRtl() ? 'rtl' : 'ltr'; ?>">

In my tests it did nothing

I'm not sure, probably historical reasons? @infograf768 do you know, if that is still necessary?

avatar drmenzelit drmenzelit - change - 19 May 2021
Labels Added: ?
Removed: ?
avatar infograf768
infograf768 - comment - 19 May 2021

I'm not sure, probably historical reasons? @infograf768 do you know, if that is still necessary?

Will check tomorrow.

Note: we do have a specific css for rtl.

html[dir=rtl] div.mod-languages ul.lang-block li { text-align: right;}

avatar infograf768
infograf768 - comment - 20 May 2021

We still do need the css.
Looks we do not need the dir anymore indeed as this PR takes off the form/select/option.
Test has to be done with Persian Content Language native name changed to فارس (Iran)

avatar drmenzelit drmenzelit - change - 20 May 2021
Labels Added: ?
Removed: ?
avatar drmenzelit drmenzelit - change - 20 May 2021
Labels Added: ?
Removed: ?
avatar hans2103 hans2103 - test_item - 20 May 2021 - Tested successfully
avatar hans2103
hans2103 - comment - 20 May 2021

I have tested this item successfully on e76e9aa


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33929.

avatar infograf768
infograf768 - comment - 20 May 2021

Looks like dir was added back in last commit.

avatar brianteeman
brianteeman - comment - 20 May 2021

double-checked and aria-selected is wrong and it should be aria-current

selected indicates which item will be
current indicates which item is

avatar drmenzelit drmenzelit - change - 20 May 2021
Labels Added: ?
Removed: ?
avatar drmenzelit
drmenzelit - comment - 20 May 2021

Changed to aria-current. Thank you @brianteeman for checking

avatar Krshivam25 Krshivam25 - test_item - 3 Jun 2021 - Tested successfully
avatar Krshivam25
Krshivam25 - comment - 3 Jun 2021

I have tested this item successfully on 7944760


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33929.

avatar drmenzelit drmenzelit - change - 4 Jun 2021
Labels Added: ?
Removed: ?
avatar carcam
carcam - comment - 5 Jun 2021

This fixes #34400

avatar drmenzelit
drmenzelit - comment - 23 Jun 2021

@hans2103 @Krshivam25 can you please repeat your tests since I have added some changes? Thank you :-)

avatar drmenzelit drmenzelit - change - 9 Jul 2021
Labels Added: ?
Removed: ?
avatar hans2103 hans2103 - test_item - 9 Jul 2021 - Tested successfully
avatar hans2103
hans2103 - comment - 9 Jul 2021

I have tested this item successfully on 8e249f7


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33929.

avatar Quy Quy - alter_testresult - 9 Jul 2021 - Krshivam25: Tested successfully
avatar Quy Quy - change - 9 Jul 2021
Status Pending Ready to Commit
avatar Quy
Quy - comment - 9 Jul 2021

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33929.

avatar drmenzelit drmenzelit - change - 12 Jul 2021
Labels Added: ?
Removed: ?
avatar richard67 richard67 - alter_testresult - 12 Jul 2021 - Krshivam25: Tested successfully
avatar richard67 richard67 - alter_testresult - 12 Jul 2021 - hans2103: Tested successfully
avatar richard67
richard67 - comment - 12 Jul 2021

I've restored both previous test results since all commits after the tests were just code style changes.

avatar brianteeman
brianteeman - comment - 12 Jul 2021

it would be a real shame if this doesnt make it into 4.0 because of the pending language freeze.

avatar wilsonge wilsonge - change - 13 Jul 2021
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-07-13 10:54:43
Closed_By wilsonge
Labels Added: ?
Removed: ?
avatar wilsonge wilsonge - close - 13 Jul 2021
avatar wilsonge wilsonge - merge - 13 Jul 2021
avatar wilsonge
wilsonge - comment - 13 Jul 2021

Thankyou!

Add a Comment

Login with GitHub to post a comment