MenuAnchor hover traversal fixes (flutter#150914)

Fixes flutter#150910 and flutter#150911. 

flutter#150910 is fixed by invalidating the focus scope whenever a hover occurs. I'm interested to hear of better fixes -- it feels a bit extreme to invalidate the focus scope so often.

flutter#150911 is fixed by replacing TextButton.onHover with MouseRegion.onHover and MouseRegion.onExit. The issue appears to be that MouseRegion.onEnter is called on scroll, whereas MouseRegion.onHover is not. I'm not confident this is a great solution, so please let me know if you all have any suggestions!

@Piinks @dkwingsmt

Author: davidhicks980

packages/flutter/lib/src/material/menu_anchor.dart

@@ -1072,6 +1072,7 @@ class _MenuItemButtonState extends State<MenuItemButton> {
   FocusNode? _internalFocusNode;
   FocusNode get _focusNode => widget.focusNode ?? _internalFocusNode!;
   _MenuAnchorState? get _anchor => _MenuAnchorState._maybeOf(context);
+  bool _isHovered = false;
 
   @override
   void initState() {
@@ -1115,7 +1116,6 @@ class _MenuItemButtonState extends State<MenuItemButton> {
 
     Widget child = TextButton(
       onPressed: widget.enabled ? _handleSelect : null,
-      onHover: widget.enabled ? _handleHover : null,
       onFocusChange: widget.enabled ? widget.onFocusChange : null,
       focusNode: _focusNode,
       style: mergedStyle,
@@ -1141,6 +1141,14 @@ class _MenuItemButtonState extends State<MenuItemButton> {
       );
     }
 
+    if (widget.onHover != null || widget.requestFocusOnHover) {
+      child = MouseRegion(
+        onHover: _handlePointerHover,
+        onExit: _handlePointerExit,
+        child: child,
+      );
+    }
+
     return MergeSemantics(child: child);
   }
 
@@ -1151,11 +1159,29 @@ class _MenuItemButtonState extends State<MenuItemButton> {
     }
   }
 
-  void _handleHover(bool hovering) {
-    widget.onHover?.call(hovering);
-    if (hovering && widget.requestFocusOnHover) {
-      assert(_debugMenuInfo('Requesting focus for $_focusNode from hover'));
-      _focusNode.requestFocus();
+  void _handlePointerExit(PointerExitEvent event) {
+    if (_isHovered) {
+      widget.onHover?.call(false);
+      _isHovered = false;
+    }
+  }
+
+  // TextButton.onHover and MouseRegion.onHover can't be used without triggering
+  // focus on scroll.
+  void _handlePointerHover(PointerHoverEvent event) {
+    if (!_isHovered) {
+      _isHovered = true;
+      widget.onHover?.call(true);
+      if (widget.requestFocusOnHover) {
+        assert(_debugMenuInfo('Requesting focus for $_focusNode from hover'));
+        _focusNode.requestFocus();
+
+        // Without invalidating the focus policy, switching to directional focus
+        // may not originate at this node.
+        FocusTraversalGroup.of(context).invalidateScopeData(
+          FocusScope.of(context),
+        );
+      }
     }
   }
 
@@ -1839,6 +1865,7 @@ class _SubmenuButtonState extends State<SubmenuButton> {
   FocusNode? _internalFocusNode;
   FocusNode get _buttonFocusNode => widget.focusNode ?? _internalFocusNode!;
   bool get _enabled => widget.menuChildren.isNotEmpty;
+  bool _isHovered = false;
 
   @override
   void initState() {
@@ -1923,7 +1950,7 @@ class _SubmenuButtonState extends State<SubmenuButton> {
           ?? widget.defaultStyleOf(context);
         mergedStyle = widget.style?.merge(mergedStyle) ?? mergedStyle;
 
-        void toggleShowMenu(BuildContext context) {
+        void toggleShowMenu() {
           if (controller._anchor == null) {
             return;
           }
@@ -1934,18 +1961,29 @@ class _SubmenuButtonState extends State<SubmenuButton> {
           }
         }
 
-        // Called when the pointer is hovering over the menu button.
-        void handleHover(bool hovering, BuildContext context) {
-          widget.onHover?.call(hovering);
-          // Don't open the root menu bar menus on hover unless something else
-          // is already open. This means that the user has to first click to
-          // open a menu on the menu bar before hovering allows them to traverse
-          // it.
-          if (controller._anchor!._root._orientation == Axis.horizontal && !controller._anchor!._root._isOpen) {
-            return;
+        void handlePointerExit(PointerExitEvent event) {
+          if (_isHovered) {
+            widget.onHover?.call(false);
+            _isHovered = false;
           }
+        }
+
+        // MouseRegion.onEnter and TextButton.onHover are called
+        // if a button is hovered after scrolling. This interferes with
+        // focus traversal and scroll position. MouseRegion.onHover avoids
+        // this issue.
+        void handlePointerHover(PointerHoverEvent event) {
+          if (!_isHovered) {
+            _isHovered = true;
+            widget.onHover?.call(true);
+            // Don't open the root menu bar menus on hover unless something else
+            // is already open. This means that the user has to first click to
+            // open a menu on the menu bar before hovering allows them to traverse
+            // it.
+            if (controller._anchor!._root._orientation == Axis.horizontal && !controller._anchor!._root._isOpen) {
+              return;
+            }
 
-          if (hovering) {
             controller.open();
             controller._anchor!._focusButton();
           }
@@ -1957,8 +1995,7 @@ class _SubmenuButtonState extends State<SubmenuButton> {
               style: mergedStyle,
               focusNode: _buttonFocusNode,
               onFocusChange: _enabled ? widget.onFocusChange : null,
-              onHover: _enabled ? (bool hovering) => handleHover(hovering, context) : null,
-              onPressed: _enabled ? () => toggleShowMenu(context) : null,
+              onPressed: _enabled ? toggleShowMenu : null,
               isSemanticButton: null,
               child: _MenuItemLabel(
                 leadingIcon: widget.leadingIcon,
@@ -1971,13 +2008,24 @@ class _SubmenuButtonState extends State<SubmenuButton> {
           ),
         );
 
-        if (_enabled && _platformSupportsAccelerators) {
+        if (!_enabled) {
+          return child;
+        }
+
+        child = MouseRegion(
+          onHover: handlePointerHover,
+          onExit: handlePointerExit,
+          child: child,
+        );
+
+        if (_platformSupportsAccelerators) {
           return MenuAcceleratorCallbackBinding(
-            onInvoke: () => toggleShowMenu(context),
+            onInvoke: toggleShowMenu,
             hasSubmenu: true,
             child: child,
           );
         }
+
         return child;
       },
       menuChildren: widget.menuChildren,

packages/flutter/test/material/menu_anchor_test.dart

@@ -2021,6 +2021,103 @@ void main() {
       expect(focusedMenu, equals('MenuItemButton(Text("Sub Sub Menu 110"))'));
     });
 
+
+    testWidgets('hover traversal invalidates directional focus scope data', (WidgetTester tester) async {
+      // Regression test for https://github.com/flutter/flutter/issues/150910
+      await tester.pumpWidget(
+        MaterialApp(
+          home: Material(
+            child: MenuBar(
+              controller: controller,
+              children: createTestMenus(
+                onPressed: onPressed,
+                onOpen: onOpen,
+                onClose: onClose,
+              ),
+            ),
+          ),
+        ),
+      );
+
+      listenForFocusChanges();
+
+      // Have to open a menu initially to start things going.
+      await tester.tap(find.text(TestMenu.mainMenu1.label));
+      await tester.pump();
+      expect(focusedMenu, equals('SubmenuButton(Text("Menu 1"))'));
+
+      await hoverOver(tester, find.text(TestMenu.subMenu12.label));
+      await tester.pump();
+      expect(focusedMenu, equals('MenuItemButton(Text("Sub Menu 12"))'));
+
+      // Move pointer to disabled menu
+      await hoverOver(tester, find.text(TestMenu.mainMenu5.label));
+      await tester.pump();
+      expect(focusedMenu, equals('MenuItemButton(Text("Sub Menu 12"))'));
+
+      await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp);
+      await tester.pump();
+      expect(focusedMenu, equals('SubmenuButton(Text("Sub Menu 11"))'));
+
+      await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp);
+      expect(focusedMenu, equals('MenuItemButton(Text("Sub Menu 10"))'));
+
+      await hoverOver(tester, find.text(TestMenu.subMenu12.label));
+      await tester.pump();
+      expect(focusedMenu, equals('MenuItemButton(Text("Sub Menu 12"))'));
+
+      await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
+      expect(focusedMenu, equals('MenuItemButton(Text("Sub Menu 12"))'));
+    });
+
+    testWidgets('scrolling does not trigger hover traversal', (WidgetTester tester) async {
+      // Regression test for https://github.com/flutter/flutter/issues/150911
+      final GlobalKey scrolledMenuItemKey = GlobalKey();
+      await tester.pumpWidget(
+        MaterialApp(
+          home: Material(
+            child: MenuAnchor(
+              style: const MenuStyle(
+                fixedSize: WidgetStatePropertyAll<Size>(Size.fromHeight(200)),
+              ),
+              controller: controller,
+              menuChildren: <Widget>[
+                for (int i = 0; i < 20; i++)
+                  MenuItemButton(
+                    key: i == 15 ? scrolledMenuItemKey : null,
+                    onPressed: () {},
+                    child: Text('Item $i'),
+                  )
+              ]
+            ),
+          ),
+        ),
+      );
+
+      listenForFocusChanges();
+
+      controller.open();
+      await tester.pumpAndSettle();
+
+      await hoverOver(tester, find.text('Item 1'));
+      await tester.pump();
+      expect(focusedMenu, equals('MenuItemButton(Text("Item 1"))'));
+
+      // Scroll the menu while the pointer is over a menu item. The focus should
+      // not change.
+      tester.renderObject(find.text('Item 15')).showOnScreen();
+      await tester.pumpAndSettle();
+      expect(focusedMenu, equals('MenuItemButton(Text("Item 1"))'));
+
+      // Traverse with the keyboard to test that the menu scrolls without hover
+      // focus affecting the focused menu.
+      for (int i = 2; i < 20; i++) {
+        await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
+        await tester.pump();
+        expect(focusedMenu, equals('MenuItemButton(Text("Item $i"))'));
+      }
+    });
+
     testWidgets('menus close on ancestor scroll', (WidgetTester tester) async {
       final ScrollController scrollController = ScrollController();
       addTearDown(scrollController.dispose);