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): use a lightweight renderer for non-animation components #17003
Conversation
9e61f8d
to
a6bf832
Compare
9ad9e1f
to
8f5b441
Compare
|
||
import {AnimationDriver} from './animation_driver'; | ||
import {getOrSetAsInMap, listenOnPlayer, makeAnimationEvent, normalizeKeyframes, optimizeGroupPlayer} from './shared'; | ||
|
||
const EMPTY_PLAYER_ARRAY: AnimationPlayer[] = []; | ||
const ANIMATE_EPOCH_ATTR = 'ng-animate-epoch'; |
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 a better name?
// cache the delegates to find out which cached delegate can | ||
// be used by which cached renderer | ||
const delegate = this.delegate.createRenderer(hostElement, type); | ||
let renderer: BaseAnimationRenderer|undefined = this._rendererCache.get(delegate); |
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.
move renderer
inside of the if
, as it is not needed anywhere else.
public destroyNode: ((node: any) => any)|null = null; | ||
public microtaskCount: number = 0; | ||
export class BaseAnimationRenderer implements Renderer2 { | ||
public namespaceId: string = ''; |
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.
take the namespaceId
in the constructor, and make it protected
.
this.delegate.removeStyle(el, style, flags); | ||
} | ||
|
||
setProperty(el: any, name: string, value: any): void { | ||
if (name.charAt(0) != '@') { |
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.
I think this check is not needed...
this.delegate.appendChild(parent, newChild); | ||
this._engine.onInsert(this._namespaceId, newChild, parent, false); | ||
listen(target: any, eventName: string, callback: (event: any) => boolean | void): () => void { | ||
if (eventName.charAt(0) == '@') { |
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.
Why is this logic needed for the base renderer?
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.
I.e. it probably should go into the regular AnimationRenderer
.
private delegate: RendererFactory2, private _engine: AnimationEngine, private _zone: NgZone) { | ||
_engine.onRemovalComplete = (element: any, delegate: any) => { | ||
private delegate: RendererFactory2, private engine: AnimationEngine, private _zone: NgZone) { | ||
engine.onRemovalComplete = (element: any, delegate: any) => { |
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.
type the delegate
to be a RendererV2
.
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.
Reviewed the changes to core and the renderer.
8f5b441
to
4200ab9
Compare
… components (angular#17003)" This reverts commit 3ab86bd.
…nts (angular#17003) This reverts commit c0981b8.
…nts (angular#17003) This reverts commit c0981b8.
… components (angular#17003)" This reverts commit 3ab86bd.
…nts (angular#17003) This reverts commit c0981b8.
… components (angular#17003)" This reverts commit 3ab86bd.
…nts (angular#17003) This reverts commit c0981b8.
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.