-
Notifications
You must be signed in to change notification settings - Fork 229
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
Rating widget accessibility #281
Conversation
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.
one quick question on one of the comments.
form/Rating.js
Outdated
_onClick: function(evt) { | ||
if (evt.target.tagName === 'LABEL') { | ||
var clickedValue = +domAttr.get(evt.target.querySelector('input'), "value"); | ||
// for backwards compatibility |
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.
backwards compatibility with previous versions of Dojo or browsers or ?
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 previous versions of Dojo, because there used to be a value
attribute directly on the <li>
tag. Added this in case anyone was listening to the click event and looking at evt.target.value
.
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.
That's fine, the comment should say that so we remember what type of compatibility we were trying to preserve. :)
Updates the rating widget to use radio inputs for each star to add proper semantics and keyboard accessibility.
Resolves #280