User tests: Successful: Unsuccessful:
This only applies to the language switcher module on the front end when you have selected the options to use image flags and not to use a dropdown
Here the visual output will be similar to this (depending on the installed languages)
Each of these flags (no discussion please here about country flags vs languages) is a link to the site in the selected language.
Currently the image has an alt text that is identical to the title text. This is bad practice. In addition the title text is of no benefit for accessibility. It is sufficient in this case to just have the alt text and this will be used by default for the link name for accessibility purposes.
Status | New | ⇒ | Pending |
Category | ⇒ | Modules Front End |
I have tested this item
Can we consider a tooltip?
#30061 (comment)
The title attribute is shown to the user when hovering. It's basically a native tooltip and I think that was the purpose of it.
The alt attribute isn't show to the user, only if the image can't be loaded - that's also the reason why screenreaders read it (because the user can't see the image).
Of course, it doesn't make sense to have the same text in both attributes. But I don't think the solution is to remove the title. The solution would be to have meaningful and appropriate texts instead. Alt should ideally probably be "flag of France" and title should be "switch to française"
Alt should ideally probably be "flag of France" and title should be "switch to française"
Although I agree on the principle and we can use for title : Switch to $language->title_native
, "flag of France" would not fit.
The reason is that this would not only require a new string and column in db, but also makes no sense for some languages images which are not country flags. Example: ar-AA, eo-XX, ta-IN, sy-IQ, si-LK, ug-CN , sw-KE
Let's not forget that the use of flags for quite a few languages is just an easy way of showing an image representing the language.
That's why I wrote "ideally"
We should now have a look what could be appropriate.
I have tested this item
I propose to close this PR, leave the title as is (when necessary) and modify the alt (including adding it for dropdown) in a new PR.
Wa would give, active language displayed. Label added and class changed in case of using select.
dropdown
inline
diff --git a/language/en-GB/mod_languages.ini b/language/en-GB/mod_languages.ini
index b23bdc7..c313ab2 100644
--- a/language/en-GB/mod_languages.ini
+++ b/language/en-GB/mod_languages.ini
@@ -14,3 +14,5 @@
MOD_LANGUAGES_FIELD_LINEHEIGHT_LABEL="Reduce Line Height"
MOD_LANGUAGES_FIELD_USEIMAGE_LABEL="Use Image Flags"
+MOD_LANGUAGES_ALT_ACTIVE="%s is the active language"
+MOD_LANGUAGES_ALT_SWITCH="Switch to %s"
MOD_LANGUAGES_XML_DESCRIPTION="This module displays a language switcher on your website of available content languages."
diff --git a/modules/mod_languages/tmpl/default.php b/modules/mod_languages/tmpl/default.php
index 5bec688..492da2e 100644
--- a/modules/mod_languages/tmpl/default.php
+++ b/modules/mod_languages/tmpl/default.php
@@ -11,4 +11,5 @@
use Joomla\CMS\HTML\HTMLHelper;
+use Joomla\CMS\Language\Text;
use Joomla\CMS\Uri\Uri;
@@ -25,5 +26,6 @@
<?php if ($params->get('dropdown', 0) && !$params->get('dropdownimage', 1)) : ?>
<form name="lang" method="post" action="<?php echo htmlspecialchars_decode(htmlspecialchars(Uri::current(), ENT_COMPAT, 'UTF-8'), ENT_NOQUOTES); ?>">
- <select class="inputbox advancedSelect" onchange="document.location.replace(this.value);" >
+ <label for="lang" class="sr-only"><?php echo Text::_('JOPTION_SELECT_LANGUAGE'); ?></label>
+ <select id="lang" class="inputbox custom-select" onchange="document.location.replace(this.value);" >
<?php foreach ($list as $language) : ?>
<option dir=<?php echo $language->rtl ? '"rtl"' : '"ltr"'; ?> value="<?php echo htmlspecialchars_decode(htmlspecialchars($language->link, ENT_QUOTES, 'UTF-8'), ENT_NOQUOTES); ?>" <?php echo $language->active ? 'selected="selected"' : ''; ?>>
@@ -39,5 +41,5 @@
<span class="caret"></span>
<?php if ($language->image) : ?>
- <?php echo HTMLHelper::_('image', 'mod_languages/' . $language->image . '.gif', '', null, true); ?>
+ <?php echo HTMLHelper::_('image', 'mod_languages/' . $language->image . '.gif', Text::sprintf('MOD_LANGUAGES_ALT_ACTIVE', $language->title_native), null, true); ?>
<?php endif; ?>
<?php echo $params->get('full_name', 1) ? $language->title_native : strtoupper($language->sef); ?>
@@ -51,5 +53,5 @@
<a href="<?php echo htmlspecialchars_decode(htmlspecialchars($language->link, ENT_QUOTES, 'UTF-8'), ENT_NOQUOTES); ?>">
<?php if ($language->image) : ?>
- <?php echo HTMLHelper::_('image', 'mod_languages/' . $language->image . '.gif', '', null, true); ?>
+ <?php echo HTMLHelper::_('image', 'mod_languages/' . $language->image . '.gif', Text::sprintf('MOD_LANGUAGES_ALT_SWITCH', $language->title_native), null, true); ?>
<?php endif; ?>
<?php echo $params->get('full_name', 1) ? $language->title_native : strtoupper($language->sef); ?>
@@ -61,5 +63,5 @@
<a href="<?php echo htmlspecialchars_decode(htmlspecialchars($base, ENT_QUOTES, 'UTF-8'), ENT_NOQUOTES); ?>">
<?php if ($language->image) : ?>
- <?php echo HTMLHelper::_('image', 'mod_languages/' . $language->image . '.gif', '', null, true); ?>
+ <?php echo HTMLHelper::_('image', 'mod_languages/' . $language->image . '.gif', Text::sprintf('MOD_LANGUAGES_ALT_ACTIVE', $language->title_native), null, true); ?>
<?php endif; ?>
<?php echo $params->get('full_name', 1) ? $language->title_native : strtoupper($language->sef); ?>
@@ -78,5 +80,5 @@
<?php if ($params->get('image', 1)) : ?>
<?php if ($language->image) : ?>
- <?php echo HTMLHelper::_('image', 'mod_languages/' . $language->image . '.gif', $language->title_native, array('title' => $language->title_native), true); ?>
+ <?php echo HTMLHelper::_('image', 'mod_languages/' . $language->image . '.gif', Text::sprintf('MOD_LANGUAGES_ALT_SWITCH', $language->title_native), array('title' => $language->title_native), true); ?>
<?php else : ?>
<span class="label" title="<?php echo $language->title_native; ?>"><?php echo strtoupper($language->sef); ?></span>
@@ -94,5 +96,5 @@
<?php if ($params->get('image', 1)) : ?>
<?php if ($language->image) : ?>
- <?php echo HTMLHelper::_('image', 'mod_languages/' . $language->image . '.gif', $language->title_native, array('title' => $language->title_native), true); ?>
+ <?php echo HTMLHelper::_('image', 'mod_languages/' . $language->image . '.gif', Text::sprintf('MOD_LANGUAGES_ALT_ACTIVE', $language->title_native), array('title' => $language->title_native), true); ?>
<?php else : ?>
<span class="badge badge-secondary"><?php echo strtoupper($language->sef); ?></span>
RTC?
There was no reply to my proposal.
Will make alternative PR as proposed above and close this one.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-10-30 09:01:37 |
Closed_By | ⇒ | infograf768 | |
Labels |
Added:
?
|
This should not be closed just because you disagree
Status | Closed | ⇒ | New |
Closed_Date | 2020-10-30 09:01:37 | ⇒ | |
Closed_By | infograf768 | ⇒ |
Status | New | ⇒ | Pending |
re-opened. I am not the only one to disagree and you have not even taken the time to comment my proposal which make more sense.
I have tested this item
This is wrong. See alternative: #31275
I have commented on your proposal
Please explain what is the criteria for marking this as a failed test
RTC
It is not rtc...
thats why i wrote RTC as you ignored "Please explain what is the criteria for marking this as a failed test".
Please explain what is the criteria for marking this as a failed test - not saying if you are right or wrong but it should at least be recorded here why you are saying that you tested it and it failed.
As you very well know, the PR does what it says. Marking it as unsuccessful here is a way to oppose to it and preventing it from being set as RTC as asked by @gostn .
It is not because we have 2 testers who test OK without taking into account remarks by senior devs and an alternative proposed that it should be set RTC and merged by accident.
Basically, it's a red flag which means "Don't merge this!"
I am not interested into further discussion.
Great - you've invented a new way of working here and redefined the words Failed test and cant be bothered to explain why are you so special that you get to reinvent the way we work and dont have to explain anything?
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-11-04 10:25:19 |
Closed_By | ⇒ | brianteeman |
It is not because we have 2 testers who test OK without taking into account remarks by senior devs
careless handling of time and work of tester.
Normally I do not engage much in these discussions, but spend some time testing since I saw that this project lacks testers. Since I was directly adressed by @infograf768 here the response
Marking it as unsuccessful here is a way to oppose to it and preventing it from being set as RTC as asked by @gostn.
From the little I know this decision is up to the release lead/module owners!
It is not because we have 2 testers who test OK without taking into account remarks by senior devs and an alternative proposed that it should be set RTC and merged by accident.
I have rarely seen such an disrespectful comment in this repo. Maybe the testers are senior senior devs
Not sure what your intention with the senior dev part is. But again, when you want to make the decision what is merged, you should come forward for the release lead role.
Basically, it's a red flag which means "Don't merge this!"
A comment or label would have been sufficient!
I am not interested into further discussion.
Not very team-minded.
PR for #30061?