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

refactor(animations): improve unit tests for stagger timing #47221

Conversation

dario-piotrowicz
Copy link
Contributor

improve the stagger timing unit tests added in #47208 by also checking
that the duration and keyframes of the players are correct

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: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@pullapprove pullapprove bot requested a review from dylhunn August 22, 2022 20:54
const animationDuration = 700;
const absoluteStaggerDelay = 500;

@Component({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this gets wrongly indented by the formatter 馃槗

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok I managed to fix it by shortening the it text 馃檪

improve the stagger timing unit tests added in angular#47208 by also checking
that the duration and keyframes of the players are correct
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 馃崻

Thanks, @dario-piotrowicz!

@jessicajaniuk jessicajaniuk requested review from jessicajaniuk and removed request for dylhunn and jessicajaniuk August 22, 2022 21:01
@jessicajaniuk jessicajaniuk added action: merge The PR is ready for merge by the caretaker area: animations target: patch This PR is targeted for the next patch release labels Aug 22, 2022
@ngbot ngbot bot modified the milestone: Backlog Aug 22, 2022
@dario-piotrowicz
Copy link
Contributor Author

@jessicajaniuk I'm closing the PR as I will have to create a new one to fix the stagger issue you've found, so I guess it just makes more sense for me to include the updated unit tests in my next PR 馃檪

@angular-automatic-lock-bot
Copy link

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 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker 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.

None yet

2 participants