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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(animations): apply setStyles to only rootTimelines #44515

Conversation

dario-piotrowicz
Copy link
Contributor

during keyframe building only consider the root element's timelines
for the style setting, so that the states styles (applied with '*')
can be applied correctly

resolves #32133
resolves #28654

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?

'*' animation values can be lost based where there are animation's timelines not belonging
to the root element

Issue Number: #32133, #28654

What is the new behavior?

only the root element's timelines are considered, this makes sure that '*' get handled correctly

Does this PR introduce a breaking change?

  • Yes
  • No

@dario-piotrowicz
Copy link
Contributor Author

Just wanted to mention that this is likely to also fix: #25194 🙂

Copy link
Member

@JoostK JoostK left a comment

Choose a reason for hiding this comment

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

Mostly a drive-by comment, I haven't attempted to understand the changes in this PR.

@dario-piotrowicz
Copy link
Contributor Author

@JoostK thanks a lot for the comments! 😃

I've addressed all of them, hopefully is all good from your point of view 🙂

Copy link
Member

@JoostK JoostK left a comment

Choose a reason for hiding this comment

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

This LGTM.

@JoostK JoostK requested review from jessicajaniuk and removed request for alxhub December 17, 2021 16:52
@Manndung75
Copy link

www have

@ngbot ngbot bot added this to the Backlog milestone Jan 4, 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 🍪

Thanks, @dario-piotrowicz!

during keyframe building only consider the root element's timelines
for the style setting, so that the states styles (applied with '*')
can be applied correctly

resolves angular#32133
resolves angular#28654
@dario-piotrowicz
Copy link
Contributor Author

Hi @jessicajaniuk , could I get a merge label? 😛 😄

@jessicajaniuk jessicajaniuk added target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit and removed action: presubmit The PR is in need of a google3 presubmit labels Jan 13, 2022
@dylhunn
Copy link
Contributor

dylhunn commented Jan 13, 2022

This PR was merged into the repository by commit adc487f.

dylhunn pushed a commit that referenced this pull request Jan 13, 2022
during keyframe building only consider the root element's timelines
for the style setting, so that the states styles (applied with '*')
can be applied correctly

resolves #32133
resolves #28654

