Skip to content

[Bug] Leading & Trailing Visual Colors for Button #3447

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 11 commits into from
Jul 18, 2023
Merged

Conversation

jesskuo4
Copy link
Contributor

@jesskuo4 jesskuo4 commented Jun 23, 2023

Describe your changes here.

  • Changed the Leading and Trailing Visual Icon Colors for the Button Component to fg.muted
  • Change ButtonCounter font size to 0 (12px)

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

Screenshots

Before After
Before image of trailing/leading icons After image of trailing/leading icons
Before image of counter with wrong size After image of counter with corrected size

Please provide before/after screenshots for any visual changes

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.

@jesskuo4 jesskuo4 requested review from a team and josepmartins June 23, 2023 21:04
@changeset-bot
Copy link

changeset-bot bot commented Jun 23, 2023

🦋 Changeset detected

Latest commit: 84af8ad

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 Patch

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

Uh oh! @jesskuo4, the image you shared is missing helpful alt text. Check your pull request body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 23, 2023

size-limit report 📦

Path Size
dist/browser.esm.js 102.76 KB (+0.02% 🔺)
dist/browser.umd.js 103.31 KB (+0.03% 🔺)

@github-actions
Copy link
Contributor

Uh oh! @jesskuo4, the image you shared is missing helpful alt text. Check your pull request body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

@jesskuo4 jesskuo4 temporarily deployed to github-pages June 23, 2023 21:11 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3447 June 23, 2023 21:12 Inactive
@github-actions
Copy link
Contributor

Uh oh! @jesskuo4, the image you shared is missing helpful alt text. Check your pull request body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

@github-actions github-actions bot temporarily deployed to storybook-preview-3447 June 23, 2023 21:19 Inactive
@langermank langermank temporarily deployed to github-pages June 23, 2023 21:20 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3447 June 23, 2023 21:20 Inactive
@primer primer bot temporarily deployed to github-pages June 23, 2023 22:24 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3447 June 23, 2023 22:24 Inactive
@langermank langermank requested a review from maximedegreve June 23, 2023 23:14
@langermank langermank temporarily deployed to github-pages June 23, 2023 23:14 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3447 June 23, 2023 23:15 Inactive
@primer primer bot temporarily deployed to github-pages June 23, 2023 23:28 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3447 June 23, 2023 23:28 Inactive
@primer primer deleted a comment from github-actions bot Jun 27, 2023
@jesskuo4
Copy link
Contributor Author

jesskuo4 commented Jun 28, 2023

Boom 🎉

Found two small things:

1️⃣ The icon button example is still not muted.

2️⃣ Then the counter is vertically not aligned centred.

Screenshot 2023-06-24 at 13 19 35

Hi @maximedegreve! 👋 Thank you so much for pointing these two things out and waiting, I wanted to double check some things with my mentor, Katie, before responding:

Regarding num1 of this comment, I was thinking that it might be better to keep the icon button only to remain as fg.default since the current disabled icon button uses the fg.muted look. It might be good to consider keeping it this way so users can differentiate the status of the button. We also have an invisible variant, which has a muted color by default but when it becomes disabled, it will look more like this with a background (its broken right now in PRC). What are your thoughts?
image

Regarding num2, I tried correcting the issue. However, I shared with Katie and she mentioned how to solve the problem, we have to change how the the counter button is layered, which might cause other issues. She is currently looking into this and will get back to you!

Thank you!! 😄

@github-actions
Copy link
Contributor

Uh oh! @jesskuo4, the image you shared is missing helpful alt text. Check #3447 (comment).

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

2 similar comments
@github-actions
Copy link
Contributor

Uh oh! @jesskuo4, the image you shared is missing helpful alt text. Check #3447 (comment).

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

@github-actions
Copy link
Contributor

Uh oh! @jesskuo4, the image you shared is missing helpful alt text. Check #3447 (comment).

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

@langermank langermank temporarily deployed to github-pages July 18, 2023 18:48 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3447 July 18, 2023 18:48 Inactive
@langermank langermank temporarily deployed to github-pages July 18, 2023 18:57 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3447 July 18, 2023 18:57 Inactive
@langermank langermank temporarily deployed to github-pages July 18, 2023 21:39 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3447 July 18, 2023 21:39 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.

4 participants