Check Accessibility in Cypress with Axe

[cconard96]

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

Testing using cypress-axe to automatically scan GLPI pages for accessibility issues.

Example of accessibility issue report:

[cedric-anne]

Seems OK to me, but I have two remarks.

1. It would have been better to rely on a popular library for color manipulation functions, but I did not find any library that would match our needs. Indeed, there are many color manipulation libraries, but I did not find any maintained and GPLv3 compatible library that just propose basic features like we have in the Color js class. Therefore, libraries I found are not as light as we could expect. For instance, the minified dist of TinyColor weights 15KB.

2. As far as I understand, it will be necessary to call the injectAndCheckA11y() method manually in all of our tests cases, to check only specific elements. I guess it is preferable for many reasons (checking the whole page too often would probably be too long, there are many issues to fix before all of our pages could be validated, ...), but it means that we have to take care that, each test, will call this method when it is necessary. It is probable that we will miss this point often. I think that it could be usefull to provide a quick guide to explain the way our E2E tests should be made.

[cedric-anne]

The foreground color seems only necessary when the initials are displayed (when the thumbnail is null).

Suggested change

	style="{% if user_thumbnail is not null %}background-image: url({{ user_thumbnail }}); background-color: inherit; {% else %} background-color: {{ bg_color }};{% endif %}
	color: {{ fg_color }}">
	style="{% if user_thumbnail is not null %}background-image: url({{ user_thumbnail }}); background-color: inherit; {% else %} background-color: {{ bg_color }}; color: {{ fg_color }};{% endif %}">

[orthagh]

the minified dist of TinyColor weights 15KB.

It's nothing and the changes permit to:

• remove code (probably) copypasted from the web without clear licensing
• can be reused
• reduce the size of the mess of common.js

So I'm pretty in favor
Tasks to do:

• check license compatibility with GLPI's GPLv3
• verify that another small library exists with more recent commits (but in the case of the feature set, it's not that important)

[cedric-anne]

the minified dist of TinyColor weights 15KB.

It's nothing and the changes permit to:

• remove code (probably) copypasted from the web without clear licensing
• can be reused
• reduce the size of the mess of common.js

So I'm pretty in favor Tasks to do:

• check license compatibility with GLPI's GPLv3
• verify that another small library exists with more recent commits (but in the case of the feature set, it's not that important)

I did not find any smaller alternative that is as popular and maintained as TinyColor. Also, TinyColor has no dependencies, meaning that it will never be subject to issues like usage of an unmaintained/unsecure/incompatible dependency.

So OK to use TinyColor.

@cconard96 Do you prefer to do it in the current PR, or to do it in another PR ?

[cconard96]

For the color functions:
The only big addition I think is the new getContrastingColors function which I ended up using a local copy rather than the new Color class. Typically, we relied on just changing how bright or dark a color is to get a foreground color for it, but that doesn't always change the contrast significantly. The saturation of the color also seems to play a role. The new Color class doesn't need to be included in this PR, but I don't know if TinyColor offers any similar function for automatically picking contrasting colors. They do have functions to check the contrast though.

For the subject of the PR:

As far as I understand, it will be necessary to call the injectAndCheckA11y() method manually in all of our tests cases, to check only specific elements.

It could be used to test an entire page, but the goal is to test specific things. Since we finally moved to Twig for most things and reuse components, there isn't really a need to test the entire page I think.

It is probable that we will miss this point often. I think that it could be usefull to provide a quick guide to explain the way our E2E tests should be made.

I agree for some documentation.

In general, the goal was to add at least some accessibility checks, start making fixes as we find issues, and keep a list of outstanding issues we know about but haven't fixed yet. This, alongside using the selectors from testing-library which were already added should be a significant improvement to most of GLPI as we add new E2E tests. Testing the entirety of GLPI at once would be very slow and I'm not sure there is a 100% perfect way for us to do that even if we did have a sitemap that we could use alongside a tool like Google's Lighthouse.

[cedric-anne]

The only big addition I think is the new getContrastingColors function which I ended up using a local copy rather than the new Color class. Typically, we relied on just changing how bright or dark a color is to get a foreground color for it, but that doesn't always change the contrast significantly. The saturation of the color also seems to play a role. The new Color class doesn't need to be included in this PR, but I don't know if TinyColor offers any similar function for automatically picking contrasting colors. They do have functions to check the contrast though.

Maybe it can be done using the mostReadable() function on the result of the monochromatic function, with a fallback to black or white if none matches (includeFallbackColors option).