fix(animations): getAnimationStyle causes exceptions in older browsers by Serginho · Pull Request #29709 · angular/angular

Serginho · 2019-04-04T12:13:50Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

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: #24094

What is the new behavior?

Animation doesn't break in old browsers now

Does this PR introduce a breaking change?

Yes
No

Other information

matsko · 2020-02-28T23:14:10Z

@Serginho sorry this took us forever to get back to you on.

Could you add a test to this? You would need to export the function you modified and import it into https://github.com/angular/angular/blob/master/packages/animations/browser/test/render/css_keyframes/element_animation_style_handler_spec.ts

Then test by mocking out the a fake element a style property.

Serginho · 2020-03-01T18:53:09Z

@matsko I didn't do any additional test because getAnimationStyle is already tested, for example here:

angular/packages/animations/browser/test/render/css_keyframes/element_animation_style_handler_spec.ts

Lines 66 to 68 in 5bc39f8

    
           assertStyle(element, 'animation-name', 'fooAnimation'); 
        
           handler.apply(); 
        
           assertStyle(element, 'animation-name', 'fooAnimation, barAnimation');

So I assume you want a testcase for this bug, and this is what I did in the updated commit.
The fix is squashed, formatted, and rebased to master.

Let me know if you need something else.

petebacondarwin

LGTM - please can you add some more information to the commit message body?

petebacondarwin · 2020-11-10T15:53:29Z

@Serginho - would you mind rebasing this PR on top of master. Then I will try to chase reviewers to get it merged.

Serginho · 2020-11-17T00:01:05Z

@petebacondarwin Sure.

Rebased and ready to merge.

petebacondarwin

Thanks for rebasing @Serginho. Just one minor correction in the test.

packages/animations/browser/test/render/css_keyframes/element_animation_style_handler_spec.ts

PR angular#29709 getAnimationStyle causes exceptions in older browsers

petebacondarwin

Great! Thanks.

AndrewKushnir · 2020-11-18T16:46:52Z

Presubmit.

packages/animations/browser/src/render/css_keyframes/element_animation_style_handler.ts

#29709) PR #29709 getAnimationStyle causes exceptions in older browsers PR Close #29709

AndrewKushnir · 2020-11-20T16:33:34Z

@Serginho this PR is now merged, thanks for contributing to Angular!

angular-automatic-lock-bot · 2020-12-21T16:40:23Z

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

Serginho requested a review from a team April 4, 2019 12:13

googlebot added the cla: yes label Apr 4, 2019

Serginho force-pushed the fix_getAnimation_style branch from 6a6e992 to 94d1810 Compare April 4, 2019 12:15

jasonaden added the area: animations label Apr 4, 2019

ngbot bot added this to the needsTriage milestone Apr 4, 2019

BojanKogoj mentioned this pull request Nov 18, 2019

getAnimationStyle causes exceptions in older browsers #24094

Closed

kara added 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 Feb 28, 2020

pullapprove bot requested a review from matsko February 28, 2020 22:48

kara removed the request for review from a team February 28, 2020 22:49

Serginho force-pushed the fix_getAnimation_style branch 2 times, most recently from fb2f0a8 to bd07373 Compare March 1, 2020 18:37

Serginho force-pushed the fix_getAnimation_style branch 3 times, most recently from 6f9210a to 9625229 Compare March 1, 2020 19:34

petebacondarwin requested review from petebacondarwin and removed request for matsko August 20, 2020 18:38

pullapprove bot requested a review from crisbeto August 20, 2020 18:38

petebacondarwin approved these changes Aug 31, 2020

View reviewed changes

petebacondarwin added the action: presubmit The PR is in need of a google3 presubmit label Nov 10, 2020

Serginho force-pushed the fix_getAnimation_style branch from 9625229 to 8865d0c Compare November 16, 2020 23:25

Serginho force-pushed the fix_getAnimation_style branch from 8865d0c to ba35a0f Compare November 16, 2020 23:39

petebacondarwin suggested changes Nov 17, 2020

View reviewed changes

packages/animations/browser/test/render/css_keyframes/element_animation_style_handler_spec.ts Outdated Show resolved Hide resolved

fix(animations): getAnimationStyle causes exceptions in older browsers

2e26399

PR angular#29709 getAnimationStyle causes exceptions in older browsers

Serginho force-pushed the fix_getAnimation_style branch from ba35a0f to 2e26399 Compare November 17, 2020 10:40

petebacondarwin approved these changes Nov 17, 2020

View reviewed changes

crisbeto approved these changes Nov 17, 2020

View reviewed changes

AndrewKushnir reviewed Nov 18, 2020

View reviewed changes

packages/animations/browser/src/render/css_keyframes/element_animation_style_handler.ts Show resolved Hide resolved

AndrewKushnir added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer action: presubmit The PR is in need of a google3 presubmit labels Nov 20, 2020

AndrewKushnir pushed a commit that referenced this pull request Nov 20, 2020

fix(animations): getAnimationStyle causes exceptions in older browsers (

cb1d77a

#29709) PR #29709 getAnimationStyle causes exceptions in older browsers PR Close #29709

AndrewKushnir closed this in 66d863f Nov 20, 2020

angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(animations): getAnimationStyle causes exceptions in older browsers#29709

fix(animations): getAnimationStyle causes exceptions in older browsers#29709
Serginho wants to merge 1 commit intoangular:masterfrom
Serginho:fix_getAnimation_style

Serginho commented Apr 4, 2019

Uh oh!

matsko commented Feb 28, 2020

Uh oh!

Serginho commented Mar 1, 2020

Uh oh!

petebacondarwin left a comment

Uh oh!

petebacondarwin commented Nov 10, 2020

Uh oh!

Serginho commented Nov 17, 2020

Uh oh!

petebacondarwin left a comment

Uh oh!

Uh oh!

petebacondarwin left a comment

Uh oh!

AndrewKushnir commented Nov 18, 2020

Uh oh!

Uh oh!

AndrewKushnir commented Nov 20, 2020

Uh oh!

angular-automatic-lock-bot bot commented Dec 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

Serginho commented Apr 4, 2019

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

Uh oh!

matsko commented Feb 28, 2020

Uh oh!

Serginho commented Mar 1, 2020

Uh oh!

petebacondarwin left a comment

Choose a reason for hiding this comment

Uh oh!

petebacondarwin commented Nov 10, 2020

Uh oh!

Serginho commented Nov 17, 2020

Uh oh!

petebacondarwin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

petebacondarwin left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir commented Nov 18, 2020

Uh oh!

Uh oh!

AndrewKushnir commented Nov 20, 2020

Uh oh!

angular-automatic-lock-bot bot commented Dec 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants