Skip to content
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(web-components): Use ElementInternals for Radio and RadioGroup components #31783

Merged
merged 14 commits into from
Jul 3, 2024

Conversation

radium-v
Copy link
Contributor

@radium-v radium-v commented Jun 20, 2024

Previous Behavior

Currently, form-associated custom elements use the FormAssociated class mixin. This mixin is heavy and duplicative, being copied wholesale for every component which utilizes it. While FormAssociated includes minimal support for ElementInternals, the implementation isn't fully baked.

New Behavior

  • Rewrites <fluent-radio> and <fluent-radio-group> to use ElementInternals
  • Improves ARIA accessibility for <fluent-field>, <fluent-checkbox>, and <fluent-switch>

Related Issue(s)

@radium-v radium-v self-assigned this Jun 20, 2024
@radium-v radium-v requested review from a team as code owners June 20, 2024 17:10
@fabricteam
Copy link
Collaborator

fabricteam commented Jun 20, 2024

📊 Bundle size report

✅ No changes found

@radium-v radium-v force-pushed the users/radium-v/wc3-radio branch 2 times, most recently from da4b780 to 492772d Compare June 21, 2024 01:29
@radium-v
Copy link
Contributor Author

Support for radio groups in a toolbar will need to come at a later time when a toolbar component is added via #26717

@radium-v radium-v force-pushed the users/radium-v/wc3-radio branch 2 times, most recently from dcc4b33 to f142afa Compare June 24, 2024 19:20
@tudorpopams tudorpopams requested a review from Hotell June 26, 2024 12:05
@radium-v radium-v force-pushed the users/radium-v/wc3-radio branch 3 times, most recently from 5948663 to e004311 Compare June 28, 2024 00:53
@Hotell Hotell removed request for a team and Hotell June 28, 2024 12:22
packages/web-components/src/checkbox/checkbox.ts Outdated Show resolved Hide resolved
packages/web-components/src/radio/radio.ts Outdated Show resolved Hide resolved
packages/web-components/src/radio/radio.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@davatron5000 davatron5000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work. Such a big update and clear you put a lot of time into. I had a couple questions but there's anything in particular that's blocking me or can't be addressed later in a follow up.

packages/web-components/src/radio/radio.ts Outdated Show resolved Hide resolved
packages/web-components/src/radio/radio.ts Show resolved Hide resolved
packages/web-components/src/checkbox/checkbox.ts Outdated Show resolved Hide resolved
packages/web-components/src/field/field.ts Show resolved Hide resolved
@radium-v radium-v merged commit f02ae0b into microsoft:master Jul 3, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature: web-components] Update Radio/RadioGroup implementation to use ElementInternals
6 participants