From 37a7080e00dad36acc2f5fb392c2a1568e2d21d7 Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@users.noreply.github.com>
Date: Wed, 19 Dec 2018 16:07:15 +0100
Subject: [PATCH] fix(menu): scroll position jumping to top after animation is
 done on scrollable menu (#14190)

Originally in #11859 we introduced some logic that makes sure that the menu panel is always scrolled to the top when it's open. This works, but is janky, because the user can see the menu being scrolled down while it's animating. These changes move the logic, that reset the scroll position, from the `done` callback to `start`.

Fixes #11790.
---
 src/lib/menu/menu-directive.ts | 16 ++++++++++------
 src/lib/menu/menu.html         |  2 +-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/lib/menu/menu-directive.ts b/src/lib/menu/menu-directive.ts
index 2798b138d93..0090c27d334 100644
--- a/src/lib/menu/menu-directive.ts
+++ b/src/lib/menu/menu-directive.ts
@@ -384,13 +384,17 @@ export class MatMenu implements AfterContentInit, MatMenuPanel<MatMenuItem>, OnI
   _onAnimationDone(event: AnimationEvent) {
     this._animationDone.next(event);
     this._isAnimating = false;
+  }
 
-    // Scroll the content element to the top once the animation is done. This is necessary, because
-    // we move focus to the first item while it's still being animated, which can throw the browser
-    // off when it determines the scroll position. Alternatively we can move focus when the
-    // animation is done, however moving focus asynchronously will interrupt screen readers
-    // which are in the process of reading out the menu already. We take the `element` from
-    // the `event` since we can't use a `ViewChild` to access the pane.
+  _onAnimationStart(event: AnimationEvent) {
+    this._isAnimating = true;
+
+    // Scroll the content element to the top as soon as the animation starts. This is necessary,
+    // because we move focus to the first item while it's still being animated, which can throw
+    // the browser off when it determines the scroll position. Alternatively we can move focus
+    // when the animation is done, however moving focus asynchronously will interrupt screen
+    // readers which are in the process of reading out the menu already. We take the `element`
+    // from the `event` since we can't use a `ViewChild` to access the pane.
     if (event.toState === 'enter' && this._keyManager.activeItemIndex === 0) {
       event.element.scrollTop = 0;
     }
diff --git a/src/lib/menu/menu.html b/src/lib/menu/menu.html
index b173e3cccc3..b0713131299 100644
--- a/src/lib/menu/menu.html
+++ b/src/lib/menu/menu.html
@@ -5,7 +5,7 @@
     (keydown)="_handleKeydown($event)"
     (click)="closed.emit('click')"
     [@transformMenu]="_panelAnimationState"
-    (@transformMenu.start)="_isAnimating = true"
+    (@transformMenu.start)="_onAnimationStart($event)"
     (@transformMenu.done)="_onAnimationDone($event)"
     tabindex="-1"
     role="menu">