[Rating] Clear value if selected value is clicked #18999
Conversation
Details of bundle changes.Comparing: f3218f2...46ddeec
|
mbrookes
added
the
component: rating
oliviertassinari
changed the title
|
@oliviertassinari While looking at the code to understand why the value is set to null, rather than zero to reset the rating (given that the defaultChecked input has a value of 0), I noticed that the zero input is never actually rendered. This is because but the (abstract) equality check is for null: Tangentially related - if the hidden input represents no current rating, should the label say that, rather than "0 Stars"? (Zero stars sounds like the lowest rating, rather than no rating.) Unrelated - should the disabled rating have a role and aria-label? (And what's the practical distinction between disabled and read-only for rating component?) |
|
@mbrookes Great catch! I have updated the pull request to reflect it.
I don't know, I couldn't find any better resource for a11y than https://www.w3.org/WAI/tutorials/forms/custom-controls/.
Read-only is for presentational rating, e.g. the ones you will see when looking at a review. |
oliviertassinari
force-pushed
the
clear-rating
branch
2 times, most recently
from
06a66a1
to
9a8055e
Compare
oliviertassinari
force-pushed
the
clear-rating
branch
from
9a8055e
to
46ddeec
Compare
| @@ -433,6 +456,10 @@ Rating.propTypes = { | |||
| * The icon to display when empty. | |||
| */ | |||
| emptyIcon: PropTypes.node, | |||
| /** | |||
| * The label read when the rating input is empty. | |||
There was a problem hiding this comment.
| * The label read when the rating input is empty. | |
| * The label read by a screen reader when the rating input is empty. |
?
There was a problem hiding this comment.
"You do not have permission to push to this repository."
|
The update looks good. Regarding the disabled state - at the moment in the docs example we have the legend "Disabled", then a set of presentational SVGs. There's nothing to indicate to a screen reader what it is that's disabled (hence the question about a label). We could make the legend more explicit "Disabled rating input", but I wonder instead if it should have actual disabled radio inputs? (Would that more accurately represent to a screen reader the current state of the component?) |
Agree, it sounds better. Probably something for a follow-up. |
|
@ivowork Thanks for the care on the component. It's a good step forward. |
|
Side note, the component popularity relative to the others, position 51:
|
|
I'm sorry for bumping such an old PR but this seems like a good place to ask my question so people with the same question will find the answer. How can you change the rating to zero? |
While using the Rating, I faced the challenge that it was not possible to clear the rating without adding a "clear" button. This PR sets the rating value to null if the current rating is clicked.
