From e715145d866fe25652fa3384684ec8e035f1d825 Mon Sep 17 00:00:00 2001 From: mister-ben <1676039+mister-ben@users.noreply.github.com> Date: Tue, 13 Aug 2024 10:54:53 +0200 Subject: [PATCH] fix: Change requestNamedAnimationFrame to apply last change per frame instead of first (#8799) ## Description The current implementation of `requestNamedAnimationFrame` prevents multiple updates on a frame but by disregarding all but the first request per frame. This throttling behaviour is apparent when playing a very short video - if a `timeupdate` occurs just before the `ended` event, the progress bar position on the `timeupdate` is set at say 98% and 100% from the `ended` is discarded. Although #8633 removed the throttle from the `ended` handler itself, the throttle and non-throttled update can still both execute between frames. ## Specific Changes proposed Changes the implementation to apply only the last callback instead. If any exist they will be cancelled. There will still be only one update, but now it's the last. Updates tests to reflect the changed behaviour. Fixes #8782 ## Requirements Checklist - [x] Feature implemented / Bug fixed - [ ] If necessary, more likely in a feature request than a bug fix - [x] Change has been verified in an actual browser (Chrome, Firefox, IE) - [x] Unit Tests updated or fixed - [ ] Docs/guides updated - [ ] Example created ([starter template on JSBin](https://codepen.io/gkatsev/pen/GwZegv?editors=1000#0)) - [x] Has no DOM changes which impact accessiblilty or trigger warnings (e.g. Chrome issues tab) - [x] Has no changes to JSDoc which cause `npm run docs:api` to error - [ ] Reviewed by Two Core Contributors --- src/js/component.js | 2 +- test/unit/component.test.js | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/js/component.js b/src/js/component.js index b64e19c41..4340eed55 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -1724,7 +1724,7 @@ class Component { */ requestNamedAnimationFrame(name, fn) { if (this.namedRafs_.has(name)) { - return; + this.cancelNamedAnimationFrame(name); } this.clearTimersOnDispose_(); diff --git a/test/unit/component.test.js b/test/unit/component.test.js index 903dc8f42..2b384bcd9 100644 --- a/test/unit/component.test.js +++ b/test/unit/component.test.js @@ -1377,7 +1377,7 @@ QUnit.test('requestNamedAnimationFrame should only allow one raf of a specific n comp.requestNamedAnimationFrame(name, handlerTwo); - assert.deepEqual(cancelNames, [], 'no named cancels'); + assert.deepEqual(cancelNames, ['testing'], 'one handler was cancelled'); assert.equal(comp.namedRafs_.size, 1, 'still only one named raf'); assert.equal(comp.rafIds_.size, 1, 'still only one raf id'); @@ -1386,16 +1386,16 @@ QUnit.test('requestNamedAnimationFrame should only allow one raf of a specific n assert.equal(comp.namedRafs_.size, 0, 'we removed a named raf'); assert.equal(comp.rafIds_.size, 0, 'we removed a raf id'); assert.deepEqual(calls, { - one: 1, - two: 0, + one: 0, + two: 1, three: 0 - }, 'only handlerOne was called'); + }, 'only handlerTwo was called'); comp.requestNamedAnimationFrame(name, handlerOne); comp.requestNamedAnimationFrame(name, handlerTwo); comp.requestNamedAnimationFrame(name, handlerThree); - assert.deepEqual(cancelNames, [], 'no named cancels for testing'); + assert.deepEqual(cancelNames, ['testing', 'testing', 'testing'], 'two more cancels'); assert.equal(comp.namedRafs_.size, 1, 'only added one named raf'); assert.equal(comp.rafIds_.size, 1, 'only added one named raf'); @@ -1404,10 +1404,10 @@ QUnit.test('requestNamedAnimationFrame should only allow one raf of a specific n assert.equal(comp.namedRafs_.size, 0, 'we removed a named raf'); assert.equal(comp.rafIds_.size, 0, 'we removed a raf id'); assert.deepEqual(calls, { - one: 2, - two: 0, - three: 0 - }, 'only the handlerOne called'); + one: 0, + two: 1, + three: 1 + }, 'now handlerThree has also been called'); window.requestAnimationFrame = oldRAF; window.cancelAnimationFrame = oldCAF;