-
Notifications
You must be signed in to change notification settings - Fork 4
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
FORNO-1690: Add toggle-group component #415
Conversation
✅ Deploy Preview for vip-design-system-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Thanks for bringing this to the vip-design-system, I really appreciate!
It looks like the implementation of this component is very similar to the RadioBoxGroup. Also, there are some concerning accessibility issues with the tabIndex implemented.
Can you check how much work would take to improve RadioBoxGroup and make this a variant of that component? Maybe a chip
variant. I am not the best for names, but I can see a big future on extending RadioBoxGroup.
Thank you!
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.
@aswasif007 Can you check Storybook A11Y issues?

Ps: This is a "radio" choice, so it must have a "radio" input under the hood. You should be able to navigate the keyboard keys left and right. Could you check the original RadioBoxGroup and navigate using the arrow keys?
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 left comments in the existing threads.
2eca0ef
to
6e24e6d
Compare
Co-authored-by: Todd Wright <4177859+t-wright@users.noreply.github.com>
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.
Thanks for working on this 🎉 LGTM
Thanks Todd! Djalma is on vacation so merging without his approval. |
Description
Add toggle group component.
Click for design.
Checklist
Steps to Test
npm run dev
.