-
Notifications
You must be signed in to change notification settings - Fork 601
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: add radio component #2985
Conversation
3b32d58
to
e04221a
Compare
packages/web-components/fast-components/src/radio/radio.styles.ts
Outdated
Show resolved
Hide resolved
e04221a
to
4214ba0
Compare
packages/web-components/fast-components/src/radio/radio.template.ts
Outdated
Show resolved
Hide resolved
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.
Similar to checkbox, I think our intent here is that this control behaves and is communicated to AT just as the native HTML radio buttons are input[type="radio"]
. My primary concern is whether or not we have validated that is the case here. I've had feedback (linked in this PR as comments) on the W3C repo that label may not be valid for ARIA radios. Given that our goal here is parity with the platform, I think we should validate our assumption that this gives us parity from an AT standpoint. If we have parity with platform controls in terms of how these behave with AT, we're all good.
Two things to look for which are top of mind (not exhaustive):
- Focus behavior differs from the platform for ARIA spec (noted here: https://w3c.github.io/aria-practices/#radiobutton)
- Label behavior and validity may be different from HTML radios.
Again, the list is not exhaustive and my primary point is that we should validate this works as expected with assistive tech. My primary concerns here stem from feedback on existing implementations (react) where our aria radios do not have parity with the platform.
The last thing I will point out is that the W3C keyboard guidance calls out specific navigation behaviors for radios within a toolbar. What is our story for that? A possible solution: If we have parity with the platform in how these are communicated to AT, I would expect that implementation to match how you would do it with native platform controls. If we don't have parity with AT, I think we need to consider if we should map to the aria specification and the aria radio behaviors.
be3022a
to
76331ae
Compare
packages/web-components/fast-components/src/radio/radio.styles.ts
Outdated
Show resolved
Hide resolved
packages/web-components/fast-components/src/radio/radio.styles.ts
Outdated
Show resolved
Hide resolved
packages/web-components/fast-components/src/radio/radio.styles.ts
Outdated
Show resolved
Hide resolved
2681459
to
6988d53
Compare
packages/web-components/fast-components/src/radio/fixtures/base.html
Outdated
Show resolved
Hide resolved
6988d53
to
cc07b6e
Compare
… via the radiogroup component
cc07b6e
to
7998012
Compare
Add radio web component
Is this a breaking change?
Process & policy checklist