Skip to content

feat(cdk-experimental/radio): add radio demo to dev-app #31180

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

Merged
merged 5 commits into from
May 21, 2025

Conversation

wagnermaciel
Copy link
Contributor

No description provided.

@wagnermaciel wagnermaciel requested a review from a team as a code owner May 21, 2025 17:39
@wagnermaciel wagnermaciel requested review from crisbeto and mmalerba and removed request for a team May 21, 2025 17:39
@wagnermaciel wagnermaciel added dev-app preview When applied, previews of the dev-app are deployed to Firebase target: minor This PR is targeted for the next minor release labels May 21, 2025
@wagnermaciel wagnermaciel requested a review from ok7sai May 21, 2025 17:39
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label May 21, 2025
Copy link

github-actions bot commented May 21, 2025

Deployed dev-app for e159090 to: https://ng-dev-previews-comp--pr-angular-components-31180-dev-7ftid85f.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

Copy link
Contributor

@ok7sai ok7sai left a comment

Choose a reason for hiding this comment

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

I noticed that if an option is disabled and selected programmatically, you can still toggle other options with aria-checked. It can just be a bad usage if done by developers, or should we

  1. Only have the disabled option checked, and other options always have aria-checked="false".
  2. Force the disabled option to be not selectable (always has aria-checked="false").

@wagnermaciel
Copy link
Contributor Author

wagnermaciel commented May 21, 2025

I noticed that if an option is disabled and selected programmatically, you can still toggle other options with aria-checked. It can just be a bad usage if done by developers, or should we

  1. Only have the disabled option checked, and other options always have aria-checked="false".
  2. Force the disabled option to be not selectable (always has aria-checked="false").

I think option 1 makes the most sense. I'm wondering if the radio group should automatically set aria-readonly="true" when the selected item is disabled. What are your thoughts?

@ok7sai
Copy link
Contributor

ok7sai commented May 21, 2025

I noticed that if an option is disabled and selected programmatically, you can still toggle other options with aria-checked. It can just be a bad usage if done by developers, or should we

  1. Only have the disabled option checked, and other options always have aria-checked="false".
  2. Force the disabled option to be not selectable (always has aria-checked="false").

I think option 1 makes the most sense. I'm wondering if the radio group should automatically set aria-readonly="true" when the selected item is disabled. What are your thoughts?

Making the group aria-readonly="true" makes sense to me. The only concern is if the "disabled and selected" option is skipped in the navigation, then there's no easy way to know which one is selected in this group.

Tested by setting readonly, skip disabled, selected a flavor that is also disabled. Navigating through the radio buttons and it skips the only one that is announced as "selected".

However this can just be a really bad setup that the developers should aware of that.

@wagnermaciel
Copy link
Contributor Author

I noticed that if an option is disabled and selected programmatically, you can still toggle other options with aria-checked. It can just be a bad usage if done by developers, or should we

  1. Only have the disabled option checked, and other options always have aria-checked="false".
  2. Force the disabled option to be not selectable (always has aria-checked="false").

I think option 1 makes the most sense. I'm wondering if the radio group should automatically set aria-readonly="true" when the selected item is disabled. What are your thoughts?

Making the group aria-readonly="true" makes sense to me. The only concern is if the "disabled and selected" option is skipped in the navigation, then there's no easy way to know which one is selected in this group.

Tested by setting readonly, skip disabled, selected a flavor that is also disabled. Navigating through the radio buttons and it skips the only one that is announced as "selected".

However this can just be a really bad setup that the developers should aware of that.

Yeah, that's true. I think setting readonly would be the best way to move forward and I can add some basic validation to warn developers about this particular edge case.

Copy link
Contributor

@ok7sai ok7sai left a comment

Choose a reason for hiding this comment

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

Validation!

@wagnermaciel wagnermaciel added the action: merge The PR is ready for merge by the caretaker label May 21, 2025
@wagnermaciel wagnermaciel merged commit 3704b7e into angular:main May 21, 2025
23 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker area: material/button-toggle detected: feature PR contains a feature commit dev-app preview When applied, previews of the dev-app are deployed to Firebase target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants