Update accessible roles for v5 #1031

clarkgunn · 2023-09-14T18:38:57Z

Description

Adjusts roles and attributes to better align with the WCAG Aria Authoring Practices Guide https://www.w3.org/WAI/ARIA/apg/patterns/carousel

New tabbed prop determines whether roles should follow the Basic Carousel or Tabbed Carousel requirements.
New landmark prop determines whether role should be a region landmark or have role group. Role and label have been moved from the slider frame to the parent carousel element so controls are contained within the label and role. Landmark regions should be intentional, so the default value is set to false.
New carouselId prop gives the carousel frame an id attribute which can be uniquely targeted by controls where there might be multiple carousels on a page. This could be replaced internally with useId with React 18.
Default frameAriaLabel has been changed to a slightly more useful 'Slider', the role description of 'carousel' will already be announced
Default pagination dots have been given the role tab, and aria-controls targeting their slide.
Pagination dots parent has been given the role tablist. As tabs need to be the immediate child of tablist, and since tablist already has list semantics, ul and li have been removed. This should resolve Attributes do not match their roles on carousel dots #1001 and the issue raised in Testing accessibility with jest-axe #803 regarding aria-selected.
When tabbed is true, slides will have the role tabpanel indicating they are the target of the tab, otherwise they will have the role group and a description of slide.
When tabbed is false, default pagination dots will not be rendered.
Default next and previous buttons now target the slider frame with aria-controls.

Adds dev dependencies: jest-axe, @types/jest-axe

Fixes #1001

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

These changes will impact the roles and attributes when default props are used. I believe this will improve accessibility but may be considered a breaking change.

How Has This Been Tested?

New jest and cypress tests were added. Existing cypress tests needed wait logic to pass, akin to this change 1f7f020

Cypress and Jest tests and Lighthouse accessibility report should all pass. Attributes and roles can be verified in devtools.

Checklist

My code follows the style guidelines of this project (I have run pnpm run lint)
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes (I have run pnpm run test:ci-with-server/pnpm run test)
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have recorded any user-facing fixes or changes with pnpm changeset.
My changes generate no new warnings
Any dependent changes have been merged and published in downstream modules

…s/carousel

…le will be announced.

…istinguish, update tests.

vercel · 2023-09-14T18:39:01Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
nuka-carousel-website	❌ Failed (Inspect)			Sep 14, 2023 6:58pm

clarkgunn · 2023-09-14T19:04:59Z

packages/nuka/src/carousel.test.tsx

    });

-    const carouselFrame = screen.getByRole('region');
+    const sliderFrame = screen.getByTestId(`${carouselId}-slider-frame`);


The region role is now applied to the whole carousel, this ensures the same slider frame element is being selected.

clarkgunn · 2023-09-14T19:06:45Z

packages/nuka/src/types.ts

+  /**
+   * Unique id attribute for the carousel which may be referenced by aria attributes.
+   */
+  carouselId: string;


I went with carouseld to disambiguate between id attributes and props on other elements.

clarkgunn · 2023-09-14T19:08:03Z

packages/nuka/src/types.ts

+  /**
+   * Whether tab pagination is used to set appropriate roles for slides.
+   */
+  tabbed: boolean;


tabbed doesn't exactly align with the "pagination" and "dots" verbiage currently used, but it aligns with WCAG terminology.

carloskelly13 · 2023-09-14T20:12:13Z

packages/nuka/src/default-controls.tsx

+    <div
+      className={pagingDotsContainerClassName}
+      style={listStyles}
+      aria-label="Choose slide to display."


Should we let this be customizable with this string as default? (maybe this is more for an eventual port of this for v6 and not this specific use case)

carloskelly13

Thank you for these improvements and getting this more inline. This will be also great to forward-port to v6 as well.

I'm not worried about the failed Vercel deployment on v5 since those docs are decommissioned, nor the Cypress tests on v5 since we knew those were broken in CI.

* Migrate accessibility roles from v5 to v6 * Fix storybook configuration * Fix existing Cypress tests * Add query param support to demo used by cypress * Fix serializing params for tests. * Update demos.tsx * Fix auto install peers * Update cypress.config.ts * Update carousel.test.tsx * Add axe test violation checker for a carousel with control callbacks * Add changeset * Switch to major bump for new a11y props.

Clark Gunn added 12 commits September 13, 2023 20:06

Update roles and attributes to align with w3.org/WAI/ARIA/apg/pattern…

4535545

…s/carousel

skip cypress tests already broken in tag/5.5.1

860efd1

commit lock file

a564f3a

add minor version changeset

8067170

Adjust selectors for jest tests, remove unneeded data-testid.

b5e1fd6

Add jest-axe and test assertions

0853b9e

Add jest-axe types

1ee5989

Add label to tablist and remove 'bullet' from tab label as the tab ro…

17059ed

…le will be announced.

Add wait and additional assertions to cypress test.

f212279

Move region/group role to containing div use new prop "landmark" to d…

a28fef8

…istinguish, update tests.

update cypress test

ba758af

target slider frame with next prev button aria-controls.

82f34e6

update changeset

eaaf96e

clarkgunn marked this pull request as draft September 14, 2023 18:50

Don't render default pagination dots if tabbed is false.

b884b7b

vercel bot had a problem deploying to Preview September 14, 2023 18:58 Failure

clarkgunn commented Sep 14, 2023

View reviewed changes

clarkgunn marked this pull request as ready for review September 14, 2023 19:12

clarkgunn requested review from carloskelly13 and gksander September 14, 2023 19:12

carloskelly13 changed the base branch from main to 6 September 14, 2023 19:55

carloskelly13 changed the base branch from 6 to 5 September 14, 2023 19:56

carloskelly13 reviewed Sep 14, 2023

View reviewed changes

carloskelly13 approved these changes Sep 14, 2023

View reviewed changes

clarkgunn merged commit 7945466 into 5 Sep 20, 2023
1 of 2 checks passed

clarkgunn deleted the task/roles branch September 20, 2023 23:40

github-actions bot mentioned this pull request Sep 21, 2023

Version Packages #1033

Merged

carloskelly13 mentioned this pull request Nov 1, 2023

Migrate #1031 to the v6 codebase #1036

Merged

carloskelly13 mentioned this pull request Nov 15, 2023

Add v7 docs, switch to React 18’s useId for carousel identifier. #1039

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update accessible roles for v5 #1031

Update accessible roles for v5 #1031

clarkgunn commented Sep 14, 2023 •

edited

vercel bot commented Sep 14, 2023 •

edited

clarkgunn Sep 14, 2023 •

edited

clarkgunn Sep 14, 2023

clarkgunn Sep 14, 2023

carloskelly13 Sep 14, 2023

carloskelly13 left a comment •

edited

Update accessible roles for v5 #1031

Update accessible roles for v5 #1031

Conversation

clarkgunn commented Sep 14, 2023 • edited

Description

Type of Change

How Has This Been Tested?

Checklist

vercel bot commented Sep 14, 2023 • edited

clarkgunn Sep 14, 2023 • edited

Choose a reason for hiding this comment

clarkgunn Sep 14, 2023

Choose a reason for hiding this comment

clarkgunn Sep 14, 2023

Choose a reason for hiding this comment

carloskelly13 Sep 14, 2023

Choose a reason for hiding this comment

carloskelly13 left a comment • edited

Choose a reason for hiding this comment

clarkgunn commented Sep 14, 2023 •

edited

vercel bot commented Sep 14, 2023 •

edited

clarkgunn Sep 14, 2023 •

edited

carloskelly13 left a comment •

edited