-
Notifications
You must be signed in to change notification settings - Fork 125
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
role=note fleshed out a little. #1639
Conversation
Language a little less terse. Explicit mention of aria-details to associate a note with another element.
Normative "SHOULD" replaced with informative "can". Use cases for aria-describedby and aria-details added
Should this PR be linked to a particular issue? |
@cookiecrook it arose from #1629 |
jnurthen marked as non substantive for IPR from ash-nazg. |
I have made edits and provided examples to flesh out the work that Brennan has started, while addressing the concerns I had noted. @brennanyoung I would ask that you review to make sure this is now clear from your end. If so, then we can merge this branch into yours, and then request the working group to provide further review to move this along.
While my commit message mentioned I was creating a separate PR to review and to then merge into this branch, unfortunately something was not working with that process, and my separate branch kept errorring out. Fortunately, through the magic of version control, we can revert / make changes to anything that need be. But as my commit message mentions, @brennanyoung please review these updates when you can. Thanks |
@jnurthen hmm, tried to remove myself from the reviewers since i've now added a bunch of content to this PR - clearly that didn't work. Worth getting someone else besides me to review now. |
if one is going to make an example to show how to use `role=note`, one should remember to actually use `role=note`...
Co-authored-by: James Nurthen <[email protected]>
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.
Good!
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.
Approve with suggested editorial change.
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.
I added a code suggestion for one small typo, otherwise this looks terrific to me. 🚀
Co-authored-by: James Craig <[email protected]>
Co-authored-by: Adam Page <[email protected]>
thank you @adampage and @cookiecrook @jnurthen seems we can merge this now :) |
SHA: abbef89 Reason: push, by pkra Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Language a little less terse. Explicit mention of aria-details to associate a note with another element. Co-authored-by: Brennan Young <[email protected]> Co-authored-by: Scott O'Hara <[email protected]> Co-authored-by: James Nurthen <[email protected]> Co-authored-by: James Craig <[email protected]> Co-authored-by: Adam Page <[email protected]>
Language a little less terse. Explicit mention of aria-details to associate a note with another element.
Preview | Diff
Preview | Diff