-
Notifications
You must be signed in to change notification settings - Fork 10.8k
Add role="status"
to woocommerce-info
type notices
#60253
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
Add role="status"
to woocommerce-info
type notices
#60253
Conversation
In #44283, when moving new notice styles to display in block themes only, there was an oversight in removing the `role="alert"` for the classic info notices. The other notices, `success` and `error`, already have `role="alert"` applied to them, so that was inconsistent and meant that `info` notices were not read by screen readers. This affected all extensions that used `wc_add_notice` to display info notices.
Testing GuidelinesHi @Aljullu , Apart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
📝 WalkthroughWalkthroughA patch was applied to WooCommerce to address accessibility issues with classic store notices generated by Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
role="alert"
to woocommerce-info
type noticesrole="status"
to woocommerce-info
type notices
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes proposed in this Pull Request:
In #44283, when moving new notice styles to display in block themes only, there was an oversight in removing the
role="alert"
for the classic info notices.The other notices,
success
anderror
, already haverole="alert"
applied to them, so that was inconsistent and meant thatinfo
notices were not read by screen readers. This affected all extensions that usedwc_add_notice
to display info notices.Closes #59383.
Bug introduced in PR #44283.
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Fixed accessibility for all classic store notices added by the
wc_add_notice
function.Changelog Entry Comment
Comment