Accessibility improvements

[cconard96]

Q	A
Bug fix?	no
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	-

Fixed:

• Modals from glpi_html_dialog were missing a label
• Modals from glpi_html_dialog did not capture focus (could not tab through the inputs in the dialog without first clicking inside it)
• Modals from glpi_html_dialog had the wrong header level for the title
• The button next to date/datetime inputs had no label
• Automatically generated IDs from names may break labels because the name wasn't slugified
• Planning view button to add a new calendar had no label
• Planning view filter color inputs had no label
• Massive action checkboxes in search results had no label
• Search criteria value input had no label
• List limit control was not properly labeled

New discovered issues that haven't been addressed yet:

• Flatpickr breaks association with labels when altInput is true because they make the original input hidden and create a new one. The label becomes associated with the hidden input rather than the visible one.
• There are cases where we use Popper tooltips (qtip) in place of the title attribute, and those tooltips are not considered labels for the elements they are linked to.
• Menu entries the extend CommonDropdown all have the same accesskey/shortcut by default
• Fancytree has no label for the expand/collapse 'buttons'
• Fancytree adds an empty table header
• Search results export button triggers both a tooltip and a dropdown menu and it isn't considered correctly labeled. I thought adding 'none' as the role to the wrapper element which triggers the tooltip would fix the issue, but it doesn't. Manually initializing the tooltip on the button itself doesn't work either. It seems like only one plugin can be initialized on an element at once.

Possible solution notes:

• The Fancytree issues may be resolved if the Vue replacement (Replace Fancytree with Vue component in Entity Selector #16617) can get stabilized and merged.
• Maybe since GLPI has so many menu entries, it is best to not have these shortcuts except maybe for the simplified interface or only the most common entries (Nothing at the CommonDropdown level).
• Even though qtip tooltips look better, they probably shouldn't be used as a replacement for titles/labels.

[trasher]

LGTM from a code point of view (I did not test "visually"), but current e2e tests are failing.

[cconard96]

Not sure why there are color contrast failures. The errors are not present when run locally and the screenshot from the CI failure looks OK too.

[cconard96]

Seems to have been issues with animations. Locally, I have animations disabled in my OS settings (browser therefore sets prefers-reduced-motion) because I hate useless animations. In the CI, all animations would be used and it seems that the color contrast check is being done in the middle of an animation/transition (like a fade in). I added an option to allow disabling animations. Most people that have vision issues will also have non-essential animations disabled, so it shouldn't be a big deal to disable them when needed in accessibility checks.

[AdrienClairembault]

Almost all the selectors in the new e2e tests of this PR seems extremely brittle as they are css classes based.

This kind of selectors break easily and will make maintaining the tests much harder in the long run.

IMO we need to use findByRole selectors in 99% of the cases, with a fallback to less ideal selectors like ids when there is no other possibilities (and css classes as the very last resort).

The findByRole selectors also indirectly improve accessibility, which would be ideal given the title of this PR ;)

[cedric-anne]

Almost all the selectors in the new e2e tests of this PR seems extremely brittle as they are css classes based.

This kind of selectors break easily and will make maintaining the tests much harder in the long run.

IMO we need to use findByRole selectors in 99% of the cases, with a fallback to less ideal selectors like ids when there is no other possibilities (and css classes as the very last resort).

The findByRole selectors also indirectly improve accessibility, which would be ideal given the title of this PR ;)

IMHO, we should distinct 2 cases here:

• the selectors used for the click operations;
• the selectors used to target a container that have to be checked for its accessibility compliance.

The click operations should indeed be done on elements retrieved by a findByRole or equivalent method, but I guess that most of the "containers" can only be targetted by CSS selectors.

[AdrienClairembault]

the selectors used to target a container that have to be checked for its accessibility complianc

For container your have findByRole('region') that should cover it (but open to suggestions if there is a better role available).

[cedric-anne]

the selectors used to target a container that have to be checked for its accessibility complianc

For container your have findByRole('region') that should cover it (but open to suggestions if there is a better role available).

Maybe using <section> with a aria-label could indeed help to have better targetting.

I do not know if it is a valid option, but anyways, having tests based on CSS selectors in indeed not the best option.

@cconard96 What is your opinion on this?

[cconard96]

• #planning_container - No. It shouldn't need any special role or label. The label should be on the tab panel itself. This is something that needs done still.
• .modal.show - Could select by dialog role as long as it waits for the modal to be completely shown and we are sure that multiple dialogs won't be open.
• aside.navbar.sidebar - It should have a menu role. Maybe a "Main menu" label for it makes sense. We can have multiple menus, so I don't think you can select just by the role.
• header.navbar - No. The implicit role is correct here and a label for the entire header doesn't make sense. Maybe we can assume there will ever only be one banner role element on the page (excluding iframe content).
• .navbar-nav.user-menu - Maybe it needs a label along with a specific role. Its just annoying to target because there is a hidden copy of the menu added to the DOM. Other attempts to select the visible user menu didn't seem to work. The template needs reworked to stop doing that if possible.
• div.search_page - Maybe this could be a section with a "Search" label. There are child elements of this that could also be sections (search controls, search results).
• button.show-search-filters - No. Cannot use the label because it changes based on the current search.
• button.show-search-sorts - No. Cannot use the label because it changes.

[cedric-anne]

I guess the selectors used in tests could be improved, but having accessibility tests is still a good point and I guess we could merge this PR and improve the selectors later, if tests fails due to a DOM structure/content refactoring.

@cconard96 Could you please rebase this PR to fix conflicts ?

[cedric-anne]

Tests are failing.