-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
CheckboxControl: Add background transition #65212
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
c912e62
to
e505e40
Compare
Size Change: -11.6 kB (-0.65%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
I saw some neat designs by @nick-a8c for this: The way the check mark appears to be 'drawn' is a nice detail... do you think that's feasible? |
If the check was a simple path + |
Stroke with dash-width or box sliding above, both seem fine. But we need to think about this in a way that can ideally scale to most icons, as I could see some icon animation across many more aspects of the interface, even things like block transform icons which would apply to every block icon. I mention this in an aspirational sense to mainly make sure we think ahead to what's scalable once we have an icon build system too. And in that world, we also need these icons to function without any background color, the glyph only. Does that mean we need to use masking? Should all icons be strokes mainly? Or a combination thereof? Where can the animation CSS live and how can it be triggered? Doesn't have to be solved for this PR, but it's a question of whether the CSS should live with the icon, or the wrapping component. |
The box sliding technique seems like a nice short term solution. |
I'm struggling to imagine an animation that could be meaningfully applied to all icons — do you have anything in mind? For example, I'm not sure that the "reveal" effect applied to the checkbox icon could be applied to all icons. For sure there are some general common-sense rule that come to mind: keep paths simple, keep paths separate if we need them to be animated / colored independently from other paths
I believe that having monochromatic icons that use the CSS
Good question. If we want a specific animation to be the canonical animation associated with a given icon, it could make sense to add the animation code to the same package. Maybe we could add a prop (or offer an "animated" version of the icon on the side) — definitely something to think about.
Alternatively, we could simply the animation by having the checkbox icon simply fade in + scale up, without the "sliding mask" effect. |
Thanks, folks, for the feedback! To achieve the box sliding effect, we'll need a markup change. To achieve the drawing effect properly, we'll need to alter the SVG. FWIW, I intended this to be a quick win without changing the markup or redrawing the SVG. Keeping it as simple as possible. With that in mind, I prefer going with what @ciampo suggested here:
Here's how that looks like: Screen.Recording.2024-09-11.at.20.10.54.movI'm happy to tweak timings or starting/ending points, although I opted for the simplest possible configuration. I'm happy to close this if we instead aim at а more complex / rewriting solution or any of these:
WDYT? |
@keyframes components-checkbox__appear-animation { | ||
from { | ||
opacity: 0; | ||
transform: translate(-50%, -50%) scale(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll let folks share more detailed animation specs here, but normally we'd scale from a higher initial value, eg 0.8
@@ -84,9 +84,24 @@ svg.components-checkbox-control__indeterminate { | |||
@include break-small() { | |||
--checkmark-size: calc(var(--checkbox-input-size) + 4px); | |||
} | |||
|
|||
|
|||
animation: components-checkbox__appear-animation 0.1s ease-in-out; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we wanted to show an animation also when unchecking the checkbox, we could use transition
instead of animation, and apply the two different transition states using on input:checked + icon
selector.
Also, what's the deal with the indeterminate state? Should we animate to/from it?
I'm a bit skeptical about merging without some of the little details in the design (#65212 (comment)); particularly the scale animation on the active checkbox surface, and the 'drawing' effect of the checkmark itself. I forked Marcos Codepen to create something I think we could shoot for in the short term. |
Thanks @jameskoster, makes sense. Going to close this one, since I wasn't planning on spending additional time for a bigger refactoring. My intent was to do a quick improvement as I was driving by. Happy to revisit this once I have a more fitting chunk of time. |
What?
This PR updates
CheckboxControl
to also add a transition to the background color.Why?
Previously, we were only doing it for border color. Animating the background color makes checking of the checkbox more visually appealing.
How?
We're just adding a
background
transition.Testing Instructions
CheckboxControl
component in Storybook: http://localhost:50240/?path=/docs/components-checkboxcontrol--docsprefers-reduced-motion
is enabled.Testing Instructions for Keyboard
Same
Screenshots or screencast
Screen.Recording.2024-09-10.at.17.20.58.mov
Screen.Recording.2024-09-10.at.17.19.25.mov
Screen.Recording.2024-09-10.at.17.21.53.mov
Screen.Recording.2024-09-10.at.17.24.39.mov