PR Close #44515
@dylhunn dylhunn closed this in adc487f Jan 13, 2022
@dario-piotrowicz dario-piotrowicz deleted the rootTimelines-setStyles branch January 15, 2022 15:12
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Feb 7, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@angular/animations](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fanimations/13.1.2/13.2.1) |
| [@angular/common](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcommon/13.1.2/13.2.1) |
| [@angular/compiler](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/13.1.2/13.2.1) |
| [@angular/compiler-cli](https://github.com/angular/angular) | devDependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/13.1.2/13.2.1) |
| [@angular/core](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcore/13.1.2/13.2.1) |
| [@angular/forms](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fforms/13.1.2/13.2.1) |
| [@angular/platform-browser](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/13.1.2/13.2.1) |
| [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/13.1.2/13.2.1) |
| [@angular/router](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2frouter/13.1.2/13.2.1) |

---

### Release Notes

<details>
<summary>angular/angular</summary>

### [`v13.2.1`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1321-2022-02-02)

[Compare Source](angular/angular@13.2.0...13.2.1)

##### animations

| Commit | Type | Description |
| -- | -- | -- |
| [4644886aaf](angular/angular@4644886) | perf | remove no longer needed CssKeyframes classes ([#&#8203;44903](angular/angular#44903)) ([#&#8203;44919](angular/angular#44919)) |

##### common

| Commit | Type | Description |
| -- | -- | -- |
| [b4e4617807](angular/angular@b4e4617) | fix | include query parameters for open HTTP requests in `verify` ([#&#8203;44917](angular/angular#44917)) |

##### compiler-cli

| Commit | Type | Description |
| -- | -- | -- |
| [0778e6f7d7](angular/angular@0778e6f) | fix | accept nullish coalescing operator for any and unknown types ([#&#8203;44862](angular/angular#44862)) |
| [07185f4ed1](angular/angular@07185f4) | fix | enable nullish coalescing check only with `strictNullChecks` ([#&#8203;44862](angular/angular#44862)) |
| [4a5ad1793f](angular/angular@4a5ad17) | fix | ensure casing of logical paths is preserved ([#&#8203;44798](angular/angular#44798)) |

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [7ec482d9c2](angular/angular@7ec482d) | fix | Add back support for namespace URIs in createElement of dom renderer ([#&#8203;44914](angular/angular#44914)) |
| [250dc40a46](angular/angular@250dc40) | fix | flush delayed scoping queue while setting up TestBed ([#&#8203;44814](angular/angular#44814)) |

##### forms

| Commit | Type | Description |
| -- | -- | -- |
| [1aebbf8714](angular/angular@1aebbf8) | fix | ensure OnPush ancestors are marked dirty when the promise resolves ([#&#8203;44886](angular/angular#44886)) |
| [6b7fffcbeb](angular/angular@6b7fffc) | fix | Update the typed forms migration schematic to find all files. ([#&#8203;44881](angular/angular#44881)) |

#### Special Thanks

Alan, Andrew Kushnir, Andrew Scott, Aristeidis Bampakos, Arjen, Daniel Díaz, David Shevitz, Doug Parker, Dylan Hunn, Esteban Gehring, George Kalpakas, Jessica Janiuk, JoostK, Juri Strumpflohner, Lee Robinson, Maarten Tibau, Paul Gschwendtner, Theodore Brown, arturovt, dario-piotrowicz, fru2, markostanimirovic and mgechev

<!-- CHANGELOG SPLIT MARKER -->

### [`v13.2.0`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1320-2022-01-26)

[Compare Source](angular/angular@13.1.3...13.2.0)

#### Deprecations

#####

-   The `CachedResourceLoader` and `RESOURCE_CACHE_PROVIDER` symbols were previously necessary in some cases to test AOT-compiled components with View Engine, but they are no longer needed since Ivy.

-   The `ComponentFactory` and `ComponentFactoryResolver` classes are deprecated. Since Ivy, there is no need to resolve Component factories. Please use other APIs where you Component classes can be used directly (without resolving their factories).

-   Since Ivy, the `CompilerOptions.useJit` and `CompilerOptions.missingTranslation` config options are unused, passing them has no effect.

#####

| Commit | Type | Description |
| -- | -- | -- |
| [9c11183e74](angular/angular@9c11183) | docs | deprecate `CachedResourceLoader` and `RESOURCE_CACHE_PROVIDER` symbols ([#&#8203;44749](angular/angular#44749)) |
| [9f12e7fea4](angular/angular@9f12e7f) | docs | deprecate `ComponentFactory` and `ComponentFactoryResolver` symbols ([#&#8203;44749](angular/angular#44749)) |
| [4e95a316ce](angular/angular@4e95a31) | docs | deprecate unused config options from the `CompilerOptions` interface ([#&#8203;44749](angular/angular#44749)) |

##### compiler

| Commit | Type | Description |
| -- | -- | -- |
| [a4ab6d6b72](angular/angular@a4ab6d6) | feat | add support for safe calls in templates ([#&#8203;44580](angular/angular#44580)) |
| [abd1bc8039](angular/angular@abd1bc8) | fix | correct spans when parsing bindings with comments ([#&#8203;44785](angular/angular#44785)) |
| [ed67a074ce](angular/angular@ed67a07) | fix | properly compile DI factories when coverage reporting is enabled ([#&#8203;44732](angular/angular#44732)) |

##### compiler-cli

| Commit | Type | Description |
| -- | -- | -- |
| [fa835b5a29](angular/angular@fa835b5) | feat | enable extended diagnostics by default ([#&#8203;44712](angular/angular#44712)) |
| [73424def13](angular/angular@73424de) | feat | provide the animations for `DirectiveMeta` ([#&#8203;44630](angular/angular#44630)) |
| [fe3e4d6865](angular/angular@fe3e4d6) | fix | Handle `ng-template` with structural directive in indexer ([#&#8203;44788](angular/angular#44788)) |
| [7316e72ec5](angular/angular@7316e72) | fix | properly index <svg> elements when on a template ([#&#8203;44785](angular/angular#44785)) |
| [100091ebf0](angular/angular@100091e) | fix | remove leftover `_extendedTemplateDiagnostics` requirements ([#&#8203;44777](angular/angular#44777)) |
| [d2ae96f742](angular/angular@d2ae96f) | fix | skip `ExtendedTemplateCheckerImpl` construction if there were configuration errors ([#&#8203;44778](angular/angular#44778)) |

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [5626b34264](angular/angular@5626b34) | fix | consistently use namespace short name rather than URI ([#&#8203;44766](angular/angular#44766)) |
| [94bfcdd9de](angular/angular@94bfcdd) | fix | error if NgZone.isInAngularZone is called with a noop zone ([#&#8203;44800](angular/angular#44800)) |

##### forms

| Commit | Type | Description |
| -- | -- | -- |
| [72092ebd26](angular/angular@72092eb) | feat | Allow a FormControl to use initial value as default. ([#&#8203;44434](angular/angular#44434)) |
| [f7aa937cac](angular/angular@f7aa937) | fix | Make some minor fixups for forward-compatibility with typed forms. ([#&#8203;44540](angular/angular#44540)) |

##### router

| Commit | Type | Description |
| -- | -- | -- |
| [5a4ddfd4f5](angular/angular@5a4ddfd) | feat | Allow symbol keys for `Route` `data` and `resolve` properties ([#&#8203;44519](angular/angular#44519)) |

#### Special Thanks

Alex Rickabaugh, Andrew Kushnir, Andrew Scott, Dario Piotrowicz, Derek Cormier, Doug Parker, Douglas Parker, Dylan Hunn, George Kalpakas, Jessica Janiuk, JoostK, Kristiyan Kostadinov, Martin Probst, Oleg Postoev, Stephanie Tuerk, Tim Bowersox, Wiley Marques, Yousaf Nawaz, dario-piotrowicz, iRealNirmal, ivanwonder and shejialuo

<!-- CHANGELOG SPLIT MARKER -->

### [`v13.1.3`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1313-2022-01-19)

[Compare Source](angular/angular@13.1.2...13.1.3)

##### animations

| Commit | Type | Description |
| -- | -- | -- |
| [af0a152a2c](angular/angular@af0a152) | fix | apply setStyles to only rootTimelines ([#&#8203;44515](angular/angular#44515)) |

##### compiler-cli

| Commit | Type | Description |
| -- | -- | -- |
| [626f3f230b](angular/angular@626f3f2) | perf | reduce analysis work during incremental rebuilds ([#&#8203;44731](angular/angular#44731)) |

##### ngcc

| Commit | Type | Description |
| -- | -- | -- |
| [f9ca4d8499](angular/angular@f9ca4d8) | fix | support element accesses for export declarations ([#&#8203;44669](angular/angular#44669)) |

#### Special Thanks

Alan Agius, Andrew Kushnir, AnkitSharma-007, Daniel Díaz, Dmytro Mezhenskyi, Jessica Janiuk, Joey Perrott, JoostK, Ramesh Thiruchelvam, dario-piotrowicz, iRealNirmal and Łukasz Holeczek

<!-- CHANGELOG SPLIT MARKER -->

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Co-authored-by: crapStone <crapstone@noreply.codeberg.org>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1148
Reviewed-by: crapStone <crapstone@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@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 Feb 15, 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
6 participants