-
Notifications
You must be signed in to change notification settings - Fork 90
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
feat(RadioGroup): improve radio-group components a11y #196
Conversation
Preview is ready. |
@@ -14,10 +14,12 @@ const options: RadioButtonOption[] = [ | |||
{value: 'Value 3', content: 'Value 3'}, | |||
]; | |||
|
|||
const DefaultTemplate: Story<RadioButtonProps> = (args) => <RadioButton {...args} />; | |||
const DefaultTemplate: Story<RadioButtonProps> = (args) => { | |||
const [value, setValue] = React.useState<string>(options[0].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.
Why not use storybook controls?
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.
Is it able to set control value from callback? Otherwise example wont be interactive.
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.
Nope, storybook is not intended to be interactive, it is more like UI snapshots than demos.
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.
It's kinda weird when you open a demo and it's not working properly. And the only way to change the value is to do it by hand.
0c438d3
to
4b5df18
Compare
Preview is ready. |
'aria-label': props['aria-label'], | ||
'aria-labelledby': props['aria-labelledby'], |
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.
if aria-label
set, aria-labelledby
should not be setted
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.
and vice versa
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.
Looks like i was wrong
Closes #179