-
Notifications
You must be signed in to change notification settings - Fork 25.4k
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
fix(animations): ensure animations work with web-workers #12656
Conversation
@matsko please add labels to your PRs |
725a0e4
to
1a27d6e
Compare
1a27d6e
to
57fd9bc
Compare
57fd9bc
to
ab0ea10
Compare
ab0ea10
to
2e4b595
Compare
2e4b595
to
5d5697a
Compare
], | ||
this._animate.bind(this)); | ||
|
||
// |
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.
maybe move these into a method so that the grouping is clear, i.e. then you don't need this comment.
]); | ||
|
||
const player = new _AnimationWorkerRendererPlayer(this._rootRenderer, renderElement); | ||
this._rootRenderer.renderStore.store(player, playerId); |
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.
Where are you removing the animation players from the renderStore again?
renderer: Renderer, element: any, startingStyles: any, keyframes: any[], duration: number, | ||
delay: number, easing: string, playerId: any) { | ||
var player = renderer.animate(element, startingStyles, keyframes, duration, delay, easing); | ||
this._renderStore.store(player, playerId); |
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.
Where are you removing the animation player from the renderStore again (ui side)?
export class WebWorkerRenderNode { events: NamedEventEmitter = new NamedEventEmitter(); } | ||
export class WebWorkerRenderNode { | ||
events = new NamedEventEmitter(); | ||
animationPlayers = new AnimationPlayerEmitter(); |
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.
rename property to animationPlayerEvents
} | ||
|
||
|
||
@Component({ |
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.
Could you create 1 e2e test as well that does a simple animation with webworkers?
5d5697a
to
c23a32c
Compare
c23a32c
to
ada7407
Compare
ada7407
to
ef1779b
Compare
ef1779b
to
5029246
Compare
5029246
to
45cad2c
Compare
45cad2c
to
7f19ede
Compare
7f19ede
to
9c0438b
Compare
9c0438b
to
4ccadff
Compare
5326770
to
6865fa1
Compare
6865fa1
to
bf6fa94
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.