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.

Sign up for GitHub

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

Jump to bottom

docs: update RadioCardGroup to v2 docs #1176

Merged
merged 2 commits into from
Aug 22, 2023
Merged

docs: update RadioCardGroup to v2 docs #1176

merged 2 commits into from
Aug 22, 2023

Conversation

scurker
Copy link
Member

@scurker scurker commented Aug 22, 2023

No description provided.

@scurker scurker requested a review from a team as a code owner August 22, 2023 19:08
@github-actions
Copy link
Contributor

Preview branch generated at https://radiocardgroup-v2-doc.d1gko6en628vir.amplifyapp.com

@dequejosie
Copy link
Contributor

dequejosie commented Aug 22, 2023 •
edited
Loading

Not so much about the docs, the dark theme of disabled RadioCardGroup looks weird to me. It does not have explicit visual indication that the card is disabled:

Screen Shot 2023-08-22 at 4 50 29 PM

And it looks like this during hover:

Screen Shot 2023-08-22 at 4 50 37 PM

Just wanted to check with you, @scurker

@scurker
Copy link
Member Author

scurker commented Aug 22, 2023

Not so much about the docs, the dark theme of disabled RadioCardGroup looks weird to me. It does not have explicit visual indication that the card is disabled:

Correct. This is a documentation ticket so it doesn't implement any fixes. There is a ticket to address some of the a11y issues with RadioCardGroup (#777) but it's low priority since this component is currently only used in one spot that doesn't have themes.

@scurker scurker merged commit 5668df8 into develop Aug 22, 2023
@scurker scurker deleted the radiocardgroup-v2-doc branch August 22, 2023 22:10
@github-actions
Copy link
Contributor

Preview branch generated at https://radiocardgroup-v2-doc.d1gko6en628vir.amplifyapp.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dequejosie dequejosie dequejosie left review comments

@thuey thuey thuey approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@scurker @dequejosie @thuey