Skip to content

feat: add live region support with Alert, Status, and Announce - #4313

Merged
joshblack merged 5 commits into
mainfrom
feat/add-primer-live-region-element
Mar 5, 2024
Merged

feat: add live region support with Alert, Status, and Announce#4313
joshblack merged 5 commits into
mainfrom
feat/add-primer-live-region-element

Conversation

@joshblack

@joshblackjoshblack commented Feb 27, 2024

Copy link
Copy Markdown
Member

Add support for using the @primer/live-region-element package through three components:

  • Alert, analogous to role="alert", used for assertive announcements
  • Status, analogous to role="status", used for polite announcements
  • Announce, a wrapper component that makes announcements when its text contents have changed using the <live-region> element

These components use Box under the hood and can be styled and customized as-needed. They can be used as the visual parts of a component. They can also be hidden when wrapped with VisuallyHidden.

Note

Wanted to add a section in case you're wondering why we aren't using role="alert" and role="status" directly 👀

The core challenge we're trying to work around is adding the corresponding live region and message at the same time.

Put another way, support for rendering and announcing the following snippet is not great:

<divrole="status">Test message</div>

Broadly speaking, making sure that the element exists first before adding the message has better support than doing both at the same time. This is what the components in this PR, and corresponding @primer/live-region-element, help to accomplish.

Changelog

New

  • Add Alert, Status, and Announce as internal components

Changed

  • Add the file path to Announce to the list of side effects to the package due to its usage of @primer/live-region-element/define

Removed

Rollout strategy

  • None; if selected, include a brief description as to why

This is adding components to our internal set of components and does not impact the public API of @primer/react

Testing & Reviewing

  • Checkout the storybook stories for the Status component and verify that the live region works as expected (let me know if I can help out with instructions for this!)
  • Go through the tests to make sure they have accurate coverage for the component
  • Try to use this component in the app-router example and verify that it can be called in a server component, has no SSR errors, and appropriately announces on render
  • Let me know if you have any feedback on the approach here! Very open to feedback, suggestions, and ideas 👀

@joshblack
joshblack requested review from a team and siddharthkpFebruary 27, 2024 17:56
@changeset-bot

changeset-botBot commented Feb 27, 2024

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: e0ff928

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@joshblackjoshblack added the skip changeset This change does not need a changelog label Feb 27, 2024
@github-actions

github-actionsBot commented Feb 27, 2024

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
packages/react/dist/browser.esm.js113.58 KB (0%)
packages/react/dist/browser.umd.js114.25 KB (0%)

Comment threadpackages/react/src/internal/components/Alert.tsx
Comment threadpackages/react/src/internal/components/Announce.tsx
Comment threadpackages/react/src/internal/components/Announce.tsx
Comment threadpackages/react/src/internal/components/Announce.tsx
return (
<>
<p>This is an example</p>
<VisuallyHidden>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(non blocking) Ah, I was wondering if this should be a prop on the component or the default on the component. I guess we can find out once we start using it!

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Ideally, I want people to think of Status as <div role="status">. It won't do anything more or less than that role, but under-the-hood we make it work. So you can style it if you're using it visually, or if you want to hide then you use VisuallyHidden.

@siddharthkpsiddharthkpMar 5, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ideally, I want people to think of Status as

I see... do you see the default of folks using it an visible element? Maybe in Banner?

@joshblackjoshblackMar 5, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@siddharthkp good question, I definitely see a mix of visible and hidden use-cases but don't have a sense of which one is the majority case yet 👀

@siddharthkpsiddharthkp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Left a few comments, none of them are blocking. Approving in advance

@github-actions
github-actionsBottemporarily deployed to storybook-preview-4313 March 5, 2024 20:19 Inactive
@joshblack
joshblack added this pull request to the merge queueMar 5, 2024
Merged via the queue into main with commit 9e5a0eeMar 5, 2024
@joshblack
joshblack deleted the feat/add-primer-live-region-element branch March 5, 2024 21:13
lukasoppermann pushed a commit that referenced this pull request Apr 16, 2024
* feat: add live region support with Alert, Status, and Announce
* chore: run prettier
* chore(deps): update live-region-element dep and remove sideEffects change
* chore(eslint): fix lint warnings
---------
Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip changesetThis change does not need a changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@joshblack@siddharthkp