Language Change a11y NPM Resource Changed PBF bug Small PR-4.4-dev Pending

User tests: Successful: Unsuccessful:

avatar JackKellyUK
JackKellyUK
6 Dec 2022

Summary of Changes

Awesomplete is raising a Serious issue in axe for a missing title / aria-label attribute.

image

Added title attribute to results list.

There is an open issue about this, a fix looks to be implemented with the listLabel property, but it is currently unreleased.

Testing Instructions

  • Setup Smart Search > Search menu item (Smart Search > Search Suggestions global setting must be enabled)
  • Run axe browser plugin / find awesomplete list ul and check for title / aria-label attribute

Actual result BEFORE applying this Pull Request

No title / aria-label attribute present on Awesomplete list

Expected result AFTER applying this Pull Request

Title attribute present on Awesomplete list

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

avatar JackKellyUK JackKellyUK - open - 6 Dec 2022
avatar JackKellyUK JackKellyUK - change - 6 Dec 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 6 Dec 2022
Category JavaScript Repository NPM Change
avatar JackKellyUK JackKellyUK - change - 6 Dec 2022
Title
Added title to awesomplete results list
Awesomplete accessibility fix
avatar JackKellyUK JackKellyUK - edited - 6 Dec 2022
avatar brianteeman
brianteeman - comment - 6 Dec 2022

What am I missing here as I dont see an accessibility issue
image

avatar JackKellyUK
JackKellyUK - comment - 7 Dec 2022

What am I missing here as I dont see an accessibility issue image

If you enter a search term that populates the list you should see that the name property is blank.

avatar dgrammatiko
dgrammatiko - comment - 7 Dec 2022

@JackKellyUK right now the label you're applying will only work for English language. What I'll suggest is either

  • extend the Joomla.getOptions('finder-search') to pass also the translated string for the label
  • or use a data-ul-label="<?php echo Text::_('SOME_STRING'); ?>" and grab the text from there

Although I'm the original author of the script (#21438) I think the whole script could be rewritten with way less code (eg the DOMContentLoaded event is harmful since the script is loaded either as a module or deferred, etc)

avatar wilsonge
wilsonge - comment - 7 Dec 2022

What am I missing here as I dont see an accessibility issue image

Not sure. But wouldn't this be exposed once you've entered some text

avatar joomla-cms-bot joomla-cms-bot - change - 10 Dec 2022
Category JavaScript Repository NPM Change JavaScript Repository NPM Change Front End com_finder Language & Strings Modules
avatar JackKellyUK JackKellyUK - change - 10 Dec 2022
Labels Added: a11y NPM Resource Changed ?
avatar JackKellyUK
JackKellyUK - comment - 10 Dec 2022

@JackKellyUK right now the label you're applying will only work for English language. What I'll suggest is either

  • extend the Joomla.getOptions('finder-search') to pass also the translated string for the label
  • or use a data-ul-label="<?php echo Text::_('SOME_STRING'); ?>" and grab the text from there

Although I'm the original author of the script (#21438) I think the whole script could be rewritten with way less code (eg the DOMContentLoaded event is harmful since the script is loaded either as a module or deferred, etc)

Thanks for the suggestion. Does this implementation look correct to you? 8eefbd4

avatar dgrammatiko
dgrammatiko - comment - 10 Dec 2022

Thanks for the suggestion. Does this implementation look correct to you? 8eefbd4

LGTM

avatar brianteeman
brianteeman - comment - 10 Dec 2022

The new string should probably just be "Search Results" - otherwise it will be "list of Results list"

avatar JackKellyUK JackKellyUK - change - 12 Dec 2022
Labels Added: Language Change
avatar HLeithner
HLeithner - comment - 2 May 2023

This pull request has been automatically rebased to 4.3-dev.

avatar obuisard obuisard - change - 15 Jul 2023
Labels Added: bug PR-4.3-dev ?
Removed: ?
avatar obuisard
obuisard - comment - 15 Jul 2023

Awesomplete has not been updated in over 3 years, although the issue has been fixed in that library...

avatar dgrammatiko
dgrammatiko - comment - 15 Jul 2023

Joomla is using the latest version:

"awesomplete": "^1.1.5",

https://www.npmjs.com/package/awesomplete?activeTab=versions

avatar obuisard
obuisard - comment - 15 Jul 2023

Joomla is using the latest version:

"awesomplete": "^1.1.5",

https://www.npmjs.com/package/awesomplete?activeTab=versions

Indeed, which means the fix that has been done in that library is not included in Joomla, therefore the fix here is still relevant.

avatar brianteeman
brianteeman - comment - 15 Jul 2023

i must be missing something as i dont see any fix upstream just an open issue

avatar obuisard
obuisard - comment - 15 Jul 2023

i must be missing something as i dont see any fix upstream just an open issue

No problem, Brian @brianteeman. My understanding is this:

At the Awesomplete project, has been created a PR LeaVerou/awesomplete#17200 that fixes the issue we are having in Joomla. Because our issue comes from the library we are using apparently. The PR that fixes the issue in Awesomplete has been merged, but after 1.1.5 was released, which is the version we are using (but also the last official update).

avatar HLeithner
HLeithner - comment - 30 Sep 2023

This pull request has been automatically rebased to 4.4-dev.

avatar HLeithner HLeithner - change - 24 Apr 2024
Title
Awesomplete accessibility fix
[4.4] Awesomplete accessibility fix
avatar HLeithner HLeithner - edited - 24 Apr 2024
avatar drmenzelit drmenzelit - change - 10 Aug 2024
Labels Added: PBF Small PR-4.4-dev
Removed: PR-4.3-dev ?
avatar drmenzelit
drmenzelit - comment - 10 Aug 2024

After years of inactivity there is a new version of Awesomplete available: 1.1.7
https://www.npmjs.com/package/awesomplete?activeTab=versions

We should check, if we can update it in Joomla. @dgrammatiko what do you think?

avatar dgrammatiko
dgrammatiko - comment - 10 Aug 2024

The 1.1.7 has solved this with: LeaVerou/awesomplete@3c7ee84

	configure(this, {
		minChars: 2,
		maxItems: 10,
		autoFirst: false,
		data: _.DATA,
		filter: _.FILTER_CONTAINS,
		sort: o.sort === false ? false : _.SORT_BYLENGTH,
		container: _.CONTAINER,
		item: _.ITEM,
		replace: _.REPLACE,
		tabSelect: false,
		listLabel: "Results List", // <-- this
		statusNoResults: "No results found",
		statusXResults: "{0} results found", // uses index placeholder {0}
		statusTypeXChar: "Type {0} or more characters for results"
	}, o);

So this PR should just pass the listLabel: Text._('COM_FINDER_SEARCH_FORM_LIST_LABEL') to the initialization of the script. Probably use a FORM_FIELD_AUTOCOMPLETE_RESULTS or something similar (in the global translation files).
Probably it is easier to do another PR

Add a Comment

Login with GitHub to post a comment