Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Stepper accessibility fixes #4571
Stepper accessibility fixes #4571
Changes from all commits
b8b18fb
0f07f3c
5ace489
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 was thinking of changing this prop name from
title
toariaLabel
inButtonPrimitive
and making it more specific, however, this would be (probably) BC. I searched where this prop is used and I've found it's used here.I'm not sure if it makes sense to change this prop name, if it could be included within this PR or in a separate one.
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.
So your suggestion is to keep prop names in Stepper as
titleIncrement
andtitleDecrement
and change the prop on theButtonPrimitive
fromtitle
toariaLabel
so the usage would beariaLabel={titleIncrement}
?But anyway, I think it's out of scope and this should be tackled when making
Button
/ButtonPrimitive
accessible.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.
No, that wouldn't make sense. :D My suggestion was to rename aria attribute in
ButtonPrimitive
(probably in a different PR) and also changetitleIncrement
(titleDecrement
). I kepttitleDecrement
(titleIncrement
) because of attribute naming in ButtonPrimitive.As you said, I also think it's out of the scope of this PR, that's the reason why I didn't touch ButtonPrimitive at all.