From 1db7fe1fcd53c7393997b8aa7bd06ab47ab874ec Mon Sep 17 00:00:00 2001 From: Robert Flack Date: Wed, 20 Dec 2023 04:56:17 +0000 Subject: [PATCH] Address review comments. --- src/proxy-animation.js | 7 ++++++- src/scroll-timeline-css.js | 20 +++++++++++--------- test/expected.txt | 8 ++++---- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/proxy-animation.js b/src/proxy-animation.js index d85dfc63..451f1921 100644 --- a/src/proxy-animation.js +++ b/src/proxy-animation.js @@ -37,7 +37,7 @@ function createReadyPromise(details) { details.readyPromise = new PromiseWrapper(); // Trigger the pending task on the next animation frame. requestAnimationFrame(() => { - const timelineTime = details.timeline.currentTime; + const timelineTime = details.timeline ? details.timeline.currentTime : null; if (timelineTime !== null) notifyReady(details); }); @@ -612,6 +612,11 @@ function renormalizeTiming() { } function notifyReady(details) { + // If the timeline has been changed, resolve the previous ready promise. + if (!details.timeline) { + details.readyPromise.resolve(details.proxy); + return; + } if (details.pendingTask == 'pause') { commitPendingPause(details); } else if (details.pendingTask == 'play') { diff --git a/src/scroll-timeline-css.js b/src/scroll-timeline-css.js index 60f412a3..39f1fd98 100644 --- a/src/scroll-timeline-css.js +++ b/src/scroll-timeline-css.js @@ -143,15 +143,17 @@ export function initCSSPolyfill() { window.addEventListener('animationstart', (evt) => { evt.target.getAnimations().filter(anim => anim.animationName === evt.animationName).forEach(anim => { const result = createScrollTimeline(anim, anim.animationName, evt.target); - // If the CSS Animation refers to a scroll or view timeline we need to proxy the animation instance. - if (result.timeline && !(anim instanceof ProxyAnimation)) { - const proxyAnimation = new ProxyAnimation(anim, result.timeline, result.animOptions); - anim.pause(); - proxyAnimation.play(); - } else { - // If the timeline was removed or the animation was already an instance of a proxy animation, - // invoke the set the timeline procedure on the existing animation. - anim.timeline = result.timeline; + if (result) { + // If the CSS Animation refers to a scroll or view timeline we need to proxy the animation instance. + if (result.timeline && !(anim instanceof ProxyAnimation)) { + const proxyAnimation = new ProxyAnimation(anim, result.timeline, result.animOptions); + anim.pause(); + proxyAnimation.play(); + } else { + // If the timeline was removed or the animation was already an instance of a proxy animation, + // invoke the set the timeline procedure on the existing animation. + anim.timeline = result.timeline; + } } }); }); diff --git a/test/expected.txt b/test/expected.txt index ee21004b..56bbdc98 100644 --- a/test/expected.txt +++ b/test/expected.txt @@ -79,10 +79,10 @@ FAIL /scroll-animations/css/animation-timeline-deferred.html Animation.timeline FAIL /scroll-animations/css/animation-timeline-deferred.html Animation.timeline returns null for inactive deferred timeline FAIL /scroll-animations/css/animation-timeline-deferred.html Animation.timeline returns null for inactive (overattached) deferred timeline FAIL /scroll-animations/css/animation-timeline-ignored.tentative.html Changing animation-timeline changes the timeline (sanity check) -FAIL /scroll-animations/css/animation-timeline-ignored.tentative.html animation-timeline ignored after setting timeline with JS (ScrollTimeline from JS) +PASS /scroll-animations/css/animation-timeline-ignored.tentative.html animation-timeline ignored after setting timeline with JS (ScrollTimeline from JS) FAIL /scroll-animations/css/animation-timeline-ignored.tentative.html animation-timeline ignored after setting timeline with JS (ScrollTimeline from CSS) -PASS /scroll-animations/css/animation-timeline-ignored.tentative.html animation-timeline ignored after setting timeline with JS (document timeline) -PASS /scroll-animations/css/animation-timeline-ignored.tentative.html animation-timeline ignored after setting timeline with JS (null) +FAIL /scroll-animations/css/animation-timeline-ignored.tentative.html animation-timeline ignored after setting timeline with JS (document timeline) +FAIL /scroll-animations/css/animation-timeline-ignored.tentative.html animation-timeline ignored after setting timeline with JS (null) FAIL /scroll-animations/css/animation-timeline-in-keyframe.html The animation-timeline property may not be used in keyframes PASS /scroll-animations/css/animation-timeline-multiple.html animation-timeline works with multiple timelines FAIL /scroll-animations/css/animation-timeline-none.html Animation with animation-timeline:none holds current time at zero @@ -957,4 +957,4 @@ FAIL /scroll-animations/view-timelines/view-timeline-sticky-block.html View time FAIL /scroll-animations/view-timelines/view-timeline-sticky-inline.html View timeline with sticky target, block axis. FAIL /scroll-animations/view-timelines/view-timeline-subject-size-changes.html View timeline with subject size change after the creation of the animation FAIL /scroll-animations/view-timelines/zero-intrinsic-iteration-duration.tentative.html Intrinsic iteration duration is non-negative -Passed 356 of 959 tests. +Passed 355 of 959 tests.