Switch: New component contribution #825
Conversation
michael-iden
requested review from
sirrah-tam,
jialin-he and
mtorres3
and removed request for
a team
🦋 Changeset detectedLatest commit: 19ebdfa The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
size-limit report 📦
|
| height: 32px; | ||
| width: 70px; |
There was a problem hiding this comment.
Are these something that can/should be calculated from other values, or are they pretty squarely hand-crafted? Same question really for any hard-coded values, though some needing more scrutiny than others I'm sure.
There was a problem hiding this comment.
From the mockup they appear to rather handcrafted. Technically 32px is 2rem but that could be mostly coincidental? @fswlee anything here you suggest or worth adding context to?
There was a problem hiding this comment.
the little knob is 1rem x 1rem and there's .5rem space above ad below which creates the height. the width is a little more arbitrary. we could consider being more specific and intentional with our values, for sure. let me look at some options
There was a problem hiding this comment.
Ahh i might have the knob 1px small in both dimensions right now at 15px. I can tweak as part of this too
packages/pharos/src/components/switch/pharos-switch.wc.stories.jsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Brent Swisher <[email protected]>
| border-radius: 2rem; | ||
| background-color: var(--pharos-color-marble-gray-94); | ||
| transition: | ||
| background-color 0.3s ease-out, |
There was a problem hiding this comment.
I do like the way this works, but see it is not one of our transition tokens. I wonder if switching to --pharos-transition-duration-default (which is 250 ms) so it matches our other transitions would be make a noticeable difference?(cc @fswlee as this is mostly a design decision)
There was a problem hiding this comment.
FWIW 50ms is generally imperceptible to humans so I'd go for the token value too 😄
|
🤔 Running pharos site locally, I am not seeing the component render in the examples. but I am seeing the label. 
|
Ah, never mind, the site just needs the new component added where it is initializing Pharos components, I can push a fix for that |
This change: (check at least one)
Is this a breaking change? (check one)
Is the: (complete all)
What does this change address?
Closes #821 by creating a new toggle switch component
How does this change work?
Adds a stylized variant of a checkbox to indicate an enabled/disabled state and a mechanisms to update the state
Additional context