? ? Pending

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
3 Jul 2018

The modules on the control panel contain multi-dimensional data and so should be tables and not lists. By doing this we are also able to make them accessible.

The actual markup (the classes) will need to be reviewed before merging by someone other than me.

Accessibility improvements

Each column has a table header and each table header has a scope
The cell for the item has a scope of row (it therefore has to be a th not a td)
Adding the scope ensures that screenreaders understand what the numbers are eg 1024 hits for article link "similar tags"
If the site owner chooses not to display the module title then a sr-only caption is created - this is essential for a11y

Design improvements

By using tables etc then we avoid the crazy layout we can get now

Notes

the logout button in mod_logged is an <a href= role=button> - this could be changed to a real button if people feel strongly
checked out is not changed in this pr - that will need to be done in separate pr as it effects a library etc

Before

chrome_2018-07-05_17-11-35

After

chrome_2018-07-05_17-10-25

avatar brianteeman brianteeman - open - 3 Jul 2018
avatar brianteeman brianteeman - change - 3 Jul 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 3 Jul 2018
Category Modules Administration
avatar brianteeman brianteeman - change - 3 Jul 2018
Labels Added: ?
avatar SharkyKZ
SharkyKZ - comment - 3 Jul 2018

Any particular reason for using module title as ID? ID must be unique. You can have multiple modules with the same title on the same page which means duplicate IDs.

If the site owner choses not to display the module title then a sr-only caption is created

I feel like this should be done at module chrome level rather than in module layout.

avatar brianteeman
brianteeman - comment - 3 Jul 2018

Any particular reason for using module title as ID? ID must be unique. You can have multiple modules with the same title on the same page which means duplicate IDs.

No reason - this is just a proof of concept PR. can easily be changed to moduletitle and id

If the site owner choses not to display the module title then a sr-only caption is created

I feel like this should be done at module chrome level rather than in module layout.

That would require a new module chrome to be created and for this proof of concept I didnt want to do that if it can be avoided - and it can

avatar chmst
chmst - comment - 4 Jul 2018

Looks good for me, but I think the checked-out icon and the edit icon should have additional sr-only information.

avatar brianteeman
brianteeman - comment - 4 Jul 2018

As the Edit icon is purely decorative I don;t agree with it needing any sr-only text - it is inside the same link as the href etc.

I will look into the checked out one - that does need some improvement. Thanks for testing

avatar brianteeman
brianteeman - comment - 4 Jul 2018

