-
Notifications
You must be signed in to change notification settings - Fork 151
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
Add accessibility props to Seat #4512
Conversation
7e881a9
to
000462e
Compare
Deploying orbit with Cloudflare Pages
|
Storybook staging is available at https://kiwicom-orbit-dsil-add-a11y-seat.surge.sh |
Size Change: +46 B (+0.01%) Total Size: 459 kB
ℹ️ View Unchanged
|
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.
Could you also add those labels to SB stories (Playground/Legend), please? :)
@sarkaaa you mean as controls or as defined props? I don't think we should have as controls, as it visually does not change anything on the story. I don't mind adding dummy constant values to the rendered components in stories, so we can try them with screen readers, if we want |
domi and I added those props to Playground stories as well, although they are dummies props. It doesn't have any impact on the story itself at all, it's just a convention that we created in summer. :D |
docs/src/documentation/03-components/08-visuals/seat/03-accessibility.mdx
Show resolved
Hide resolved
ac10505
to
336c97b
Compare
336c97b
to
320152d
Compare
9e8e308
to
101c89a
Compare
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.
There are some changes needed in the Story file after your changes:
title
anddescription
can be removed fromexclude
in the Default story as they no longer have default values - they won't be displayed in controls without default values.aria-labelledby
needs to be added to theexclude
in the Default story as now there is broken control withSet object
button.- Also Playground story needs to be handled similarly.
So there are two options:
- If you want to exclude
aria-labelledby
from both Default and Playground stories,aria-labelledby
needs to be added toexclude
on meta (this will exclude it from Playground story) and also added to the Default story exclude (withtitle
anddescription
removed). - If you want to exclude it only from the Default story, Default exclude need to be adjusted but also
text
type needs to be set on meta argTypes as it seem to be incorrectly inferred :/
"aria-labelledby": {
control: {
type: "text",
},
},
101c89a
to
343031a
Compare
BREAKING CHANGE: `title` and `description` props no longer have default values. They can be manually set with text to be announced by screen readers.
It receives a string of id(s)
BREAKING CHANGE: `label` and `price` props now only accept string. They no longer accept ReactNode
343031a
to
b6ec76a
Compare
Proposal for adding accessibility to Seat component.
New prop
aria-labelledby
added to Seat component. Thetitle
anddescription
props no longer have default values (Breaking change).New prop
aria-label
added to SeatLegend component.New accessibility documentation page added for Seat component.
FEPLT-2157