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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(animations): fix stagger timing not handling params #47233

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dario-piotrowicz
Copy link
Contributor

prior to this change the stagger timing was being built during the
ast building instead of dynamically when visiting the stagger animation,
thus it could not handle params correctly, this change makes it so that
during ast building a timing ast is built instead and that ast is used
dynammically to build animations which can handle params correctly

(this PR reinstates the changes done in #47208 which have been reverted
because negative non-parametrized stagger values were not handled
correctly, alongside the original changes the current commit also
handles corretly negative non-parametrized values and improves unit
testing)

resolves #19786

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #19786

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@dario-piotrowicz dario-piotrowicz changed the title fix(animations): fix stagger timing not handling params (#47208) fix(animations): fix stagger timing not handling params Aug 23, 2022
prior to this change the stagger timing was being built during the
ast building instead of dynamically when visiting the stagger animation,
thus it could not handle params correctly, this change makes it so that
during ast building a timing ast is built instead and that ast is used
dynammically to build animations which can handle params correctly

(this PR reinstates the changes done in angular#47208 which have been reverted
because negative non-parametrized stagger values were not handled
correctly, alongside the original changes the current commit also
handles corretly negative non-parametrized values and improves unit
testing)

resolves angular#19786
@jessicajaniuk jessicajaniuk added area: animations action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Aug 23, 2022
@ngbot ngbot bot modified the milestone: Backlog Aug 23, 2022
Copy link
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

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

LGTM 馃崻

We'll want to run a TGP just to be safe.

@jessicajaniuk jessicajaniuk added the action: global presubmit The PR is in need of a google3 global presubmit label Aug 23, 2022
@dario-piotrowicz
Copy link
Contributor Author

LGTM cookie

We'll want to run a TGP just to be safe.

yeah of course 馃槄馃憤

@Flo0806
Copy link

Flo0806 commented Nov 15, 2022

Hello!
Thx @dario-piotrowicz!
I hope the feature will come soon in Angular!

Greetings, Flo

@Flo0806
Copy link

Flo0806 commented Mar 5, 2024

@jessicajaniuk @dario-piotrowicz Hello!

Will this feature coming soon, or is it histroy? I see the pull requests fails?

Thx and greetings,
Florian

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: global presubmit The PR is in need of a google3 global presubmit action: review The PR is still awaiting reviews from at least one requested reviewer area: animations target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parameters inside stagger() don't work
3 participants