Skip to content

fix(animations): use a lightweight renderer for non-animation components - #17003

Merged
chuckjaz merged 1 commit into
angular:masterfrom
matsko:lightweight_renderer
May 26, 2017
Merged

fix(animations): use a lightweight renderer for non-animation components#17003
chuckjaz merged 1 commit into
angular:masterfrom
matsko:lightweight_renderer

Conversation

@matsko

@matsko matsko commented May 24, 2017

Copy link
Copy Markdown
Contributor

No description provided.

@mary-poppins

mary-poppins commented May 24, 2017

Copy link
Copy Markdown

The angular.io preview for 9e61f8d12dd5145b36a27ab855abf3fed3934604 is available here.

@chuckjaz chuckjaz added the area: animations legacy animations package only. Otherwise use area: core. label May 24, 2017
@matsko
matsko force-pushed the lightweight_renderer branch from 9e61f8d to a6bf832 Compare May 25, 2017 17:27
@mary-poppins

mary-poppins commented May 25, 2017

Copy link
Copy Markdown

The angular.io preview for a6bf832a1bcee1cd2af8815cefc59c3cb2a12e0b is available here.

@matsko
matsko force-pushed the lightweight_renderer branch 2 times, most recently from 9ad9e1f to 8f5b441 Compare May 25, 2017 20:56
@mary-poppins

mary-poppins commented May 25, 2017

Copy link
Copy Markdown

The angular.io preview for 9ad9e1f39472238ad468bc717e02d840106731ac is available here.

@mary-poppins

mary-poppins commented May 25, 2017

Copy link
Copy Markdown

The angular.io preview for 8f5b441887b30b19be5ecaeede513d1c28c8453b is available here.

@matsko matsko changed the title WIP: fix(animations): use a lightweight renderer for non-animation components fix(animations): use a lightweight renderer for non-animation components May 25, 2017
tbosch
tbosch reviewed May 25, 2017
View reviewed changes
Comment thread packages/animations/browser/src/render/transition_animation_engine.ts Outdated

@tbosch tbosch May 25, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe a better name?

tbosch
tbosch reviewed May 25, 2017
View reviewed changes
Comment thread packages/platform-browser/animations/src/animation_renderer.ts Outdated

@tbosch tbosch May 25, 2017

Copy link
Copy Markdown
Contributor

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.

tbosch
tbosch reviewed May 25, 2017
View reviewed changes
Comment thread packages/platform-browser/animations/src/animation_renderer.ts Outdated

@tbosch tbosch May 25, 2017

Copy link
Copy Markdown
Contributor

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.

tbosch
tbosch reviewed May 25, 2017
View reviewed changes
Comment thread packages/platform-browser/animations/src/animation_renderer.ts Outdated

@tbosch tbosch May 25, 2017

Copy link
Copy Markdown
Contributor

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...

tbosch
tbosch reviewed May 25, 2017
View reviewed changes
Comment thread packages/platform-browser/animations/src/animation_renderer.ts Outdated

@tbosch tbosch May 25, 2017

Copy link
Copy Markdown
Contributor

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?

@tbosch tbosch May 25, 2017

Copy link
Copy Markdown
Contributor

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.

tbosch
tbosch reviewed May 25, 2017
View reviewed changes
Comment thread packages/platform-browser/animations/src/animation_renderer.ts Outdated

@tbosch tbosch May 25, 2017

Copy link
Copy Markdown
Contributor

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.

tbosch
tbosch approved these changes May 25, 2017
View reviewed changes

@tbosch tbosch left a comment

Copy link
Copy Markdown
Contributor

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.

@matsko
matsko force-pushed the lightweight_renderer branch from 8f5b441 to 4200ab9 Compare May 25, 2017 23:44
@mary-poppins

mary-poppins commented May 25, 2017

Copy link
Copy Markdown

The angular.io preview for 4200ab9 is available here.

@chuckjaz chuckjaz added the action: merge The PR is ready for merge by the caretaker label May 26, 2017
@chuckjaz
chuckjaz merged commit 3ab86bd into angular:master May 26, 2017
chuckjaz added a commit to chuckjaz/angular that referenced this pull request May 26, 2017
@chuckjaz
Revert "fix(animations): use a lightweight renderer for non-animation…
c0981b8 
… components (angular#17003)"

This reverts commit 3ab86bd.
matsko added a commit to matsko/angular that referenced this pull request May 26, 2017
@matsko
fix(animations): use a lightweight renderer for non-animation compone…
8b35b6d 
…nts (angular#17003)

This reverts commit c0981b8.
chuckjaz pushed a commit that referenced this pull request May 26, 2017
@matsko @chuckjaz
fix(animations): use a lightweight renderer for non-animation compone…
d108479 
…nts (#17003)

This reverts commit c0981b8.
asnowwolf pushed a commit to asnowwolf/angular that referenced this pull request Aug 11, 2017
@matsko Zhicheng Wang
fix(animations): use a lightweight renderer for non-animation compone…
36b1033 
…nts (angular#17003)
asnowwolf pushed a commit to asnowwolf/angular that referenced this pull request Aug 11, 2017
@chuckjaz Zhicheng Wang
Revert "fix(animations): use a lightweight renderer for non-animation…
c27cb83 
… components (angular#17003)"

This reverts commit 3ab86bd.
asnowwolf pushed a commit to asnowwolf/angular that referenced this pull request Aug 11, 2017
@matsko Zhicheng Wang
fix(animations): use a lightweight renderer for non-animation compone…
9a36e0b 
…nts (angular#17003)

This reverts commit c0981b8.
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
@matsko @juleskremer
fix(animations): use a lightweight renderer for non-animation compone…
6e1315a 
…nts (angular#17003)
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
@chuckjaz @juleskremer
Revert "fix(animations): use a lightweight renderer for non-animation…
df4a1d8 
… components (angular#17003)"

This reverts commit 3ab86bd.
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
@matsko @juleskremer
fix(animations): use a lightweight renderer for non-animation compone…
a1abfdd 
…nts (angular#17003)

This reverts commit c0981b8.
@matsko
matsko deleted the lightweight_renderer branch January 17, 2019 19:42
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 14, 2019

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers
@mhevery mhevery mhevery approved these changes
@tbosch tbosch tbosch approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: animations legacy animations package only. Otherwise use area: core. cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@matsko @mary-poppins @mhevery @tbosch @chuckjaz @googlebot