feat(ivy): introduce the `animate` pipe for styling bindings #26931

matsko · 2018-11-02T22:50:05Z

No description provided.

mary-poppins · 2018-11-02T23:02:04Z

You can preview 7f1a9be at https://pr26931-7f1a9be.ngbuilds.io/.

JoostK · 2018-11-03T15:20:03Z

packages/core/src/render3/animations/util.ts

+}
+
+export function now(): number {
+  return window ? window.performance && window.performance.now() : Date.now();


There's a bug here, should be:

Suggested change

return window ? window.performance && window.performance.now() : Date.now();

return window && window.performance && window.performance.now() || Date.now();

JoostK · 2018-11-03T15:21:49Z

packages/core/src/render3/styling/util.ts

-          playerContext[index] = null;
-        } else {
-          playerContext.splice(index, 1);
+    player.status.subscribe(state => {


This subscription is not unsubscribed from, nor is the status Subject being completed, so this will probably leak by the looks of it.

JoostK · 2018-11-03T15:28:37Z

packages/core/src/render3/animations/util.ts

+export function computeStyle(element: HTMLElement, prop: string): string {
+  if (!window || !window.getComputedStyle) return '';
+  const gcs = window.getComputedStyle(element);
+  let val = (gcs as any)[prop];


nit:

Suggested change

let val = (gcs as any)[prop];

const val = (gcs as any)[prop] || gcs.getPropertyValue(prop);

JoostK · 2018-11-03T15:30:37Z

packages/core/src/render3/animations/css_transition_animator.ts

+    const bool = classes[className];
+    element.classList.toggle(className, revert ? !bool : bool);
+    if (store) {
+      store[className] = revert ? false : true;


nit:

Suggested change

store[className] = revert ? false : true;

store[className] = !revert;

JoostK · 2018-11-03T15:32:44Z

packages/core/src/render3/animations/css_transition_animator.ts

+
+function applyStyleChanges(
+    element: HTMLElement, styles: {[key: string]: any}, backupStyles: {[key: string]: any} | null,
+    revert?: boolean, store?: {[key: string]: any} | null) {


nit: what I think is strange is that backupStyles is not optional, whereas its read is conditional on the optional revert param being true.

packages/core/src/render3/animations/util.ts

packages/core/src/render3/animations/animate_pipe.ts

packages/core/src/render3/animations/css_transition_animator.ts

packages/core/src/render3/animations/interfaces.ts

JoostK · 2018-11-03T15:54:06Z

packages/core/src/render3/animations/interfaces.ts

+  scheduleFlush(): void;
+  flushEffects(): boolean;
+  destroy(): void;
+  onAllEffectsDone(cb: () => any): void;


Suggested change

onAllEffectsDone(cb: () => any): void;

onAllEffectsDone(cb: () => void): void;

This is needed so you can do something like onAllEffectsDone(x => foo()); (incase foo() returns something ... otherwise typings would fail)

It is perfectly fine to use a function with a non-void return type in place of a () => void function (e.g. see microsoft/TypeScript#20006 (comment)).
(There are discussions about changing that, but I don't see it as likely in the near future - famous last words.)

If you wanted to be fancy (and safe), you could use () => unknown 😛

mary-poppins · 2018-11-06T09:24:47Z

You can preview 1533bc2 at https://pr26931-1533bc2.ngbuilds.io/.

googlebot · 2018-11-06T23:36:19Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

mary-poppins · 2018-11-06T23:40:27Z

You can preview 6a548fe at https://pr26931-6a548fe.ngbuilds.io/.

googlebot · 2018-11-07T05:22:59Z

CLAs look good, thanks!

packages/core/src/render3/animations/util.ts

packages/core/src/render3/animations/animate_pipe.ts

packages/core/src/render3/animations/css_transition_animator.ts

packages/core/test/render3/animations/animate_pipe_spec.ts

mary-poppins · 2018-11-11T15:55:19Z

You can preview a6789ad at https://pr26931-a6789ad.ngbuilds.io/.

mary-poppins · 2018-12-05T21:02:03Z

You can preview dd02c54 at https://pr26931-dd02c54.ngbuilds.io/.

mary-poppins · 2018-12-05T21:02:42Z

You can preview 13fe0dc at https://pr26931-13fe0dc.ngbuilds.io/.

mary-poppins · 2018-12-05T21:02:47Z

You can preview df73dbf at https://pr26931-df73dbf.ngbuilds.io/.

mary-poppins · 2018-12-05T21:24:20Z

You can preview 8fdff6c at https://pr26931-8fdff6c.ngbuilds.io/.

mary-poppins · 2018-12-05T22:03:59Z

You can preview 1d1e08d at https://pr26931-1d1e08d.ngbuilds.io/.

mary-poppins · 2018-12-05T22:07:00Z

You can preview 67ce48f at https://pr26931-67ce48f.ngbuilds.io/.

mary-poppins · 2018-12-05T22:34:08Z

You can preview 8f0c799 at https://pr26931-8f0c799.ngbuilds.io/.

mary-poppins · 2018-12-06T15:18:24Z

You can preview f1d1d6b at https://pr26931-f1d1d6b.ngbuilds.io/.

mary-poppins · 2018-12-07T19:09:14Z

You can preview bbe62d3 at https://pr26931-bbe62d3.ngbuilds.io/.

mary-poppins · 2018-12-11T19:01:20Z

You can preview 6826828 at https://pr26931-6826828.ngbuilds.io/.

mary-poppins · 2018-12-20T17:28:01Z

You can preview d684d5d at https://pr26931-d684d5d.ngbuilds.io/.

mohammedzamakhan · 2019-04-24T14:26:59Z

@matsko I was excited about animate pipe, but don't see it landing in labs too :(

angular-automatic-lock-bot · 2019-09-15T08:20:52Z

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

googlebot added the cla: yes label Nov 2, 2018

matsko force-pushed the animate_pipe branch 3 times, most recently from 39d8bef to 8cd7e9b Compare November 2, 2018 22:53

matsko added the target: major This PR is targeted for the next major release label Nov 2, 2018

JoostK requested changes Nov 3, 2018

View reviewed changes

kara added the comp: ivy label Nov 5, 2018

ngbot bot added this to the needsTriage milestone Nov 5, 2018

matsko force-pushed the animate_pipe branch 2 times, most recently from 80c3b59 to 1533bc2 Compare November 6, 2018 09:20

matsko force-pushed the animate_pipe branch from 1533bc2 to feed596 Compare November 6, 2018 21:58

googlebot added cla: no and removed cla: yes labels Nov 6, 2018

matsko force-pushed the animate_pipe branch from 6a548fe to e2e5dc2 Compare November 7, 2018 05:22

googlebot added cla: yes and removed cla: no labels Nov 7, 2018

trotyl reviewed Nov 8, 2018

View reviewed changes

matsko force-pushed the animate_pipe branch 5 times, most recently from 98032a6 to a6789ad Compare November 11, 2018 15:51

matsko force-pushed the animate_pipe branch 2 times, most recently from 6f83273 to 30d7ce1 Compare November 11, 2018 18:30

matsko force-pushed the animate_pipe branch from df73dbf to 8fdff6c Compare December 5, 2018 21:19

matsko force-pushed the animate_pipe branch 2 times, most recently from 1d1e08d to 67ce48f Compare December 5, 2018 22:01

matsko force-pushed the animate_pipe branch from 67ce48f to 8f0c799 Compare December 5, 2018 22:29

matsko force-pushed the animate_pipe branch from 8f0c799 to f1d1d6b Compare December 6, 2018 15:14

matsko force-pushed the animate_pipe branch from f1d1d6b to bbe62d3 Compare December 7, 2018 19:05

matsko force-pushed the animate_pipe branch from bbe62d3 to 6826828 Compare December 11, 2018 18:56

matsko force-pushed the animate_pipe branch from 6826828 to d684d5d Compare December 20, 2018 17:23

matsko mentioned this pull request Jan 10, 2019

Animation on IOS chrome #27506

Closed

matsko force-pushed the animate_pipe branch from d684d5d to 40b1797 Compare June 10, 2019 15:29

matsko requested review from a team as code owners June 10, 2019 15:29

feat(ivy): introduce the animate pipe for styling bindings

cafe11e

matsko force-pushed the animate_pipe branch from 40b1797 to cafe11e Compare June 10, 2019 15:51

matsko closed this Jul 19, 2019

matsko deleted the animate_pipe branch July 19, 2019 17:44

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ivy): introduce the `animate` pipe for styling bindings #26931

feat(ivy): introduce the `animate` pipe for styling bindings #26931

matsko commented Nov 2, 2018

mary-poppins commented Nov 2, 2018

JoostK Nov 3, 2018

JoostK Nov 3, 2018

JoostK Nov 3, 2018

JoostK Nov 3, 2018

JoostK Nov 3, 2018

JoostK Nov 3, 2018

matsko Nov 6, 2018

gkalpak Nov 6, 2018 •

edited

Loading

mary-poppins commented Nov 6, 2018

googlebot commented Nov 6, 2018

mary-poppins commented Nov 6, 2018

googlebot commented Nov 7, 2018

mary-poppins commented Nov 11, 2018

mary-poppins commented Dec 5, 2018

mary-poppins commented Dec 5, 2018

mary-poppins commented Dec 5, 2018

mary-poppins commented Dec 5, 2018

mary-poppins commented Dec 5, 2018

mary-poppins commented Dec 5, 2018

mary-poppins commented Dec 5, 2018

mary-poppins commented Dec 6, 2018

mary-poppins commented Dec 7, 2018

mary-poppins commented Dec 11, 2018

mary-poppins commented Dec 20, 2018

mohammedzamakhan commented Apr 24, 2019

angular-automatic-lock-bot bot commented Sep 15, 2019

	return window ? window.performance && window.performance.now() : Date.now();
	return window && window.performance && window.performance.now() \|\| Date.now();

	let val = (gcs as any)[prop];
	const val = (gcs as any)[prop] \|\| gcs.getPropertyValue(prop);

	store[className] = revert ? false : true;
	store[className] = !revert;

	onAllEffectsDone(cb: () => any): void;
	onAllEffectsDone(cb: () => void): void;

feat(ivy): introduce the animate pipe for styling bindings #26931

feat(ivy): introduce the animate pipe for styling bindings #26931

Conversation

matsko commented Nov 2, 2018

mary-poppins commented Nov 2, 2018

JoostK Nov 3, 2018

Choose a reason for hiding this comment

JoostK Nov 3, 2018

Choose a reason for hiding this comment

JoostK Nov 3, 2018

Choose a reason for hiding this comment

JoostK Nov 3, 2018

Choose a reason for hiding this comment

JoostK Nov 3, 2018

Choose a reason for hiding this comment

JoostK Nov 3, 2018

Choose a reason for hiding this comment

matsko Nov 6, 2018

Choose a reason for hiding this comment

gkalpak Nov 6, 2018 • edited Loading

Choose a reason for hiding this comment

mary-poppins commented Nov 6, 2018

googlebot commented Nov 6, 2018

mary-poppins commented Nov 6, 2018

googlebot commented Nov 7, 2018

mary-poppins commented Nov 11, 2018

mary-poppins commented Dec 5, 2018

mary-poppins commented Dec 5, 2018

mary-poppins commented Dec 5, 2018

mary-poppins commented Dec 5, 2018

mary-poppins commented Dec 5, 2018

mary-poppins commented Dec 5, 2018

mary-poppins commented Dec 5, 2018

mary-poppins commented Dec 6, 2018

mary-poppins commented Dec 7, 2018

mary-poppins commented Dec 11, 2018

mary-poppins commented Dec 20, 2018

mohammedzamakhan commented Apr 24, 2019

angular-automatic-lock-bot bot commented Sep 15, 2019

feat(ivy): introduce the `animate` pipe for styling bindings #26931

feat(ivy): introduce the `animate` pipe for styling bindings #26931

gkalpak Nov 6, 2018 •

edited

Loading