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

Update ActionList checkbox styles #3607

Merged
merged 20 commits into from
Aug 24, 2023
Merged

Update ActionList checkbox styles #3607

merged 20 commits into from
Aug 24, 2023

Conversation

langermank
Copy link
Contributor

@langermank langermank commented Aug 9, 2023

Checkbox styles for ActionList should match our Checkbox component. And that's what I did.

This is a bit of a copy/paste from Checkbox, but it doesn't totally make sense to share styles with a semantic input. Also, this isn't accounting for an upcoming change to make ActionMenu use checkmarks for single and multi. This is mostly needed for SelectPanel.

Question for reviewers

Is this a minor or breaking change?

Closes https://github.com/github/primer/issues/2487

Screenshots

| Before | After |
| Old checkbox design | New checkbox design |

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Changes are SSR compatible
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

@changeset-bot
Copy link

changeset-bot bot commented Aug 9, 2023

🦋 Changeset detected

Latest commit: 0556808

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2023

size-limit report 📦

Path Size
dist/browser.esm.js 104.66 KB (+0.71% 🔺)
dist/browser.umd.js 105.21 KB (+0.72% 🔺)

@github-actions github-actions bot temporarily deployed to storybook-preview-3607 August 9, 2023 01:53 Inactive
@primer primer deleted a comment from github-actions bot Aug 9, 2023
@github-actions github-actions bot temporarily deployed to storybook-preview-3607 August 9, 2023 01:54 Inactive
@langermank langermank temporarily deployed to github-pages August 9, 2023 01:55 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3607 August 9, 2023 01:56 Inactive
placeContent: 'center',
width: 'var(--base-size-16, 16px)',
backgroundColor: selected ? 'accent.fg' : 'canvas.default',
transition: selected
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mperrotti I could use some eyes on the transitions. I know something is off about it but I can't quite find it, and I think everything is backwards here compared to a checked state since I'm using selected.. but fresh eyes on it will probably reveal the issue.

@primer primer bot temporarily deployed to github-pages August 9, 2023 02:05 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3607 August 9, 2023 02:06 Inactive
@langermank langermank temporarily deployed to github-pages August 9, 2023 02:15 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3607 August 9, 2023 02:16 Inactive
@primer primer bot temporarily deployed to github-pages August 9, 2023 02:22 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3607 August 9, 2023 02:22 Inactive
@joshblack
Copy link
Member

Is this a minor or breaking change?

I feel like this should be a minor change, personally. Curious what others think 👀

@github-actions github-actions bot temporarily deployed to storybook-preview-3607 August 23, 2023 21:04 Inactive
@langermank langermank temporarily deployed to github-pages August 23, 2023 21:06 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3607 August 23, 2023 21:06 Inactive
@langermank langermank temporarily deployed to github-pages August 23, 2023 21:17 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3607 August 23, 2023 21:18 Inactive
@langermank langermank temporarily deployed to github-pages August 23, 2023 21:28 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3607 August 23, 2023 21:28 Inactive
@primer primer bot temporarily deployed to github-pages August 23, 2023 21:42 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3607 August 23, 2023 21:43 Inactive
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