firka/flutter
firka/flutter
4
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

MenuAnchor hover traversal fixes (#150914)

Browse Source 
Fixes https://github.com/flutter/flutter/issues/150910 and https://github.com/flutter/flutter/issues/150911. 

https://github.com/flutter/flutter/issues/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.

https://github.com/flutter/flutter/issues/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
This commit is contained in:
davidhicks980 2024-08-07 13:43:00 -04:00 committed by GitHub
parent 0f7bceb9c4
commit 1f4c6ebc97
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 166 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
packages/flutter/lib/src/material/menu_anchor.dart
View File

@ -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) {
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,9 +1961,21 @@ 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);
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
@ -1945,7 +1984,6 @@ class _SubmenuButtonState extends State<SubmenuButton> {
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,

97
packages/flutter/test/material/menu_anchor_test.dart
View File

@ -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);