DSD-1900: ARIA landmarks guide #1806

7emansell · 2025-04-28T15:52:20Z

Fixes JIRA ticket DSD-1900

This PR does the following:

Adds guide to ARIA landmarks to Storybook documentation

How has this been tested?

Accessibility concerns or updates

Accessibility Checklist

Checked Storybook's "Accessibility" tab for color contrast and other issues.
The feature works with keyboard inputs including tabbing back and forward and pressing space, enter, arrow, and esc keys.
For hidden text or when aria-live is used, a screenreader was used to verify the text is read.
For features that involve UI updates and focusing on DOM refs, focus management was reviewed.
The feature works when the page is zoomed in to 200% and 400%.

Open Questions

Checklist:

I have updated the Storybook documentation and changelog accordingly.
I have added relevant accessibility documentation for this pull request.
All new and existing tests passed.

Front End Review:

Review the Vercel preview deployment once it is ready.

vercel · 2025-04-28T15:52:24Z

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

Name	Status	Preview	Comments	Updated (UTC)
nypl-design-system	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 6, 2025 8:09pm

EdwinGuzman · 2025-04-28T19:47:00Z

src/components/SearchBar/SearchBar.stories.tsx

+export const WithSearchElement: Story = {
+  render: () => (
+    <search>
+      <SearchBar


SearchBar still contains role="search" so this is technically not the appropriate pattern (and it won't be until v4 where the DOM is updated). I think you should replace SearchBar with separate heading, input, button, and checkbox components.

Oh true. Then maybe this should go into v4 since the original intention of this guide was to clarify how the search landmark should be used, and I'd like to keep this specific example with Searchbar

Discussed in standup– this is not gonna go into v4 so replacing the searchbar with a TextInput for now so the pattern is clear

EdwinGuzman

This is great, thanks! Just some minor comments.

src/components/AccessibilityGuide/AriaLandmarks.mdx

EdwinGuzman · 2025-04-28T20:05:52Z

src/components/AccessibilityGuide/AriaLandmarks.mdx

+
+Landmarks can be indicated with `role="landmark"` or by using the landmark's HTML element.
+
+**Some Reservoir components already define their own landmark roles, and some landmarks must be defined by the consuming app since they could wrap many components.**


This makes it seem like the components are defining roles beyond the eight native ones.

Does this update make it clearer

src/components/AccessibilityGuide/AriaLandmarks.mdx

EdwinGuzman

Minor comments but not blockers.

EdwinGuzman · 2025-04-29T21:31:20Z

src/components/SearchBar/SearchBar.stories.tsx

+  render: () => (
+    <search>
+      {/* TODO: Replace with Searchbar component. */}
+      <TextInput id={"search-input"} labelText={"Search"} />


nit: you don't need braces around string prop values.

This could also use a simple button component.

oliviawongnyc

I learned a lot from this! Some small suggestions included.

src/components/AccessibilityGuide/AriaLandmarks.mdx

oliviawongnyc · 2025-04-30T14:19:44Z

src/components/AccessibilityGuide/AriaLandmarks.mdx

+
+Elements under [Navigation](../?path=/docs/components-navigation) should be wrapped by the `navigation` landmark.
+
+### region


I wonder how to differentiate between complementary and region. Could you include something about how to think about this?

Added a note below

Great! Small typo: "content should supports or modifies" - remove extra s's.

Further, should region landmarks be nested inside the main landmark or do they typically sit outside main?

EdwinGuzman

Approving latest update, thanks for swapping the searchbar out.

bigfishdesign13

Great information. I just have some suggestions.

bigfishdesign13 · 2025-05-06T20:35:17Z

src/components/AccessibilityGuide/AriaLandmarks.mdx

+| region        | `<section>`  |
+| search        | `<search>`   |
+
+Landmarks can be indicated with `role="landmark"` or by using the landmark's HTML element– both methods mean the same thing in the document.


If you can get away with it, replace the dash with — (em dash). If that does not render properly, use —. Also, remove the space after the em dash character.

bigfishdesign13 · 2025-05-06T20:43:52Z

src/components/AccessibilityGuide/AriaLandmarks.mdx

+### banner
+
+The banner landmark is an area that has global, rather than page-specific, information or actionable elements.
+For example, a website's logo and primary navigation would be expected child elements of a `banner` landmark. There should only be a single `banner` landmark per interface.


It was my understanding, and it is also stated on w3schools.com, that it is ok to have multiple <header> elements on one page. For example, each significant section on a page may have a <header> element.

I'd follow MDN's doc and not w3schools (they have a bad reputation). That said, you can have more than one header/banner role on a page but if so, it should have a unique label. So that should be updated here (and for contentinfo)

bigfishdesign13 · 2025-05-06T20:48:17Z

src/components/AccessibilityGuide/AriaLandmarks.mdx

+
+The `complementary` landmark indicates an area that contains additional information that "complements" the main content. Though complementary, the content it represents should also be understandable on its own.
+
+The `Banner`, `StatusBadge`, `Notification` , `Hero`, and `FeaturedContent` components could all be `complementary` landmarks.


Out of the box, Banner and Notification are <aside> elements, so they are inherently complementary landmarks. Would consuming apps need to explicitly add the role to the other components that are mentioned?

bigfishdesign13 · 2025-05-06T20:50:44Z

src/components/AccessibilityGuide/AriaLandmarks.mdx

+
+### contentinfo
+
+`contentinfo` is another globally repeating landmark, like `banner`, with information about the parent document. If `banner` is usually the site's header, `contentinfo` is usually the footer. There should only be a single `contentinfo` landmark per interface.


Same thing here...it is generally accepted that more than one <footer> element can be added to a page.

bigfishdesign13 · 2025-05-06T20:54:05Z

src/components/AccessibilityGuide/AriaLandmarks.mdx

+<Banner
+  content={
+    <>
+      <strong>IMPORTANT:</strong> Use the search landmark instead of the form
+      landmark when the form is used for search functionality.
+    </>
+  }
+  type="warning"
+/>


bigfishdesign13 · 2025-05-06T20:56:02Z

src/components/AccessibilityGuide/AriaLandmarks.mdx

+
+There should only be a single `main` landmark per interface. Since `main` represents the primary content of a page, it is the most important destination for [skip links](../?path=/docs/accessibility-guide-skip-navigation--docs), bypassing recurring elements like the header or navigation.
+
+The `Template` [component](../?path=/docs/components-page-layout-template--docs) renders the `main` landmark (and the `header` and `footer`).


This will need to be updated for v4, as the header and footer regions are not part of the updated Template component.

bigfishdesign13 · 2025-05-06T20:58:23Z

src/components/AccessibilityGuide/AriaLandmarks.mdx

+### navigation
+
+The `navigation` landmark identifies groups (e.g. lists) of links that are intended to be used for website or page content navigation.
+There can be multiple `navigation`s on a page, but if a `navigation` landmark has the same set of links as another `navigation` landmark on the page (think mobile versus desktop), use the same HTML `label` attribute for both.


Nit: change "multiple navigations" to "multiple navigation elements"

bigfishdesign13 · 2025-05-06T21:04:46Z

src/components/AccessibilityGuide/AriaLandmarks.mdx

+The `navigation` landmark identifies groups (e.g. lists) of links that are intended to be used for website or page content navigation.
+There can be multiple `navigation`s on a page, but if a `navigation` landmark has the same set of links as another `navigation` landmark on the page (think mobile versus desktop), use the same HTML `label` attribute for both.
+
+Elements under [Navigation](../?path=/docs/components-navigation) should be wrapped by the `navigation` landmark.


Only two components in our Navigation category do not have a <nav> element baked in. In fact, one of the two that doe snot have it should be updated to have it added. So that would leave Link as the only component that needs to potentially be wrapped in a navigation landmark, and even that would depend on how it was being used.

This is a long winded way of saying this comment should be reworded.

bigfishdesign13 · 2025-05-06T21:09:08Z

src/components/AccessibilityGuide/AriaLandmarks.mdx

+
+Elements under [Navigation](../?path=/docs/components-navigation) should be wrapped by the `navigation` landmark.
+
+### region


Further, should region landmarks be nested inside the main landmark or do they typically sit outside main?

bigfishdesign13 · 2025-05-06T21:15:34Z

src/components/AccessibilityGuide/AriaLandmarks.mdx

+
+The `search` landmark contains the page's search and filter functionality, which will likely include form controls (such as an `input type="search"`).
+
+Inspect below to see `<search>` in the DOM– notice that it wraps the `Heading`, the `TextInput`, the `Button`, and the `Checkbox`, not just the searchbar:


Replace the dash with an em dash and remove the trailing space.

If we are going to ask users to look for the DS components, let's tell them to click on the Show code button in the lower right corner. Conversely, if we are going to ask them to inspect the DOM, then let's ask them to look for the native HTML elements.

7emansell added 3 commits April 27, 2025 22:02

start of landmarks guide

4e63352

First draft

984c64d

changelog

421db89

7emansell requested review from bigfishdesign13, EdwinGuzman and oliviawongnyc April 28, 2025 15:52

Adding sources

b2d2c0b

vercel bot deployed to Preview April 28, 2025 15:59 View deployment

EdwinGuzman reviewed Apr 28, 2025

View reviewed changes

EdwinGuzman added the Needs Review Pull requests that are ready for peer review. label Apr 28, 2025

7emansell added 2 commits April 28, 2025 17:36

edwin updates

06d91a4

semantics

9b5c88b

vercel bot deployed to Preview April 28, 2025 21:42 View deployment

No more searchbar in story

1dc637f

vercel bot deployed to Preview April 29, 2025 17:27 View deployment

EdwinGuzman approved these changes Apr 29, 2025

View reviewed changes

Correcting story

f1ca604

vercel bot deployed to Preview April 30, 2025 14:12 View deployment

oliviawongnyc reviewed Apr 30, 2025

View reviewed changes

7emansell mentioned this pull request May 5, 2025

DSD-1901: Remove role="search" from Searchbar #1807

Open

9 tasks

Olivia feedback

1aef8e0

7emansell requested a review from oliviawongnyc May 5, 2025 15:58

vercel bot deployed to Preview May 5, 2025 16:01 View deployment

EdwinGuzman approved these changes May 5, 2025

View reviewed changes

oliviawongnyc approved these changes May 6, 2025

View reviewed changes

typoo

3bf5d92

vercel bot deployed to Preview May 6, 2025 20:09 View deployment

bigfishdesign13 requested changes May 6, 2025

View reviewed changes

bigfishdesign13 added Changes Requested Pull requests where changes have been requested in peer review. and removed Needs Review Pull requests that are ready for peer review. labels May 6, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DSD-1900: ARIA landmarks guide #1806

DSD-1900: ARIA landmarks guide #1806

7emansell commented Apr 28, 2025 •

edited by jira bot

Loading

vercel bot commented Apr 28, 2025 •

edited

Loading

EdwinGuzman Apr 28, 2025 •

edited

Loading

7emansell Apr 28, 2025

7emansell Apr 29, 2025

EdwinGuzman left a comment

EdwinGuzman Apr 28, 2025

7emansell Apr 28, 2025

EdwinGuzman left a comment

EdwinGuzman Apr 29, 2025

EdwinGuzman Apr 29, 2025

oliviawongnyc left a comment

oliviawongnyc Apr 30, 2025

7emansell May 5, 2025

oliviawongnyc May 6, 2025

bigfishdesign13 May 6, 2025

EdwinGuzman left a comment

bigfishdesign13 left a comment

bigfishdesign13 May 6, 2025

bigfishdesign13 May 6, 2025

EdwinGuzman May 7, 2025

bigfishdesign13 May 6, 2025

bigfishdesign13 May 6, 2025

bigfishdesign13 May 6, 2025

bigfishdesign13 May 6, 2025

bigfishdesign13 May 6, 2025

bigfishdesign13 May 6, 2025

bigfishdesign13 May 6, 2025

bigfishdesign13 May 6, 2025


		Landmarks can be indicated with `role="landmark"` or by using the landmark's HTML element.

		Some Reservoir components already define their own landmark roles, and some landmarks must be defined by the consuming app since they could wrap many components.


		Elements under [Navigation](../?path=/docs/components-navigation) should be wrapped by the `navigation` landmark.

		### region


		The `complementary` landmark indicates an area that contains additional information that "complements" the main content. Though complementary, the content it represents should also be understandable on its own.

		The `Banner`, `StatusBadge`, `Notification` , `Hero`, and `FeaturedContent` components could all be `complementary` landmarks.


		### contentinfo

		`contentinfo` is another globally repeating landmark, like `banner`, with information about the parent document. If `banner` is usually the site's header, `contentinfo` is usually the footer. There should only be a single `contentinfo` landmark per interface.


		There should only be a single `main` landmark per interface. Since `main` represents the primary content of a page, it is the most important destination for [skip links](../?path=/docs/accessibility-guide-skip-navigation--docs), bypassing recurring elements like the header or navigation.

		The `Template` [component](../?path=/docs/components-page-layout-template--docs) renders the `main` landmark (and the `header` and `footer`).


		The `search` landmark contains the page's search and filter functionality, which will likely include form controls (such as an `input type="search"`).

		Inspect below to see `<search>` in the DOM– notice that it wraps the `Heading`, the `TextInput`, the `Button`, and the `Checkbox`, not just the searchbar:

DSD-1900: ARIA landmarks guide #1806

Are you sure you want to change the base?

DSD-1900: ARIA landmarks guide #1806

Conversation

7emansell commented Apr 28, 2025 • edited by jira bot Loading

This PR does the following:

How has this been tested?

Accessibility concerns or updates

Accessibility Checklist

Open Questions

Checklist:

Front End Review:

vercel bot commented Apr 28, 2025 • edited Loading

EdwinGuzman Apr 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EdwinGuzman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EdwinGuzman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviawongnyc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EdwinGuzman left a comment

Choose a reason for hiding this comment

bigfishdesign13 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

7emansell commented Apr 28, 2025 •

edited by jira bot

Loading

vercel bot commented Apr 28, 2025 •

edited

Loading

EdwinGuzman Apr 28, 2025 •

edited

Loading