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

feat(button): add "featured" variant #1857

Merged
merged 17 commits into from
Dec 6, 2024

Conversation

dancormier
Copy link
Contributor

@dancormier dancormier commented Nov 14, 2024

STACKS-680

TODO

Note

I've opened this PR up for review but there's a couple of outstanding items that need to be resolved:

  • Verify name "featured"
  • Write description in docs

This PR adds a "featured" (purple) variant to the button component.

Screenshots

Light

image

Dark

image

Light high contrast

image

Dark high contrast

image

Testing

  1. Go to /product/components/buttons/#featured
  2. Verify the newly added featured buttons look and act as expected

Notable changes

High contrast filled background color

To meet accessibility requirements, the featured filled variant background color has been set to --purple-500 (normally --purple-400).

We've modified the purple-400 light HC mode color instead. See #1857 (review)

Borders on filled, selected buttons

This PR includes a minor fix to button border colors when hovered and filled and/or selected. Previously, the value of --bu-bc (currentColor) was applied and now the border color applied is equal to --bu-bg, ensuring the border and button background are unified.

Button group visual regression test images

There are six visual regression tests (all on s-btn-group-highcontrast-* in Chromium runs) that have been updated. They are visually identical to me but there could be extremely slight differences due to 838e25a.

@dancormier dancormier requested a review from CGuindon November 14, 2024 22:43
Copy link

netlify bot commented Nov 14, 2024

Deploy Preview for stacks ready!

Name Link
🔨 Latest commit 7967508
🔍 Latest deploy log https://app.netlify.com/sites/stacks/deploys/674f801ce3da8e0008103f6d
😎 Deploy Preview https://deploy-preview-1857--stacks.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

When filled/selected buttons were hovered, the currentColor would be used for the border color. This commit ensures the correct border color is used on filled/selected buttons
@dancormier dancormier changed the title WIP: feat(button): add "featured" variant feat(button): add "featured" variant Nov 15, 2024
@dancormier dancormier requested a review from a team November 15, 2024 18:27
@dancormier dancormier marked this pull request as ready for review November 19, 2024 19:11
Copy link
Contributor

@giamir giamir left a comment

Choose a reason for hiding this comment

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

Overall looks good, there is only a minor docs correction to do I left in comment.

Nice to see the PPCP abstraction making our lives easier here.
Well done @dancormier ❤️

docs/_data/buttons.json Show resolved Hide resolved
docs/_data/buttons.json Show resolved Hide resolved
docs/_data/buttons.json Show resolved Hide resolved
Addresses #1857 (comment) and similar
Copy link
Collaborator

@CGuindon CGuindon left a comment

Choose a reason for hiding this comment

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

Ok so the only thing I don't love is that in HC mode, the filled default state is the same color as the filled selected state (no difference).

Because we had to bump the background color from 400 to 500 for HC filled, and because 500 and 600 are the same in HC, we end up with no difference there.

For Base and Danger we have a modified red-400 and blue-400 just for HC mode. Those colors were modified from Light/Dark so that they meet the 7:1 ratio (they're pretty close but not quite otherwise). And so for those variants we're able to keep the 400 stop as the background color for filled default and therefore have a color difference with the selected state.

So....

I'm proposing we modify HC modes purple 400 color to meet a 7:1 ratio with white.
Change the selected state back to using purple-400 but with this new color (in dark mode the purple 400 already passes 7:1 ratio.

  • HC Light purple-400 would become #3B41BA (Figma where I played with this)
  • HC Dark purple-400 would become NO CHANGE NEEDED? since it already passes the 7:1 ratio?

Screenshot 2024-11-27 at 3 08 26 PM
Screenshot 2024-11-27 at 3 08 47 PM

@dancormier dancormier merged commit 4b7f93e into develop Dec 6, 2024
11 checks passed
@dancormier dancormier deleted the STACKS-680/button-add-featured-variant branch December 6, 2024 20:48
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