From bed6ecedd4afd4d351183eb3c801e3d0b3719da8 Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@abv.bg>
Date: Mon, 26 Feb 2024 10:18:28 +0100
Subject: [PATCH] fix(material/expansion): prevent focus from entering the
 panel while it's animating

Currently the expansion panel prevents focus from entering it using `visibility: hidden`, but that only works when it's closed. This means that if the user tabs into it while it's animating, they may scroll the content make the component look broken.

These changes resolve the issue by setting `inert` on the panel content while it's animating. Also cleans up an old workaround for IE.

Fixes #27430.
Fixes #28644.
---
 src/material/expansion/expansion-panel.html  |  3 +-
 src/material/expansion/expansion-panel.ts    | 53 +++++++++++---------
 tools/public_api_guard/material/expansion.md |  3 +-
 3 files changed, 34 insertions(+), 25 deletions(-)

diff --git a/src/material/expansion/expansion-panel.html b/src/material/expansion/expansion-panel.html
index 81b6153001fe..cdf99435966a 100644
--- a/src/material/expansion/expansion-panel.html
+++ b/src/material/expansion/expansion-panel.html
@@ -2,7 +2,8 @@
 <div class="mat-expansion-panel-content"
      role="region"
      [@bodyExpansion]="_getExpandedState()"
-     (@bodyExpansion.done)="_bodyAnimationDone.next($event)"
+     (@bodyExpansion.start)="_animationStarted($event)"
+     (@bodyExpansion.done)="_animationDone($event)"
      [attr.aria-labelledby]="_headerId"
      [id]="id"
      #body>
diff --git a/src/material/expansion/expansion-panel.ts b/src/material/expansion/expansion-panel.ts
index d4878a24d91b..19c6966d918b 100644
--- a/src/material/expansion/expansion-panel.ts
+++ b/src/material/expansion/expansion-panel.ts
@@ -36,7 +36,7 @@ import {
   ANIMATION_MODULE_TYPE,
 } from '@angular/core';
 import {Subject} from 'rxjs';
-import {distinctUntilChanged, filter, startWith, take} from 'rxjs/operators';
+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';
@@ -147,9 +147,6 @@ export class MatExpansionPanel
   /** ID for the associated header element. Used for a11y labelling. */
   _headerId = `mat-expansion-panel-header-${uniqueId++}`;
 
-  /** Stream of body animation done events. */
-  readonly _bodyAnimationDone = new Subject<AnimationEvent>();
-
   constructor(
     @Optional() @SkipSelf() @Inject(MAT_ACCORDION) accordion: MatAccordionBase,
     _changeDetectorRef: ChangeDetectorRef,
@@ -165,24 +162,6 @@ export class MatExpansionPanel
     this.accordion = accordion;
     this._document = _document;
 
-    // We need a Subject with distinctUntilChanged, because the `done` event
-    // fires twice on some browsers. See https://github.com/angular/angular/issues/24084
-    this._bodyAnimationDone
-      .pipe(
-        distinctUntilChanged((x, y) => {
-          return x.fromState === y.fromState && x.toState === y.toState;
-        }),
-      )
-      .subscribe(event => {
-        if (event.fromState !== 'void') {
-          if (event.toState === 'expanded') {
-            this.afterExpand.emit();
-          } else if (event.toState === 'collapsed') {
-            this.afterCollapse.emit();
-          }
-        }
-      });
-
     if (defaultOptions) {
       this.hideToggle = defaultOptions.hideToggle;
     }
@@ -237,7 +216,6 @@ export class MatExpansionPanel
 
   override ngOnDestroy() {
     super.ngOnDestroy();
-    this._bodyAnimationDone.complete();
     this._inputChanges.complete();
   }
 
@@ -251,6 +229,35 @@ export class MatExpansionPanel
 
     return false;
   }
+
+  /** Called when the expansion animation has started. */
+  protected _animationStarted(event: AnimationEvent) {
+    if (!isInitialAnimation(event)) {
+      // 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-enabled tabbing once the animation is finished.
+      this._body?.nativeElement.removeAttribute('inert');
+    }
+  }
+}
+
+/** Checks whether an animation is the initial setup animation. */
+function isInitialAnimation(event: AnimationEvent): boolean {
+  return event.fromState === 'void';
 }
 
 /**
diff --git a/tools/public_api_guard/material/expansion.md b/tools/public_api_guard/material/expansion.md
index a3a42fa79b63..e2ed014b7b7f 100644
--- a/tools/public_api_guard/material/expansion.md
+++ b/tools/public_api_guard/material/expansion.md
@@ -101,10 +101,11 @@ export class MatExpansionPanel extends CdkAccordionItem implements AfterContentI
     accordion: MatAccordionBase;
     readonly afterCollapse: EventEmitter<void>;
     readonly afterExpand: EventEmitter<void>;
+    protected _animationDone(event: AnimationEvent_2): void;
     // (undocumented)
     _animationMode: string;
+    protected _animationStarted(event: AnimationEvent_2): void;
     _body: ElementRef<HTMLElement>;
-    readonly _bodyAnimationDone: Subject<AnimationEvent_2>;
     close(): void;
     _containsFocus(): boolean;
     _getExpandedState(): MatExpansionPanelState;