TypeScript conversion of animation package
#12702
Merged
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
added 3 commits
November 11, 2022 13:08
# Conflicts: # tsconfig.json
Note that the main story editor does not compile at this stage, but the animation package works in isolation and can be seen in action through Storybook: `npm run storybook`.
barklund
added
Type: Code Quality
barklund
commented
Nov 16, 2022
added 3 commits
November 16, 2022 20:23
# Conflicts: # packages/animation/src/components/AMPKeyframes.tsx # packages/animation/src/components/context.ts # packages/animation/src/components/provider.tsx # packages/animation/src/components/useStoryAnimationContext.ts # packages/animation/src/effects/backgroundPan/animationProps.js # packages/animation/src/effects/fadeIn.ts # packages/animation/src/effects/flyIn/animationProps.js # packages/animation/src/effects/pulse.ts # packages/animation/src/effects/zoom/animationProps.js # packages/animation/src/parts/defaultFields.ts # packages/animation/src/parts/simpleAnimation.js # packages/animation/src/parts/test/index.js # packages/animation/src/parts/types.js # packages/animation/src/types.js # packages/animation/src/types/index.ts # packages/animation/src/types/types.js # packages/elements/src/utils/index.ts # packages/media/src/types/mediaElement.ts
swissspidy
reviewed
Nov 17, 2022
added 17 commits
November 17, 2022 19:22
A bunch of anim constants have been converted from object to enums and thus had their casing updating from `SCREAMING_SNAKE_CASE` to `PascalCase`.
…-stories-wp into code/12207-ts-anim # Conflicts: # packages/story-editor/src/components/panels/design/animation/animation.js
Thís required creating a union type for all the animations. However, it works a little weird, so had to do a bunch of `as const` stuff in tests, so worry about typing the implementation of this in other packages, but for now this is good.
Yes, `panDirection` would be better, but it is already referenced a ton of places, so it is a lot of unnecessary work to change it.
barklund
changed the title
animation packageanimation package
|
Plugin builds for f864b46 are ready 🛎️!
|
|
Size Change: -15.2 kB (-1%) Total Size: 2.71 MB
ℹ️ View Unchanged
|
added 5 commits
November 24, 2022 14:52
Dunno why eslint complains, but maybe it is the missing header?
miina
reviewed
Nov 24, 2022
12 tasks
miina
approved these changes
Nov 28, 2022
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.
Context
This was a big one 😅
The
animationpackage is quite over-engineered and has a serious lack of DRY-ness with a lot of duplicated code, boilerplate and even logic. This is still only a partial conversion.At this stage, I would welcome a sanity check, but I actually feel pretty good about the conversion, because I can see all the animations working in Storybook (except for background effects, see todo).
To-do
Need-to-have
animationandelements, soelementsnow depend onanimationtsconfig)createAnimation(requires makingStoryAnimationa union type with all the properties specified for each different animation type)Bugs in editor
Nice-to-have
/parts/to/parts/simple/and move effects to/parts/effects/. They're all only ever referenced by the functions inside/parts/index.tsxanyway (which will be split up above)./parts/index.tsxinto dedicated files, and add tests (esp. forgetAnimationEffectFields)User-facing changes
None!
Testing Instructions
Test everything related to animations including old stories with animation, creating new stories with new animations, previewing animations in the editor, viewing animations in the output story.
Checklist
Type: XYZlabel to the PRFixes #12207