The checked out stuff probably needs a library change :( So lets define what it should be here and then it is correct everywhere

avatar brianteeman
brianteeman - comment - 5 Jul 2018

Is there really only one person in the accessibility team who is going to take the time to check this out and comment. It might be just one module here but the principles, if accepted, will be applied in many modules and many admin views.

avatar zwiastunsw zwiastunsw - test_item - 5 Jul 2018 - Tested successfully
avatar zwiastunsw
zwiastunsw - comment - 5 Jul 2018

I have tested this item successfully on ea32f6d

I testing with NVDA. Ideally


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

avatar zwiastunsw
zwiastunsw - comment - 5 Jul 2018

For the label for the Edit icon. In the item list view, e.g. Article Manager, the icon is hidden from screen readers. But each item has a tooltip with "Edit...." text. I think it should be the same consistently.

avatar brianteeman
brianteeman - comment - 5 Jul 2018

The tooltip really isnt needed anywhere

avatar zwiastunsw
zwiastunsw - comment - 5 Jul 2018

I do not insist. I agree that it is not necessary.

avatar SharkyKZ
SharkyKZ - comment - 5 Jul 2018

Thinking about it, is it really a good idea to show title when it's hidden? It might be unreadable or otherwise should not be displayed to the users. E.g. I can see administrators leaving titles derived from module name (mod_something).

Also, title needs to be escaped in ID.

avatar brianteeman
brianteeman - comment - 5 Jul 2018

Thinking about it, is it really a good idea to show title when it's hidden?

It is not shown

avatar SharkyKZ
SharkyKZ - comment - 5 Jul 2018

It's shown to screen reader users.

avatar brianteeman
brianteeman - comment - 5 Jul 2018

Added mod-latest

next up mod logged

avatar brianteeman
brianteeman - comment - 5 Jul 2018

It's shown to screen reader users.

As it should be

avatar chmst
chmst - comment - 5 Jul 2018

So we have only the title / tooltip for the checked_out button. This can be left until we have a general solution for this.

@brianteeman - I did not understand your remark "next up mod logged"? Do you mean that you adapt the logged_in module?

avatar brianteeman
brianteeman - comment - 5 Jul 2018
  1. Yes I plan to make the same changes to mod logged

  2. The checkout needs to be looked at as a separate issue as it is a library

avatar zwiastunsw
zwiastunsw - comment - 5 Jul 2018

The same changes are also needed in the module Recently Added Articles

avatar brianteeman
brianteeman - comment - 5 Jul 2018

Already done here

avatar brianteeman brianteeman - change - 5 Jul 2018
Title
[4.0] [a11y] Mod-popular - proof of concept
[4.0] [a11y] Mod-popular, mod_logged, mod_latest
avatar brianteeman brianteeman - edited - 5 Jul 2018
avatar brianteeman brianteeman - change - 5 Jul 2018
The description was changed
avatar brianteeman brianteeman - edited - 5 Jul 2018
avatar brianteeman
brianteeman - comment - 5 Jul 2018

I have updated the description - this pr is no longer a proof of concept and should be tested ready for merging

It includes the three modules that are multi-dimensional and therefore should not be lists

4a5e0bc 5 Jul 2018 avatar brianteeman cs
avatar zwiastunsw
zwiastunsw - comment - 5 Jul 2018

Where did you read that the role=button for the link is correct? Not applicable. Link is a link, button is a button. Why change the native semantics? Why not use a button when it is semantically a button?

https://www.w3.org/TR/using-aria/. See: 2.1 First Rule of ARIA Use, 2.2 Second Rule of ARIA Use

avatar chmst
chmst - comment - 5 Jul 2018

I think that we should make a new issue for this point when the layout is ready.
It is not as simple here.

avatar zwiastunsw
zwiastunsw - comment - 5 Jul 2018

Let @brianteeman decide.

avatar zwiastunsw
zwiastunsw - comment - 5 Jul 2018

This text also says: "Note: Where possible, it is recommended to use native HTML buttons [...] rather than the button role, as native HTML buttons are more widely supported by older user agents and assistive technology. "
Using role=button for links is bad practice.

avatar brianteeman
brianteeman - comment - 5 Jul 2018

yes but it doesnt say it is wrong

avatar zwiastunsw
zwiastunsw - comment - 5 Jul 2018

Hmmm. The issue is one of approach. Bad practice is bad practice. The rules are there to apply. You repeat this many times.

avatar brianteeman
brianteeman - comment - 5 Jul 2018

do what you want. but if i hadnt written it then we would still be waiting for xmas for anything from you

avatar chmst
chmst - comment - 5 Jul 2018

I have tested this item successfully.

My2Cent: The < a > instead of < button >can be accepted here even if it is not best practice. It is not for public and it is not used very often. And replacing it needs programming.

avatar zwiastunsw
zwiastunsw - comment - 5 Jul 2018

Let's do as @chmst proposes


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

avatar zwiastunsw
zwiastunsw - comment - 5 Jul 2018

I have tested this item successfully on 27958ee


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

avatar zwiastunsw zwiastunsw - test_item - 5 Jul 2018 - Tested successfully
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 6 Jul 2018

@chmst can you please mark your Test as successfully?

avatar chmst chmst - test_item - 6 Jul 2018 - Tested successfully
avatar chmst
chmst - comment - 6 Jul 2018

I have tested this item successfully on 27958ee


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

avatar franz-wohlkoenig franz-wohlkoenig - change - 6 Jul 2018
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 6 Jul 2018

Ready to Commit after two successful tests.

avatar wilsonge
wilsonge - comment - 11 Jul 2018

Sorry with the admin template being merged this has conflicted would you mind syncing this up and then just one tester (can be someone who tested before) to just review after that.

avatar brianteeman brianteeman - change - 11 Jul 2018
Labels Added: ?
avatar brianteeman
brianteeman - comment - 11 Jul 2018

conflicts resolved

avatar zwiastunsw zwiastunsw - test_item - 12 Jul 2018 - Tested successfully
avatar zwiastunsw
zwiastunsw - comment - 12 Jul 2018

I have tested this item successfully on 01ab023


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

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 12 Jul 2018

@chmst can you please retest?

avatar chmst chmst - test_item - 12 Jul 2018 - Tested successfully
avatar chmst
chmst - comment - 12 Jul 2018

I have tested this item successfully on 01ab023


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

avatar zwiastunsw
zwiastunsw - comment - 12 Jul 2018

I repeated the test with the new template. Everything is OK. Skipping the question of the button. Will be reported in the new PR.

avatar laoneo laoneo - change - 17 Jul 2018
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2018-07-17 06:24:05
Closed_By laoneo
avatar laoneo laoneo - close - 17 Jul 2018
avatar laoneo laoneo - merge - 17 Jul 2018
avatar laoneo
laoneo - comment - 17 Jul 2018

Thanks!

avatar brianteeman
brianteeman - comment - 17 Jul 2018

Thanks

Add a Comment

Login with GitHub to post a comment