From 0b1e23f7540f8e59fb53c02d3b18b05e1540a6cd Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 13 Aug 2019 14:46:40 -0400 Subject: [PATCH] perf(animation): remove display: none check (#19086) * wrap offsetParent in raf * Revert "wrap offsetParent in raf" merge This reverts commit 9910032964b01d9b58b3b1a199fce52853089257. * remove display none check, add note to document --- core/src/utils/animation/animation.ts | 18 +++++----- core/src/utils/animation/test/display/e2e.ts | 37 ++++++-------------- 2 files changed, 18 insertions(+), 37 deletions(-) diff --git a/core/src/utils/animation/animation.ts b/core/src/utils/animation/animation.ts index 3a548e6787..95a0b15b7e 100644 --- a/core/src/utils/animation/animation.ts +++ b/core/src/utils/animation/animation.ts @@ -923,22 +923,20 @@ export const createAnimation = () => { } }); - const animationDelay = getDelay() || 0; - const animationDuration = getDuration() || 0; - const visibleElements = elements.filter(element => element.offsetParent !== null); - if (visibleElements.length === 0 || _keyframes.length === 0 || elements.length === 0) { + if (_keyframes.length === 0 || elements.length === 0) { + animationFinish(); + } else { /** + * This is a catchall in the event that a CSS Animation did not finish. + * The Web Animations API has mechanisms in place for preventing this. * CSS Animations will not fire an `animationend` event * for elements with `display: none`. The Web Animations API * accounts for this, but using raw CSS Animations requires * this workaround. */ - animationFinish(); - } else if (_keyframes.length > 0 && elements.length > 0) { - /** - * This is a catchall in the event that a CSS Animation did not finish. - * The Web Animations API has mechanisms in place for preventing this. - */ + const animationDelay = getDelay() || 0; + const animationDuration = getDuration() || 0; + cssAnimationsTimerFallback = setTimeout(onAnimationEndFallback, animationDelay + animationDuration + ANIMATION_END_FALLBACK_PADDING_MS); animationEnd(elements[0], () => { diff --git a/core/src/utils/animation/test/display/e2e.ts b/core/src/utils/animation/test/display/e2e.ts index 71299ee1de..1b98bb8a48 100644 --- a/core/src/utils/animation/test/display/e2e.ts +++ b/core/src/utils/animation/test/display/e2e.ts @@ -4,6 +4,15 @@ import { listenForEvent, waitForFunctionTestContext } from '../../../test/utils' test(`animation:web: display`, async () => { const page = await newE2EPage({ url: '/src/utils/animation/test/display' }); + await runTest(page); +}); + +test(`animation:css: display`, async () => { + const page = await newE2EPage({ url: '/src/utils/animation/test/display?ionic:_forceCSSAnimations=true' }); + await runTest(page); +}); + +const runTest = async (page: any) => { const screenshotCompares = []; screenshotCompares.push(await page.compareScreenshot()); @@ -25,30 +34,4 @@ test(`animation:web: display`, async () => { }, { animationStatus }); screenshotCompares.push(await page.compareScreenshot()); -}); - -test(`animation:css: display`, async () => { - const page = await newE2EPage({ url: '/src/utils/animation/test/display?ionic:_forceCSSAnimations=true' }); - const screenshotCompares = []; - - screenshotCompares.push(await page.compareScreenshot()); - - const ANIMATION_FINISHED = 'onIonAnimationFinished'; - const animationStatus = []; - await page.exposeFunction(ANIMATION_FINISHED, (ev: any) => { - animationStatus.push(ev.detail); - }); - - const squareA = await page.$('.square-a'); - await listenForEvent(page, 'ionAnimationFinished', squareA, ANIMATION_FINISHED); - - await page.click('.play'); - await page.waitForSelector('.play'); - - await waitForFunctionTestContext((payload: any) => { - // CSS Animations do not account for elements with `display: none` very well, so we need to add a workaround for this. - return payload.animationStatus.join(', ') === ['AnimationAFinished', 'AnimationBFinished', 'AnimationRootFinished'].join(', '); - - }, { animationStatus }); - screenshotCompares.push(await page.compareScreenshot()); -}); +};