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

ButtonToggle: V2 Development #3611

Merged
merged 32 commits into from
Jun 14, 2024

Conversation

eniomoura
Copy link
Contributor

@eniomoura eniomoura commented Jun 4, 2024

Summary

What changed?

Introduced hair variant and color picker variant to ButtonToggle.
image
image

Why?

To bring web implementation in line with Figma specification.

Links

Checklist

  • [ Yes ] Added unit and Flow Tests
  • [ Yes ] Added documentation + accessibility tests
  • [ Yes ] Verified accessibility: keyboard & screen reader interaction
  • [ Yes ] Checked dark mode, responsiveness, and right-to-left support
  • [ Yes ] Checked stakeholder feedback (e.g. Gestalt designers, relevant feature teams)

Copy link

netlify bot commented Jun 4, 2024

Deploy Preview for gestalt ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit b97a3e2
🔍 Latest deploy log https://app.netlify.com/sites/gestalt/deploys/666b52bda5848400084537a7
😎 Deploy Preview https://deploy-preview-3611--gestalt.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.

@eniomoura eniomoura added the minor release Minor release label Jun 4, 2024
@eniomoura eniomoura self-assigned this Jun 4, 2024
@eniomoura eniomoura marked this pull request as ready for review June 5, 2024 18:34
@eniomoura eniomoura requested a review from a team as a code owner June 5, 2024 18:34
packages/gestalt/src/ButtonToggle.tsx Outdated Show resolved Hide resolved
packages/gestalt/src/ButtonToggle.tsx Outdated Show resolved Hide resolved
packages/gestalt/src/ButtonToggle.tsx Outdated Show resolved Hide resolved
packages/gestalt/src/ButtonToggle/ColorsButton.tsx Outdated Show resolved Hide resolved
packages/gestalt/src/ButtonToggle/ColorsButton.tsx Outdated Show resolved Hide resolved
docs/examples/buttontoggle/graphic.tsx Outdated Show resolved Hide resolved
packages/gestalt/src/ButtonToggle/ColorsButton.tsx Outdated Show resolved Hide resolved
packages/gestalt/src/ButtonToggle/ColorsButton.tsx Outdated Show resolved Hide resolved
packages/gestalt/src/ButtonToggle/GraphicButton.tsx Outdated Show resolved Hide resolved
packages/gestalt/src/ButtonToggle.tsx Outdated Show resolved Hide resolved
@eniomoura eniomoura requested a review from AlbertCarreras June 10, 2024 21:55
@eniomoura eniomoura force-pushed the buttontogglev2-development branch from 4e76f6e to 1b7ad6a Compare June 11, 2024 19:51
@AlbertCarreras
Copy link
Contributor

AlbertCarreras commented Jun 12, 2024

I think we need to handle borders differently

Brave Browser - ButtonToggle - Gestalt 2024-06-12 at 6 26 07 PM

It shakes the surrounding ones, I believe because we are increasing the size of the wrapping borders

@AlbertCarreras
Copy link
Contributor

Can we add here all the variants?
Screenshot by Dropbox Capture

@eniomoura
Copy link
Contributor Author

eniomoura commented Jun 13, 2024

I think we need to handle borders differently

Brave Browser - ButtonToggle - Gestalt 2024-06-12 at 6 26 07 PM Brave Browser - ButtonToggle - Gestalt 2024-06-12 at 6 26 07 PM

It shakes the surrounding ones, I believe because we are increasing the size of the wrapping borders

I tackled this with two changes:

  • The background color change made the border changes in most cases unecessary (no border in black selected states).
  • The only case with a variable size border remaining is the disabled button, and i added some css to offset the component size to compensate for the border size increase.

It is normal behavior in many cases for the button size to change on selected state though, since we recommend the text to change to reinforce the selected state.

@eniomoura eniomoura requested a review from AlbertCarreras June 13, 2024 09:57
@eniomoura eniomoura merged commit 388fec6 into pinterest:master Jun 14, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor release Minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants