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

Theme radio buttons refactor #16525

Merged
merged 11 commits into from
Apr 17, 2023
Merged

Conversation

tidy-dev
Copy link
Contributor

@tidy-dev tidy-dev commented Apr 12, 2023

xref: https://github.com/github/accessibility-audits/issues/3273

Description

This PR updates the theme selector to look like radio buttons.

This PR:

  1. Introduces a RadioGroup component that we will likely need to wrap most of radio button usages with anyways because they should be in radio groups but are not currently.
  2. Implements the RadioGroup component in the Appearance settings and adds custom styling to mimic dotcom's theme selection..

Screenshots

Preferences dialog opened to Appearance tab showing radio buttons with labels including an image swatch representative of dark, light, and system themes

Release notes

Notes: [Improved] Make radio theme selection look like radio buttons.

@tidy-dev tidy-dev marked this pull request as ready for review April 13, 2023 13:56
@tidy-dev tidy-dev changed the title POC: Theme radio buttons refactor Theme radio buttons refactor Apr 13, 2023
app/src/lib/feature-flag.ts Show resolved Hide resolved
app/src/ui/lib/radio-group.tsx Show resolved Hide resolved
app/src/ui/preferences/appearance.tsx Outdated Show resolved Hide resolved
app/styles/ui/_preferences.scss Show resolved Hide resolved
@tidy-dev tidy-dev requested a review from sergiou87 April 14, 2023 15:30
Copy link
Member

@sergiou87 sergiou87 left a comment

Choose a reason for hiding this comment

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

Code looks good and it works great! ✨ :shipit:

The only thing that I would tweak is the size of the new "theme icons" to make them smaller, because right now this makes the Appearance section a bit taller than the rest, which feels like a step back from #16313

But given we want to deploy a beta build with this, I think that change can be made in a different PR!

@tidy-dev tidy-dev merged commit ec62980 into development Apr 17, 2023
@tidy-dev tidy-dev deleted the theme-radio-buttons-refactor branch April 17, 2023 14:12
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.

3 participants