Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: Change requestNamedAnimationFrame to apply last change per frame instead of first #8799

Merged
merged 4 commits into from
Aug 13, 2024
Merged

fix: Change requestNamedAnimationFrame to apply last change per frame instead of first #8799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
138d84d
fix: Change named to apply last change per frame instead of first
mister-ben Jul 14, 2024
0799617
Delete comments
mister-ben Jul 14, 2024
dfc7ee0
Merge branch 'main' into fix-seek-throttle
mister-ben Jul 15, 2024
17eb87e
Merge branch 'main' into fix-seek-throttle
mister-ben Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/js/component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1714,7 +1714,7 @@ class Component {
*/
requestNamedAnimationFrame(name, fn) {
if (this.namedRafs_.has(name)) {
return;
this.cancelNamedAnimationFrame(name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this still return as well or do we now want to execute the code below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous, if a raf with that name is already queued, it returned and did not add the new raf of this name.

Now if the named raf is queued, instead of returning, that queued one is cancelled, then the new raf is added.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks!

}
this.clearTimersOnDispose_();

Expand Down
18 changes: 9 additions & 9 deletions test/unit/component.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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');

Expand All @@ -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;
Expand Down