fix(expansion): MatExpansionHeader transition animations #13088

chriskrolak · 2018-09-12T00:47:27Z

See issue #11990 for rationale.

No tests included - I would greatly appreciate help on how to write a unit test for this, since I can only reproduce the bug as a combination of many components.

fix MatExpansionPanel when included in a sortable MatTable See issue angular#11990 for more details. Closes angular#11990

fix MatExpansionPanel when included in a sortable MatTable See issue angular#11990 for more details. Fixes angular#11990

josephperrott · 2018-09-13T15:56:56Z

@chriskrolak Can you explain a bit about how this change addresses #11990? I am not quite following.

chriskrolak · 2018-09-13T16:06:06Z

Honestly I also have no idea how it works; I copied it from a solution somewhere online that I can't find anymore. It does fix the issue though (tested locally).

josephperrott

Can you change just the one comment?

Otherwise it looks fine to commit as it should address the exact issue you are seeing, though beyond the reproduction scope I can't say for sure.

Though I will comment that this does not address the underlying problem of ViewContainer.move calls causing the full exit, and then improper reentry of the node as far as the animation states are concerned. This is something that will have to be addressed upstream in angular/angular.

josephperrott · 2018-09-13T19:57:31Z

src/lib/expansion/expansion-animations.ts

@@ -30,12 +30,12 @@ export const matExpansionAnimations: {
  indicatorRotate: trigger('indicatorRotate', [
    state('collapsed', style({transform: 'rotate(0deg)'})),
    state('expanded', style({transform: 'rotate(180deg)'})),
-    transition('expanded <=> collapsed', animate(EXPANSION_PANEL_ANIMATION_TIMING)),
+    transition('collapsed <=> expanded', animate(EXPANSION_PANEL_ANIMATION_TIMING)),


You can make this * <=> * here.

josephperrott · 2018-09-13T20:10:35Z

For reference purposes angular/angular#18847 looks to be the issue tracking this in angular/angular

fix(expansion): MatExpansionPanel animations fix MatExpansionPanel when included in a sortable MatTable See issue angular#11990 for more details. Fixes angular#11990

josephperrott · 2018-09-13T21:26:28Z

After giving it another pass, I realized the reason that this change actually ends up being a breaking change for the current way things work. With this change, any expansion panel that starts as expanded would animate to its initial size instead of appearing immediately at the correct size.

josephperrott · 2018-09-13T22:07:43Z

src/lib/expansion/expansion-animations.ts

@@ -30,12 +30,12 @@ export const matExpansionAnimations: {
  indicatorRotate: trigger('indicatorRotate', [
    state('collapsed', style({transform: 'rotate(0deg)'})),
    state('expanded', style({transform: 'rotate(180deg)'})),
-    transition('expanded <=> collapsed', animate(EXPANSION_PANEL_ANIMATION_TIMING)),
+    transition('* <=> *', animate(EXPANSION_PANEL_ANIMATION_TIMING)),


For all of the transitions can we have it be:

void => collapsed, expanded <=> collapsed

josephperrott · 2018-09-13T22:08:05Z

src/lib/expansion/expansion-animations.ts

  ]),

  /** Animation that expands and collapses the panel header height. */
  expansionHeaderHeight: trigger('expansionHeight', [
-    state('collapsed', style({
+    state('collapsed, void', style({


All of the states of collapsed should be collapsed, void

josephperrott · 2018-09-13T22:25:06Z

src/lib/expansion/expansion-animations.ts

@@ -30,12 +30,12 @@ export const matExpansionAnimations: {
  indicatorRotate: trigger('indicatorRotate', [


Can you add a comment around line 22, above the const line:

/** * A bug in angular animation's `state` when ViewContainers are moved using ViewContainerRef.move() * causes the animation state of moved components to become `void` upon exit, and not update again * upon reentry into the DOM. This can lead a to situation for the expansion panel where the state * of the panel is `expanded` or `collapsed` but the animation state is `void`. * * To correctly handle animating to the next state, we animate between `void` and `collapsed` which * are defined to have the same styles. Since angular animates from the current styles to the * destination state's style definition, in situations where we are moving from `void`'s styles to * `collapsed` this acts a noop since no style values change. * * In the case where angular's animation state is out of sync with the expansion panel's state, the * expansion panel being `expanded` and angular animations being`void`, the animation from the * `expanded`'s effective styles (though in a `void` animation state) to the collapsed state will * occur as expected. * * Angular Bug: https://github.com/angular/angular/issues/18847 **/

Sorry to ask for such a large block of comment in the code your committing, I just didn't want to ask your to have to write out the explanation.

fix(expansion): MatExpansionPanel animations fix MatExpansionPanel when included in a sortable MatTable See issue angular#11990 for more details. Fixes angular#11990

googlebot · 2018-09-14T15:00:26Z

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.

googlebot · 2018-09-14T15:00:49Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

josephperrott · 2018-09-14T15:01:02Z

CLA fixed since it was bad from my lint fix commit.

josephperrott

LGTM, Thanks!

… height Works around the expansion panel header animating on init, if its header is different from the default one. The original issue comes from the code introduced in angular#13088, but we can't remove that code, because it's there to work around a bug in Angular. Fixes angular#16067.

… height (#16122) Works around the expansion panel header animating on init, if its header is different from the default one. The original issue comes from the code introduced in #13088, but we can't remove that code, because it's there to work around a bug in Angular. Fixes #16067.

… height (angular#16122) Works around the expansion panel header animating on init, if its header is different from the default one. The original issue comes from the code introduced in angular#13088, but we can't remove that code, because it's there to work around a bug in Angular. Fixes angular#16067.

… height (#16122) Works around the expansion panel header animating on init, if its header is different from the default one. The original issue comes from the code introduced in #13088, but we can't remove that code, because it's there to work around a bug in Angular. Fixes #16067.

angular-automatic-lock-bot · 2019-09-09T15:01:49Z

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

chriskrolak added 2 commits September 11, 2018 17:09

fix: MatExpansionPanel animations

f91e1d3

fix MatExpansionPanel when included in a sortable MatTable See issue angular#11990 for more details. Closes angular#11990

fix(expansion): MatExpansionPanel animations

a1aad13

fix MatExpansionPanel when included in a sortable MatTable See issue angular#11990 for more details. Fixes angular#11990

chriskrolak requested review from jelbourn and josephperrott as code owners September 12, 2018 00:47

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 12, 2018

jelbourn added G This is is related to a Google internal issue P2 The issue is important to a large percentage of users, with a workaround labels Sep 12, 2018

josephperrott reviewed Sep 13, 2018

View reviewed changes

josephperrott added the target: patch This PR is targeted for the next patch release label Sep 13, 2018

josephperrott changed the title ~~Fix MatExpansionHeader transition animations.~~ fix(expansion): MatExpansionHeader transition animations Sep 13, 2018

fix(expansion): MatExpansionPanel animations

c49d629

fix(expansion): MatExpansionPanel animations fix MatExpansionPanel when included in a sortable MatTable See issue angular#11990 for more details. Fixes angular#11990

josephperrott reviewed Sep 13, 2018

View reviewed changes

chriskrolak and others added 2 commits September 13, 2018 18:12

fix(expansion): MatExpansionPanel animations

9a10979

fix(expansion): MatExpansionPanel animations fix MatExpansionPanel when included in a sortable MatTable See issue angular#11990 for more details. Fixes angular#11990

Update expansion-animations.ts

c0c3d48

googlebot added cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Sep 14, 2018

josephperrott added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Sep 14, 2018

josephperrott approved these changes Sep 14, 2018

View reviewed changes

josephperrott added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Sep 14, 2018

mmalerba merged commit 4a96539 into angular:master Sep 18, 2018

crisbeto mentioned this pull request May 26, 2019

fix(expansion-panel): header animating on init when using non-default height #16122

Merged

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(expansion): MatExpansionHeader transition animations #13088

fix(expansion): MatExpansionHeader transition animations #13088

Uh oh!

chriskrolak commented Sep 12, 2018

Uh oh!

josephperrott commented Sep 13, 2018

Uh oh!

chriskrolak commented Sep 13, 2018

Uh oh!

josephperrott left a comment

Uh oh!

josephperrott Sep 13, 2018

Uh oh!

josephperrott commented Sep 13, 2018

Uh oh!

josephperrott commented Sep 13, 2018

Uh oh!

josephperrott Sep 13, 2018

Uh oh!

josephperrott Sep 13, 2018

Uh oh!

josephperrott Sep 13, 2018

Uh oh!

josephperrott Sep 13, 2018

Uh oh!

googlebot commented Sep 14, 2018

Uh oh!

googlebot commented Sep 14, 2018

Uh oh!

josephperrott commented Sep 14, 2018

Uh oh!

josephperrott left a comment

Uh oh!

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

Uh oh!

Uh oh!

		@@ -30,12 +30,12 @@ export const matExpansionAnimations: {
		indicatorRotate: trigger('indicatorRotate', [

fix(expansion): MatExpansionHeader transition animations #13088

fix(expansion): MatExpansionHeader transition animations #13088

Uh oh!

Conversation

chriskrolak commented Sep 12, 2018

Uh oh!

josephperrott commented Sep 13, 2018

Uh oh!

chriskrolak commented Sep 13, 2018

Uh oh!

josephperrott left a comment

Choose a reason for hiding this comment

Uh oh!

josephperrott Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

josephperrott commented Sep 13, 2018

Uh oh!

josephperrott commented Sep 13, 2018

Uh oh!

josephperrott Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

josephperrott Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

josephperrott Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

josephperrott Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

googlebot commented Sep 14, 2018

Uh oh!

googlebot commented Sep 14, 2018

Uh oh!

josephperrott commented Sep 14, 2018

Uh oh!

josephperrott left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!