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

fix(toggle-switch): apply lower opacity when disabled #1402

Merged
merged 10 commits into from
Aug 1, 2023

Conversation

dancormier
Copy link
Contributor

@dancormier dancormier commented Jun 26, 2023

This PR applies the disabled element opacity to s-toggle-switch components. Previously, the disabled state for input[type="checkbox].s-toggle-switch would result in no visual changes.

Before

image

After

image


Note
This should be good to merge after we containerize visual regression tests

@dancormier dancormier added the bug A reproducible problem with the Stacks code label Jun 26, 2023
@netlify
Copy link

netlify bot commented Jun 26, 2023

Deploy Preview for stacks ready!

Name Link
🔨 Latest commit 8d9584f
🔍 Latest deploy log https://app.netlify.com/sites/stacks/deploys/64c93cb9a84fff000875091f
😎 Deploy Preview https://deploy-preview-1402--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.

@giamir
Copy link
Contributor

giamir commented Jun 27, 2023

@dancormier out of curiosity why we changed so many unrelated baseline images as part of this PR?

@dancormier
Copy link
Contributor Author

@dancormier out of curiosity why we changed so many unrelated baseline images as part of this PR?

I'm not sure.Looks like when I run npm run test:visual:update it makes minor changes to every image.

image

I'm gonna dig into commits between the last time we've updated the images and now to see if there's any change that might have caused this..

@dancormier
Copy link
Contributor Author

dancormier commented Jun 27, 2023

I went back to the last time baseline images were updated (a813110), tried another update, and saw the same thrashing. My initial theory is that an OS or browser update has modified how these components get rendered, but that theory might not explain why tests are failing via GitHub actions.

I'm going to open a new PR that includes regenerated baseline images and see if it resolves test failures and consider merging in those updated images. Expect a PR review request @giamir

Edit: I just noticed they're all Chromium images that are getting regenerated, so I'm guessing a browser update included a slight rendering update 🤷‍♂️

@dancormier dancormier merged commit ddcd5d5 into develop Aug 1, 2023
5 checks passed
@dancormier dancormier deleted the dcormier/toggle-switch-disabled-fix branch August 1, 2023 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A reproducible problem with the Stacks code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants