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

Refs #4770 - Accessibility acceptance tests for blocks #6329

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

tedw87
Copy link
Contributor

@tedw87 tedw87 commented Sep 25, 2024

Update and fix #4781

Copy link

netlify bot commented Sep 25, 2024

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit bc07350
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/6707b170f63b5b00089ced15

@ichim-david ichim-david added the 99 tag: UX Accessibility Accessibility issues label Sep 25, 2024
@tedw87 tedw87 self-assigned this Sep 25, 2024
@ichim-david
Copy link
Member

@tedw87 be specific when tests are done and they pass within the changelog of what blocks you've added a11y tests for
if you are done with adding the tests.

@pnicolli pnicolli requested a review from a team September 29, 2024 08:21
Copy link
Member

@ichim-david ichim-david left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plone/volto-accessibility @plone/volto-team I am ok with this work as it's an addition to what we already had.
I have some questions regarding the placement of these tests, if we shouldn't change a bit the organization of these tests.
For instance for Volto "content types" I've asked Ana to place the tests within a11y.js from the content folder in this pull request https://github.com/plone/volto/pull/6339/files.
Similarly to me, it would make more sense to have the block a11y testing within this folder https://github.com/plone/volto/tree/main/packages/volto/cypress/tests/core/blocks (blocks-a11y.js).

I would keep the basic a11y.js for the routes and content views such as the contact-form, frontpage, and whatever else we want to test.

Should we be so neat in the test organisation or do you prefer a single long file? Personally, I am in favor of splitting each concern making it also easier to navigate but I need to know also the opinion of our community.

@pnicolli
Copy link
Contributor

I had not thought about it but now that you mention it, I like the idea of separating concerns and splitting files.

@JeffersonBledsoe
Copy link
Member

Agreed on splitting things out to make it easier to debug test failures and review component behaviour, as long term I would like to see more a11y tests that aren't just "Run axe and assume it's accessible if it passes".

I would keep the basic a11y.js for the routes and content views such as the contact-form, frontpage, and whatever else we want to test.

Lets rename a11y.js to something a little clearer to avoid this file being bloated in the future? "Automated route accessibility" or something?

@sneridagh
Copy link
Member

@ichim-david @JeffersonBledsoe @pnicolli in all our projects, we have a specific folder for that, then inside a file per component/view/block etc, using common sense. More than one level nesting is not needed, I guess.

@tedw87 maybe you can start with that one in here? We could even split the actual GHA tests to just test the A11y separately. I can take care of that in another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
99 tag: UX Accessibility Accessibility issues
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

6 participants