From f1deed4593e6dc2e546605a8ac3f8263e6bf9abf Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 3 Dec 2024 10:47:25 +0100 Subject: [PATCH] fix(material/expansion): switch away from animations module Reworks the expansion panel to animate purely with CSS, rather than going through the `@angular/animations` module. This simplifies the setup and allows us to resolve several long-standing bug reports. --- .../expansion/expansion-animations.ts | 2 + .../expansion/expansion-panel-header.html | 2 +- .../expansion/expansion-panel-header.scss | 13 +++- .../expansion/expansion-panel-header.ts | 5 -- src/material/expansion/expansion-panel.html | 23 +++--- src/material/expansion/expansion-panel.scss | 49 +++++++++---- src/material/expansion/expansion-panel.ts | 73 +++++++++---------- src/material/expansion/expansion.spec.ts | 46 ++---------- tools/public_api_guard/material/expansion.md | 12 +-- 9 files changed, 102 insertions(+), 123 deletions(-) diff --git a/src/material/expansion/expansion-animations.ts b/src/material/expansion/expansion-animations.ts index 010dee09eefe..b17dc9d84daa 100644 --- a/src/material/expansion/expansion-animations.ts +++ b/src/material/expansion/expansion-animations.ts @@ -39,6 +39,8 @@ export const EXPANSION_PANEL_ANIMATION_TIMING = '225ms cubic-bezier(0.4,0.0,0.2, * Angular Bug: https://github.com/angular/angular/issues/18847 * * @docs-private + * @deprecated No longer being used, to be removed. + * @breaking-change 21.0.0 */ export const matExpansionAnimations: { readonly indicatorRotate: AnimationTriggerMetadata; diff --git a/src/material/expansion/expansion-panel-header.html b/src/material/expansion/expansion-panel-header.html index 84c077279b98..a631bfea4c29 100644 --- a/src/material/expansion/expansion-panel-header.html +++ b/src/material/expansion/expansion-panel-header.html @@ -5,7 +5,7 @@ @if (_showToggle()) { - + -
-
- - +
+
+
+ + +
+
-
diff --git a/src/material/expansion/expansion-panel.scss b/src/material/expansion/expansion-panel.scss index fc8f348802f0..e7c133985201 100644 --- a/src/material/expansion/expansion-panel.scss +++ b/src/material/expansion/expansion-panel.scss @@ -10,8 +10,11 @@ display: block; margin: 0; overflow: hidden; - transition: margin 225ms variables.$fast-out-slow-in-timing-function, - elevation.private-transition-property-value(); + + &.mat-expansion-panel-animations-enabled { + transition: margin 225ms variables.$fast-out-slow-in-timing-function, + elevation.private-transition-property-value(); + } // Required so that the `box-shadow` works after the // focus indicator relatively positions the header. @@ -48,18 +51,42 @@ @include cdk.high-contrast { outline: solid 1px; } +} + +.mat-expansion-panel-content-wrapper { + // Based on https://css-tricks.com/css-grid-can-do-auto-height-transitions. + display: grid; + grid-template-rows: 0fr; + grid-template-columns: 100%; + overflow: hidden; + + .mat-expansion-panel-animations-enabled & { + transition: grid-template-rows 225ms cubic-bezier(0.4, 0, 0.2, 1); + } - &.ng-animate-disabled, - .ng-animate-disabled &, - &._mat-animation-noopable { - transition: none; + .mat-expansion-panel.mat-expanded > & { + grid-template-rows: 1fr; } + + // All the browsers we support have support for `grid` as well, but + // given that these styles are load-bearing for the expansion panel, + // we have a fallback to `height` which doesn't animate, just in case. + // stylelint-disable material/no-prefixes + @supports not (grid-template-rows: 0fr) { + height: 0; + + .mat-expansion-panel.mat-expanded > & { + height: auto; + } + } + // stylelint-enable material/no-prefixes } .mat-expansion-panel-content { display: flex; flex-direction: column; overflow: visible; + min-height: 0; @include token-utils.use-tokens( tokens-mat-expansion.$prefix, tokens-mat-expansion.get-token-slots()) { @@ -69,16 +96,6 @@ @include token-utils.create-token-slot(line-height, container-text-line-height); @include token-utils.create-token-slot(letter-spacing, container-text-tracking); } - - // Usually the `visibility: hidden` added by the animation is enough to prevent focus from - // entering the collapsed content, but children with their own `visibility` can override it. - // In other components we set a `display: none` at the root to stop focus from reaching the - // elements, however we can't do that here, because the content can determine the width - // of an expansion panel. The most practical fallback is to use `!important` to override - // any custom visibility. - &[style*='visibility: hidden'] * { - visibility: hidden !important; - } } .mat-expansion-panel-body { diff --git a/src/material/expansion/expansion-panel.ts b/src/material/expansion/expansion-panel.ts index 3cb9bc07f8a0..e351ce58d2ca 100644 --- a/src/material/expansion/expansion-panel.ts +++ b/src/material/expansion/expansion-panel.ts @@ -6,7 +6,6 @@ * found in the LICENSE file at https://angular.dev/license */ -import {AnimationEvent} from '@angular/animations'; import {CdkAccordionItem} from '@angular/cdk/accordion'; import {UniqueSelectionDispatcher} from '@angular/cdk/collections'; import {CdkPortalOutlet, TemplatePortal} from '@angular/cdk/portal'; @@ -31,12 +30,12 @@ import { booleanAttribute, ANIMATION_MODULE_TYPE, inject, + NgZone, } from '@angular/core'; import {_IdGenerator} from '@angular/cdk/a11y'; import {Subject} from 'rxjs'; import {filter, startWith, take} from 'rxjs/operators'; import {MatAccordionBase, MatAccordionTogglePosition, MAT_ACCORDION} from './accordion-base'; -import {matExpansionAnimations} from './expansion-animations'; import {MAT_EXPANSION_PANEL} from './expansion-panel-base'; import {MatExpansionPanelContent} from './expansion-panel-content'; @@ -76,7 +75,6 @@ export const MAT_EXPANSION_PANEL_DEFAULT_OPTIONS = templateUrl: 'expansion-panel.html', encapsulation: ViewEncapsulation.None, changeDetection: ChangeDetectionStrategy.OnPush, - animations: [matExpansionAnimations.bodyExpansion], providers: [ // Provide MatAccordion as undefined to prevent nested expansion panels from registering // to the same accordion. @@ -86,7 +84,6 @@ export const MAT_EXPANSION_PANEL_DEFAULT_OPTIONS = host: { 'class': 'mat-expansion-panel', '[class.mat-expanded]': 'expanded', - '[class._mat-animation-noopable]': '_animationsDisabled', '[class.mat-expansion-panel-spacing]': '_hasSpacing()', }, imports: [CdkPortalOutlet], @@ -96,10 +93,11 @@ export class MatExpansionPanel implements AfterContentInit, OnChanges, OnDestroy { private _viewContainerRef = inject(ViewContainerRef); - _animationMode = inject(ANIMATION_MODULE_TYPE, {optional: true}); - - protected _animationsDisabled: boolean; + private readonly _animationsDisabled = + inject(ANIMATION_MODULE_TYPE, {optional: true}) === 'NoopAnimations'; private _document = inject(DOCUMENT); + private _ngZone = inject(NgZone); + private _elementRef = inject>(ElementRef); /** Whether the toggle indicator should be hidden. */ @Input({transform: booleanAttribute}) @@ -139,6 +137,10 @@ export class MatExpansionPanel /** Element containing the panel's user-provided content. */ @ViewChild('body') _body: ElementRef; + /** Element wrapping the panel body. */ + @ViewChild('bodyWrapper') + protected _bodyWrapper: ElementRef | undefined; + /** Portal holding the user's content. */ _portal: TemplatePortal; @@ -156,7 +158,6 @@ export class MatExpansionPanel ); this._expansionDispatcher = inject(UniqueSelectionDispatcher); - this._animationsDisabled = this._animationMode === 'NoopAnimations'; if (defaultOptions) { this.hideToggle = defaultOptions.hideToggle; @@ -204,6 +205,19 @@ export class MatExpansionPanel this._portal = new TemplatePortal(this._lazyContent._template, this._viewContainerRef); }); } + + this._ngZone.runOutsideAngular(() => { + if (this._animationsDisabled) { + this.opened.subscribe(() => this.afterExpand.emit()); + this.closed.subscribe(() => this.afterCollapse.emit()); + } else { + setTimeout(() => { + const element = this._elementRef.nativeElement; + element.addEventListener('transitionend', this._transitionEndListener); + element.classList.add('mat-expansion-panel-animations-enabled'); + }, 200); + } + }); } ngOnChanges(changes: SimpleChanges) { @@ -212,6 +226,10 @@ export class MatExpansionPanel override ngOnDestroy() { super.ngOnDestroy(); + this._bodyWrapper?.nativeElement.removeEventListener( + 'transitionend', + this._transitionEndListener, + ); this._inputChanges.complete(); } @@ -226,36 +244,17 @@ export class MatExpansionPanel return false; } - /** Called when the expansion animation has started. */ - protected _animationStarted(event: AnimationEvent) { - if (!isInitialAnimation(event) && !this._animationsDisabled && this._body) { - // Prevent the user from tabbing into the content while it's animating. - // TODO(crisbeto): maybe use `inert` to prevent focus from entering while closed as well - // instead of `visibility`? Will allow us to clean up some code but needs more testing. - this._body?.nativeElement.setAttribute('inert', ''); - } - } - - /** Called when the expansion animation has finished. */ - protected _animationDone(event: AnimationEvent) { - if (!isInitialAnimation(event)) { - if (event.toState === 'expanded') { - this.afterExpand.emit(); - } else if (event.toState === 'collapsed') { - this.afterCollapse.emit(); - } - - // Re-enable tabbing once the animation is finished. - if (!this._animationsDisabled && this._body) { - this._body.nativeElement.removeAttribute('inert'); - } + private _transitionEndListener = ({target, propertyName}: TransitionEvent) => { + if (target === this._bodyWrapper?.nativeElement && propertyName === 'grid-template-rows') { + this._ngZone.run(() => { + if (this.expanded) { + this.afterExpand.emit(); + } else { + this.afterCollapse.emit(); + } + }); } - } -} - -/** Checks whether an animation is the initial setup animation. */ -function isInitialAnimation(event: AnimationEvent): boolean { - return event.fromState === 'void'; + }; } /** diff --git a/src/material/expansion/expansion.spec.ts b/src/material/expansion/expansion.spec.ts index 4196816877fe..6931cde188b3 100644 --- a/src/material/expansion/expansion.spec.ts +++ b/src/material/expansion/expansion.spec.ts @@ -225,36 +225,20 @@ describe('MatExpansionPanel', () => { }); }); - it('should not be able to focus content while closed', fakeAsync(() => { + it('should not be able to focus content while closed', () => { const fixture = TestBed.createComponent(PanelWithContent); fixture.componentInstance.expanded = true; fixture.changeDetectorRef.markForCheck(); fixture.detectChanges(); - tick(250); - - const button = fixture.debugElement.query(By.css('button'))!.nativeElement; - - button.focus(); - expect(document.activeElement) - .withContext('Expected button to start off focusable.') - .toBe(button); + const wrapper = fixture.nativeElement.querySelector('.mat-expansion-panel-content-wrapper'); + expect(wrapper.hasAttribute('inert')).toBe(false); - button.blur(); fixture.componentInstance.expanded = false; fixture.changeDetectorRef.markForCheck(); fixture.detectChanges(); - tick(250); - - // Enforce a style recalculation as otherwise browsers like Safari on iOS 14 require - // us to wait until the next tick using actual async/await. Not retrieving the computed - // styles would result in the `visibility: hidden` on the expansion content to not apply. - getComputedStyle(button).getPropertyValue('visibility'); - button.focus(); - expect(document.activeElement) - .not.withContext('Expected button to no longer be focusable.') - .toBe(button); - })); + expect(wrapper.hasAttribute('inert')).toBe(true); + }); it('should restore focus to header if focused element is inside panel on close', fakeAsync(() => { const fixture = TestBed.createComponent(PanelWithContent); @@ -347,26 +331,6 @@ describe('MatExpansionPanel', () => { expect(content.classList).toContain('mat-content-hide-toggle'); }); - it('should update the indicator rotation when the expanded state is toggled programmatically', fakeAsync(() => { - const fixture = TestBed.createComponent(PanelWithContent); - - fixture.detectChanges(); - tick(250); - - const arrow = fixture.debugElement.query(By.css('.mat-expansion-indicator'))!.nativeElement; - - expect(arrow.style.transform).withContext('Expected no rotation.').toBe('rotate(0deg)'); - - fixture.componentInstance.expanded = true; - fixture.changeDetectorRef.markForCheck(); - fixture.detectChanges(); - tick(250); - - expect(arrow.style.transform) - .withContext('Expected 180 degree rotation.') - .toBe('rotate(180deg)'); - })); - it('should make sure accordion item runs ngOnDestroy when expansion panel is destroyed', () => { const fixture = TestBed.createComponent(PanelWithContentInNgIf); fixture.detectChanges(); diff --git a/tools/public_api_guard/material/expansion.md b/tools/public_api_guard/material/expansion.md index e4c2339ae2c2..b7c9959744a3 100644 --- a/tools/public_api_guard/material/expansion.md +++ b/tools/public_api_guard/material/expansion.md @@ -6,7 +6,6 @@ import { AfterContentInit } from '@angular/core'; import { AfterViewInit } from '@angular/core'; -import { AnimationEvent as AnimationEvent_2 } from '@angular/animations'; import { AnimationTriggerMetadata } from '@angular/animations'; import { CdkAccordion } from '@angular/cdk/accordion'; import { CdkAccordionItem } from '@angular/cdk/accordion'; @@ -75,7 +74,7 @@ export type MatAccordionDisplayMode = 'default' | 'flat'; // @public export type MatAccordionTogglePosition = 'before' | 'after'; -// @public +// @public @deprecated export const matExpansionAnimations: { readonly indicatorRotate: AnimationTriggerMetadata; readonly bodyExpansion: AnimationTriggerMetadata; @@ -97,13 +96,8 @@ export class MatExpansionPanel extends CdkAccordionItem implements AfterContentI accordion: MatAccordionBase; readonly afterCollapse: EventEmitter; readonly afterExpand: EventEmitter; - protected _animationDone(event: AnimationEvent_2): void; - // (undocumented) - _animationMode: "NoopAnimations" | "BrowserAnimations" | null; - // (undocumented) - protected _animationsDisabled: boolean; - protected _animationStarted(event: AnimationEvent_2): void; _body: ElementRef; + protected _bodyWrapper: ElementRef | undefined; close(): void; _containsFocus(): boolean; _getExpandedState(): MatExpansionPanelState; @@ -171,8 +165,6 @@ export class MatExpansionPanelDescription { // @public export class MatExpansionPanelHeader implements AfterViewInit, OnDestroy, FocusableOption { constructor(...args: unknown[]); - // (undocumented) - _animationMode: "NoopAnimations" | "BrowserAnimations" | null; collapsedHeight: string; get disabled(): boolean; expandedHeight: string;