Fix input switch inconsistency#472
Merged
lucaslarroche merged 3 commits intopicocss:devfrom Mar 3, 2024
Merged
Conversation
Fixes the switch state by making it offset correctly instead of ballpark with the wrong units
Contributor
Author
|
This pull request separately calculates the variables {
/* Show parts of the calculation (current) */
margin-inline-start: calc(#{$switch-width} - #{$switch-height} / 2 - var(#{$css-var-prefix}border-width) * 3);
/* Evaluated: */
margin-inline-start: calc(2.25em - 1.25em / 2 - var(--pico-border-width) * 3);
/* Interpolate everything (alternate) */
margin-inline-start: calc(#{$switch-width - $switch-height * 0.5} - var(#{$css-var-prefix}border-width) * 3);
/* Evaluated: */
margin-inline-start: calc(1.625em - var(--pico-border-width) * 3);
}If you're planning to make the switch sizes variables for users to scale horizontally or vertically, this would probably be worse. |
lucaslarroche
approved these changes
Mar 3, 2024
Member
lucaslarroche
left a comment
There was a problem hiding this comment.
Thank you @FireIsGood, great one!
I appreciate your help.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #467
Previously, the switch
:checked:beforewas incorrectly offset by an approximation using the switch's width and border sizes. This changes them to use the units correctly.I have also rebuilt all the CSS, however you may undo that commit and rebuild yourself if you wish.
Comparison
Before:
After:
Info for CSS nerds
If you wanted to know why the units were wrong and how they were changed:
Here's the technical stuff
The issue was that the previous version was only using the SCSS variable
$switch-widthand then approximating by dividing it by two, then subtracting the border width. The issue with this approach is that the dot ends up being aligned to the center right side.What the code used to do was:
switch-width * 0.5 - border-width)Since this never accounts for the button's size, this method can never work with other knob sizes.
To fix this, we can instead get the full width and then offset based on the button's size before combining them.
What the code does now:
switch-width - border-width * 2)- switch-height * 0.5 - border-width(knob width is equal to half the switch height minus the borders))Combined, this turns into
switch-width - switch-height / 2 - border-width * 3.So yeah.