-
Notifications
You must be signed in to change notification settings - Fork 75
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
refactor(dom): consolidate transition/animation waiting utils #9341
base: main
Are you sure you want to change the base?
Conversation
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions. |
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.
Almost there!
) => void; | ||
|
||
beforeEach(() => { | ||
// we clobber Stencil's custom Mock document implementation |
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.
🔨
|
||
element.style.transition = testTransition; | ||
|
||
// need to mock due to JSDOM issue with getComputedStyle - https://github.com/jsdom/jsdom/issues/3090 |
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.
🎭
* @param targetEl The element to watch for the transition to complete. | ||
* @param transitionProp The name of the transition to watch for completion. | ||
*/ | ||
export async function whenTransitionDone(targetEl: HTMLElement, transitionProp: string): Promise<void> { |
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.
Can't we just use onTransitionEnd
on a VNode in most cases? Should we specify that?
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 think we can tweak the name and doc to better explain its usage.
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.
👍
Related Issue: N/A
Summary
Stems from #9325 (comment).
Consolidates transition/animation waiting logic and adds
whenTransitionDone
to DOM utils to complementwhenAnimationDone
.