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.

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

[4.0] Filtered by - accessibility #27741

Closed
wants to merge 2 commits into from

Conversation

brianteeman
Copy link
Contributor

@brianteeman brianteeman commented Jan 31, 2020

In a previous pull request I added code which adds a caption to every table AND indicates how the table is sorted. This is only visible to screen readers

To improve this further we should also include how the table is filtered.

This DRAFT pr which I hope the javascript gurus can help with adds the filtered information.

I have only enabled this for the table of contacts at this time while in draft

Currently the output is like this

<caption id="captionTable" class="sr-only">
Table of Contacts
<span id="orderedBy">Sorted by:Title - descending</span>
<span id="filteredBy">Filtered by:Public3</span>
</caption>

As you can see the filtered by is displaying just the selected value of the selected filters, which is kind of ok when its text but is meaningless when it is a number.

Ideally it would say
Filtered by: Access level = Public, Max Levels =3

(as this is javascript it will require eithernpm i or node build.js --compile-js to test it)

In a previous pull request I added code which adds a caption to every table AND indicates how the table is sorted.

To improve this further we should also include how the table is filtered.

This DRAFT pr which I hope the javascript gurus can help with adds the filtered information.

I have only enabled this for the table of contacts at this time while in draft

Currently the output is like this
<caption id="captionTable" class="sr-only">
Table of Contacts
<span id="orderedBy">Sorted by:Title - descending</span>
<span id="filteredBy">Filtered by:Public3</span>
</caption>

As you can see the filtered by is displaying just the selected value of the selected filters, which is kind of ok when its text but is meaningless when it is a number.
Ideally it would say
Filtered by: Access level = Public, Max Levels =3
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Jan 31, 2020
@brianteeman brianteeman added a11y Accessibility and removed Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Jan 31, 2020
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Jan 31, 2020
@angieradtke
Copy link
Contributor

that makes sense .-)

@joomla-cms-bot joomla-cms-bot removed the a11y Accessibility label Mar 6, 2020
@brianteeman
Copy link
Contributor Author

Maybe someone can convince the accessibility team to test this

@Quy Quy added the a11y Accessibility label Mar 11, 2020
@angieradtke
Copy link
Contributor

Christiane helped Stefan. He knows what do do - still waiting for results (1 week).

@brianteeman
Copy link
Contributor Author

Closed - let the accessibility team fix it

@brianteeman brianteeman closed this Apr 7, 2020
@brianteeman brianteeman deleted the filtered_by branch April 7, 2020 16:55
wilsonge added a commit to wilsonge/joomla-cms that referenced this pull request May 8, 2020
Extend joomla#27741 to fully implement the prototype of filtering. This can be rolled out to all components in a future PR. This is mainly to help people with the javascript implementation
@wilsonge
Copy link
Contributor

wilsonge commented May 8, 2020

https://github.com/joomla/joomla-cms/compare/4.0-dev...wilsonge:feature/filtered-items-a11y?expand=1

Javascript here for you hopefully. It's enough to work - although as the labels seem weird right now the text could be improved. But hopefully that's enough for someone to run with

@brianteeman
Copy link
Contributor Author

Thanks. Will check it out over the weekend

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Accessibility Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants