Skip to content

fix(expansion): MatExpansionHeader transition animations #13088

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

Merged
merged 5 commits into from
Sep 18, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 28 additions & 7 deletions src/lib/expansion/expansion-animations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,42 @@ import {
/** Time and timing curve for expansion panel animations. */
export const EXPANSION_PANEL_ANIMATION_TIMING = '225ms cubic-bezier(0.4,0.0,0.2,1)';

/** Animations used by the Material expansion panel. */
/**
* Animations used by the Material expansion panel.
*
* 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
*/
export const matExpansionAnimations: {
readonly indicatorRotate: AnimationTriggerMetadata;
readonly expansionHeaderHeight: AnimationTriggerMetadata;
readonly bodyExpansion: AnimationTriggerMetadata;
} = {
/** Animation that rotates the indicator arrow. */
indicatorRotate: trigger('indicatorRotate', [
Copy link
Member

Choose a reason for hiding this comment

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

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
 **/

Copy link
Member

Choose a reason for hiding this comment

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

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.

state('collapsed', style({transform: 'rotate(0deg)'})),
state('collapsed, void', style({transform: 'rotate(0deg)'})),
state('expanded', style({transform: 'rotate(180deg)'})),
transition('expanded <=> collapsed', animate(EXPANSION_PANEL_ANIMATION_TIMING)),
transition('expanded <=> collapsed, void => collapsed',
animate(EXPANSION_PANEL_ANIMATION_TIMING)),
]),

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

Choose a reason for hiding this comment

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

All of the states of collapsed should be collapsed, void

height: '{{collapsedHeight}}',
}), {
params: {collapsedHeight: '48px'},
Expand All @@ -45,16 +65,17 @@ export const matExpansionAnimations: {
}), {
params: {expandedHeight: '64px'}
}),
transition('expanded <=> collapsed', group([
transition('expanded <=> collapsed, void => collapsed', group([
query('@indicatorRotate', animateChild(), {optional: true}),
animate(EXPANSION_PANEL_ANIMATION_TIMING),
])),
]),

/** Animation that expands and collapses the panel content. */
bodyExpansion: trigger('bodyExpansion', [
state('collapsed', style({height: '0px', visibility: 'hidden'})),
state('collapsed, void', style({height: '0px', visibility: 'hidden'})),
state('expanded', style({height: '*', visibility: 'visible'})),
transition('expanded <=> collapsed', animate(EXPANSION_PANEL_ANIMATION_TIMING)),
transition('expanded <=> collapsed, void => collapsed',
animate(EXPANSION_PANEL_ANIMATION_TIMING)),
])
};