-
Notifications
You must be signed in to change notification settings - Fork 58
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
CPLAT-17301 Update AriaPropsMixin #742
Conversation
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
Co-authored-by: brianphillips-wk <[email protected]>
Co-authored-by: brianphillips-wk <[email protected]>
Co-authored-by: brianphillips-wk <[email protected]>
…_react into CPLAT-17301-aria-attributes
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.
Two tiny things otherwise this looks great.
Thanks for updating all the doc comments, too!
Co-authored-by: Greg Littlefield <[email protected]>
…_react into CPLAT-17301-aria-attributes
Looks like the build is failing on the analyzer test check, but this seems to be failing on the master branch as well. Please let me know how to go about this |
@sydneesampson-wk We should be able to ignore that check for now, sorry about that! |
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.
+10
@Workiva/release-management-p
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.
Looks great! Just had one nitpick about a doc comment location
///See: <https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes> | ||
|
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.
For this - when "documenting" an entire page / library, we usually put them above the library directive. In this case - that is on line 15...
/// See: <https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes>
library over_react.aria_mixin;
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.
+1 from RM
Motivation
Update AriaPropsMixin with missing attributes
Changes
Added missing Aria attributes
Added missing Aria roles
Updated outdated W3 links
Add new documentation
Release Notes
Review
See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.
Please review:
QA Checklist
Merge Checklist
While we perform many automated checks before auto-merging, some manual checks are needed: