ToggleGroupControl: improve animation #65175
Conversation
Co-authored-by: Marco Ciampini <[email protected]>
…mprove/tabs-indicator-animation-and-utils
|
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. |
DaniGuardiola
added
[Type] Enhancement
|
I'm done for the day so I haven't reviewed yet, but noting that this is also a bug fix for animation issues like this ✨ CleanShot.2024-09-10.at.23.02.29.mp4(Seen in the Query block when "Inner blocks use content width" is enabled.) |
|
@mirka nice! I didn't know about that bug, glad to learn that it is fixed by this :) |
|
Heads up, I've DRYed it up a bit by moving the animation code into a |
|
The transition feels snappier, and I think that the border radius math is working well! Although it looks like the deselectable version of the component has a visual bug when un-selecting an option item: Kapture.2024-09-30.at.19.05.04.mp4It's also probably better to incorporate changes from #64371 , since it introduces tweaks to |
packages/components/src/toggle-group-control/toggle-group-control/styles.ts
Show resolved
Hide resolved
|
@ciampo found the cause, works as expected now. Also, that PR merged and I synced here. Did one final round of testing, all looks good on my side. Final review? |
| baseId, | ||
| isBlock: ! isAdaptiveWidth, | ||
| size, | ||
| // @ts-expect-error - This is wrong and we should fix it. |
There was a problem hiding this comment.
This mismatch between null and undefined as possible value types was already there prior to this PR, and it sounds like something we should solve — cc @mirka , could this be part of the work you're intending to do about fixing ToggleGroupControl's focus bug?
packages/components/src/toggle-group-control/toggle-group-control/component.tsx
Show resolved
Hide resolved
packages/components/src/toggle-group-control/toggle-group-control/styles.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
I pushed a couple of commits to remove merge conflicts, fix snapshots and make sure that I was testing on top of latest trunk.
I couldn't find any bugs or regressions, and the animation definitely feels smoother with the recent change to using transform.
I left a couple of comments, but they're small stuff and this PR can be merged once they are addressed
LGTM 🚀
…mprove/toggle-group-control-animation
* Refactor utils and switch Tabs indicator animation to `transform`. * docs tweak * Update packages/components/src/tabs/styles.ts Co-authored-by: Marco Ciampini <[email protected]> * Add RTL support. * Addressed @ciampo's comments. * Correct for antialiasing rounding. * Make antialiasing adjustment optional. * Use larger base value and revert antialiasing adjustment code. * DRY RTL * Remove RTL story (redundant since Storybook has a dynamic setting to test RTL). * Fix bug. * Fix bug (for real this time). * Add changelog entry. * De-cleverfy code. * Sync useResizeObserver with WordPress#64943 and make useTrackElementOffsetRect resilient. * Deduplicate utility and clean up. * Minor Tabs code improvement. * Replace framer motion animation with faster CSS animation. * DRY antialiasing factor. * Changelogs. * Various improvements, fixes, and feedback addressed. * Simplify. * Simplify using derived state. * Add similar useOnValueUpdate detail to Tabs. * Fix skipping animation. * Add similar detail to Tabs animation. * Fix setState depth issue. * Fix unit test error. * Add changelog entries * Fix changelog * Update test snapshot. * Depends less on React. * Switch to layout effect for `useOnValueUpdate` * Switched to transform strategy. * Fix resizing bug. * Transition border-radius too. * Undo Tabs changes (no longer relevant). * DRY animation code. * Avoid useless re-runs in effect. * Rename `activeElement` -> `selectedElement` for clarity. * Rename `--indicator-` -> `--selected-` for accuracy. * Minor tweak. * Add safe defaults to CSS custom properties. * Tweak `useSubelementAnimation`. * Fix parent missing when there's no selected option. * Update snapshots * Add docs to utility. * Added note about border-radius. --------- Co-authored-by: Marco Ciampini <[email protected]> Co-authored-by: DaniGuardiola <[email protected]> Co-authored-by: ciampo <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: mirka <[email protected]>
This reverts commit 5ef57bf.
* Refactor utils and switch Tabs indicator animation to `transform`. * docs tweak * Update packages/components/src/tabs/styles.ts Co-authored-by: Marco Ciampini <[email protected]> * Add RTL support. * Addressed @ciampo's comments. * Correct for antialiasing rounding. * Make antialiasing adjustment optional. * Use larger base value and revert antialiasing adjustment code. * DRY RTL * Remove RTL story (redundant since Storybook has a dynamic setting to test RTL). * Fix bug. * Fix bug (for real this time). * Add changelog entry. * De-cleverfy code. * Sync useResizeObserver with #64943 and make useTrackElementOffsetRect resilient. * Deduplicate utility and clean up. * Minor Tabs code improvement. * Replace framer motion animation with faster CSS animation. * DRY antialiasing factor. * Changelogs. * Various improvements, fixes, and feedback addressed. * Simplify. * Simplify using derived state. * Add similar useOnValueUpdate detail to Tabs. * Fix skipping animation. * Add similar detail to Tabs animation. * Fix setState depth issue. * Fix unit test error. * Add changelog entries * Fix changelog * Update test snapshot. * Depends less on React. * Switch to layout effect for `useOnValueUpdate` * Switched to transform strategy. * Fix resizing bug. * Transition border-radius too. * Undo Tabs changes (no longer relevant). * DRY animation code. * Avoid useless re-runs in effect. * Rename `activeElement` -> `selectedElement` for clarity. * Rename `--indicator-` -> `--selected-` for accuracy. * Minor tweak. * Add safe defaults to CSS custom properties. * Tweak `useSubelementAnimation`. * Fix parent missing when there's no selected option. * Update snapshots * Add docs to utility. * Added note about border-radius. --------- Co-authored-by: Marco Ciampini <[email protected]> Co-authored-by: DaniGuardiola <[email protected]> Co-authored-by: ciampo <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: mirka <[email protected]>
(#64926 needs to merge first)
What?
Replaced framer motion based animation with a faster CSS animation similar to the work already done in Tabs (see #64926 for latest update there).
Why?
Faster and more lightweight, also see #60975
How?
Implement similar animation to Tabs, with the main difference being that instead of being transform-based, it is left + width based since that retains appropriate border radius dimensions.
Testing Instructions
Try the component in the Storybook or in Gutenberg (e.g. in the block inspector with a paragraph selected -> "Typography - SIZE").
Testing Instructions for Keyboard
Try the component with a keyboard.
Screenshots or screencast
Looks exactly the same as before, possibly feels a bit more snappy though it's hard to tell.
Kapture.2024-09-10.at.03.11.54.mp4