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

fix(StaticNotification): set describedby on action button #15856

Conversation

finken2
Copy link
Contributor

@finken2 finken2 commented Mar 1, 2024

Follow-on to a previous PR that added the actionButton capability to StaticNotification. I didn't realize that the aria-describedby was being enforced on the button in addition to the body. My storybook example didn't catch that, b/c the currentuseInteractiveChildrenNeedDescription is only checking the first interactive child. So when i went to use it with just the button, kaboom.

Fully tested it out locally in our microservice, recreating the issue and fixing it.
I also changed the proptype of subtitle to Node to match actionableNotification, making the swap easier.

Changelog

New

  • pass titleId to the NotificationButton as aria-describedby. No new prop for this.
  • additional storybook story for the action button only

Changed

  • updated subtitle to be PropTypes.Node to match ActionableNotification.

Testing / Reviewing

render a StaticNotification within live code with action button and embedded links.

@finken2 finken2 requested review from a team as code owners March 1, 2024 13:56
Copy link

netlify bot commented Mar 1, 2024

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 264da49
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/65e1de928d1390000886ea86
😎 Deploy Preview https://deploy-preview-15856--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@finken2 finken2 marked this pull request as draft March 1, 2024 15:19
@finken2 finken2 marked this pull request as ready for review March 1, 2024 15:20
@finken2
Copy link
Contributor Author

finken2 commented Mar 1, 2024

Not picking up my 2nd commit, so going to recreate this PR....

@finken2 finken2 closed this Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant