Animate: Add types and simplify API #26965
Closed
Conversation
This file contains 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
sirreal
force-pushed
the
try/typing-animate
branch
from
9a909e1 to
ea05b4b
Compare
sirreal
commented
Nov 13, 2020
Comment on lines
72
to
80
| if ( Boolean( _deprecatedOptions ) ) { | ||
| deprecated( '<Animate> options prop', { | ||
| version: '9.3', | ||
| hint: 'Pass options directly as props instead.', | ||
| } ); | ||
| } |
There was a problem hiding this comment.
It's something that needs more updates, documentation need to be aligned:
https://github.com/WordPress/gutenberg/tree/master/packages/components/src/animateIn general, I agree with the sentiment but in practice it might start printing those depreciations in the JS console if its usage isn't updated in Gutenberg. So I would be very careful changing the public API. It also needs to be announced as Dev Note when a new version of WordPress is released so plugins could update.
There was a problem hiding this comment.
Thanks, @gziolo. What's required for a dev note?
sirreal
added
[Package] Components
sirreal
commented
Nov 13, 2020
| // Ignore due to missing types: https://github.com/WordPress/gutenberg/pull/26429 | ||
| // @ts-ignore | ||
| import deprecated from '@wordpress/deprecated'; |
sirreal
added
the
Needs Dev Note
|
I've been testing storybook and there's a regression I'll fix… |
|
Size Change: +93 B (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
This reverts commit 6e8d8b95fca0f30af4d85b2910f114c42917a930.
sirreal
force-pushed
the
try/typing-animate
branch
from
002608e to
263d043
Compare
|
I created a hook in #26201 with the intention of deprecating the |
|
Oh, and stabilise the animate hook of course. The reason I didn't do it in the other PR is that it wasn't clear to me what package it should be in. There's no hooks in the components package yet, and usually we add new hook to |
|
no strong opinions but the hook definitely look better in "components" because it has styles. |
sirreal
added a commit
that referenced
this pull request
Nov 19, 2020
Suggested here: #26965 (comment)
|
I'm working on the proposed alternative in #27123 |
6 tasks
sirreal
added a commit
that referenced
this pull request
Nov 25, 2020
Suggested here: #26965 (comment)
sirreal
added a commit
that referenced
this pull request
Nov 26, 2020
Suggested here: #26965 (comment)
sirreal
added a commit
that referenced
this pull request
Nov 30, 2020
Suggested here: #26965 (comment)
6 tasks
youknowriad
removed
the
Needs Dev Note
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Animatecomponent.Animatecomponent API.useAnimateaccepts anoptionsobject which is a stringtype+ some options based on the type.Animatecomponent acceptedtypeproperty (the same) anoptionsobject (the other options) and achildrenrender function property.The options prop doesn't correspond to
useAnimateoptions (it doesn't include the type) and there seems to be no benefit to wrapping the options in an object.Animatecan pass its props (excludingchildren) touseAnimate.Options are still accepted although deprecated and scheduled for removal in 9.6.
Supersedes #26176
I've replaced the usage in Gutenberg that I found (just one case).
I've also updated the Animate storybook. It now uses knobs to adjust the options on the fly.
How has this been tested?
Types pass.
Storybook examples work as expected.
Screenshots
When using
optionsprop, you'll see this deprecation warning (it says 9.4 but I've changed it to 9.6):Types of changes
New feature: Add types to the Animate component.
This change deprecates the
Animateoptions prop and schedules it to be removed in two releases: 9.6. It will be a breaking change when this is removed.Checklist: