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

Allow each radio in radio group to be disabled/read-only #2474

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

benlister-okta
Copy link
Contributor

DES-7006

Summary

  • After refactoring in the read-only PR for radios, we inadvertently removed the ability to set individual radios as disabled (or read-only). This PR restores that functionality.
  • Also adds a story demonstrating how to do this.

@benlister-okta benlister-okta requested a review from a team as a code owner January 21, 2025 20:50
Copy link
Contributor

@bryancunningham-okta bryancunningham-okta left a comment

Choose a reason for hiding this comment

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

This is fine. But, wondering if we should use aria-disabled and prevent the user selection on our side. So that screen reader users will know this option exists but is disabled.

@benlister-okta
Copy link
Contributor Author

This is fine. But, wondering if we should use aria-disabled and prevent the user selection on our side. So that screen reader users will know this option exists but is disabled.

That's a good point, but I'm wondering if that overlaps into the territory of the Read-only state.

@bryancunningham-okta
Copy link
Contributor

bryancunningham-okta commented Jan 27, 2025

That's a good point, but I'm wondering if that overlaps into the territory of the Read-only state.

I think that is what we want honestly. It should be the same for a non-sighted user. And in the disabled case, would have the disabled styling, as opposed to the readonly styling.

Not necessarily a blocker to this PR going in. Just something I think we should keep in mind for a future enhancement.

@benlister-okta benlister-okta force-pushed the bl_radio_disabledsingle branch from 5558d1f to 62a6a8d Compare January 27, 2025 22:01
@oktapp-aperture-okta oktapp-aperture-okta bot merged commit 1a58030 into main Jan 27, 2025
1 check passed
@oktapp-aperture-okta oktapp-aperture-okta bot deleted the bl_radio_disabledsingle branch January 27, 2025 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants