firka/flutter
3
0
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Clean up MenuAnchor (#152946)

Browse Source 
## Description

This PR cleans up some typos and formatting issues in the `material/menu_anchor.dart` file and associated tests.
This commit is contained in:
Bruno Leroux
2024-08-08 01:39:24 +02:00
committed by GitHub
parent dc4d64c9c2
commit 64b373b1f2
2 changed files with 122 additions and 140 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -164,7 +164,7 @@ class MenuAnchor extends StatefulWidget {
final MenuController? controller;
/// The [childFocusNode] attribute is the optional [FocusNode] also associated
/// the [child] or [builder] widget that opens the menu.
/// to the [child] or [builder] widget that opens the menu.
///
/// The focus node should be attached to the widget that should receive focus
/// if keyboard focus traversal moves the focus off of the submenu with the
@@ -390,7 +390,7 @@ class _MenuAnchorState extends State<MenuAnchor> {
Widget child = OverlayPortal(
controller: _overlayController,
overlayChildBuilder: (BuildContext context) {
return _Submenu(
return _Submenu(
anchor: this,
menuStyle: widget.style,
alignmentOffset: widget.alignmentOffset ?? Offset.zero,
@@ -472,7 +472,7 @@ class _MenuAnchorState extends State<MenuAnchor> {
assert(_debugMenuInfo('Removing:\n${child.widget.toStringDeep()}'));
_anchorChildren.remove(child);
assert(_debugMenuInfo('Tree:\n${widget.toStringDeep()}'));
}
}
_MenuAnchorState get _root {
_MenuAnchorState anchor = this;
@@ -496,7 +496,6 @@ class _MenuAnchorState extends State<MenuAnchor> {
});
});
}
}
void _focusButton() {
@@ -886,7 +885,7 @@ class MenuItemButton extends StatefulWidget {
/// A screen reader will default to reading the derived text on the
/// [MenuItemButton] itself, which is not guaranteed to be readable.
/// (For some shortcuts, such as comma, semicolon, and other
/// punctuation, screen readers read silence)
/// punctuation, screen readers read silence).
///
/// Setting this label overwrites the semantics properties of the entire
/// Widget, including its children. Consider wrapping this widget in
@@ -934,7 +933,7 @@ class MenuItemButton extends StatefulWidget {
/// this property is ignored.
///
/// If [overflowAxis] is [Axis.vertical], the menu will be expanded vertically.
/// If [overflowAxis] is [Axis.horizontal], then the menu will be
/// If [overflowAxis] is [Axis.horizontal], then the menu will be
/// expanded horizontally.
///
/// Defaults to [Axis.horizontal].
@@ -1940,96 +1939,96 @@ class _SubmenuButtonState extends State<SubmenuButton> {
onClose: _onClose,
onOpen: _onOpen,
style: widget.menuStyle,
builder:
(BuildContext context, MenuController controller, Widget? child) {
builder: (BuildContext context, MenuController controller, Widget? child) {
// Since we don't want to use the theme style or default style from the
// TextButton, we merge the styles, merging them in the right order when
// each type of style exists. Each "*StyleOf" function is only called
// once.
ButtonStyle mergedStyle = widget.themeStyleOf(context)?.merge(widget.defaultStyleOf(context))
?? widget.defaultStyleOf(context);
mergedStyle = widget.style?.merge(mergedStyle) ?? mergedStyle;
ButtonStyle mergedStyle = widget.themeStyleOf(context)?.merge(widget.defaultStyleOf(context))
?? widget.defaultStyleOf(context);
mergedStyle = widget.style?.merge(mergedStyle) ?? mergedStyle;
void toggleShowMenu() {
if (controller._anchor == null) {
return;
}
if (controller.isOpen) {
controller.close();
} else {
controller.open();
}
}
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) {
void toggleShowMenu() {
if (controller._anchor == null) {
return;
}
controller.open();
controller._anchor!._focusButton();
if (controller.isOpen) {
controller.close();
} else {
controller.open();
}
}
}
child = MergeSemantics(
child: Semantics(
expanded: _enabled && controller.isOpen,
child: TextButton(
style: mergedStyle,
focusNode: _buttonFocusNode,
onFocusChange: _enabled ? widget.onFocusChange : null,
onPressed: _enabled ? toggleShowMenu : null,
isSemanticButton: null,
child: _MenuItemLabel(
leadingIcon: widget.leadingIcon,
trailingIcon: widget.trailingIcon,
hasSubmenu: true,
showDecoration: (controller._anchor!._parent?._orientation ?? Axis.horizontal) == Axis.vertical,
child: child,
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;
}
controller.open();
controller._anchor!._focusButton();
}
}
child = MergeSemantics(
child: Semantics(
expanded: _enabled && controller.isOpen,
child: TextButton(
style: mergedStyle,
focusNode: _buttonFocusNode,
onFocusChange: _enabled ? widget.onFocusChange : null,
onPressed: _enabled ? toggleShowMenu : null,
isSemanticButton: null,
child: _MenuItemLabel(
leadingIcon: widget.leadingIcon,
trailingIcon: widget.trailingIcon,
hasSubmenu: true,
showDecoration: (controller._anchor!._parent?._orientation ?? Axis.horizontal) == Axis.vertical,
child: child,
),
),
),
),
);
);
if (!_enabled) {
return child;
}
if (!_enabled) {
return child;
}
child = MouseRegion(
onHover: handlePointerHover,
onExit: handlePointerExit,
child: child,
);
if (_platformSupportsAccelerators) {
return MenuAcceleratorCallbackBinding(
onInvoke: toggleShowMenu,
hasSubmenu: true,
child = MouseRegion(
onHover: handlePointerHover,
onExit: handlePointerExit,
child: child,
);
}
return child;
},
menuChildren: widget.menuChildren,
child: widget.child,
if (_platformSupportsAccelerators) {
return MenuAcceleratorCallbackBinding(
onInvoke: toggleShowMenu,
hasSubmenu: true,
child: child,
);
}
return child;
},
menuChildren: widget.menuChildren,
child: widget.child,
)
);
}
@@ -2089,7 +2088,9 @@ class _SubmenuDirectionalFocusAction extends DirectionalFocusAction {
_SubmenuDirectionalFocusAction({
required this.submenu,
});
final _SubmenuButtonState submenu;
_MenuAnchorState get _anchor => submenu._menuController._anchor!;
FocusNode get _buttonFocusNode => submenu._buttonFocusNode;
_MenuAnchorState? get _parent => _anchor._parent;
@@ -2292,11 +2293,11 @@ class _LocalizedShortcutLabeler {
final ShortcutSerialization serialized = shortcut.serializeForMenu();
final String keySeparator;
if (_usesSymbolicModifiers) {
// Use "⌃ ⇧ A" style on macOS and iOS.
keySeparator = ' ';
// Use "⌃ ⇧ A" style on macOS and iOS.
keySeparator = ' ';
} else {
// Use "Ctrl+Shift+A" style.
keySeparator = '+';
// Use "Ctrl+Shift+A" style.
keySeparator = '+';
}
if (serialized.trigger != null) {
final LogicalKeyboardKey trigger = serialized.trigger!;
@@ -3161,7 +3162,7 @@ class _MenuLayout extends SingleChildLayoutDelegate {
// List of rectangles that we should avoid overlapping. Unusable screen area.
final Set<Rect> avoidBounds;
// The orientation of this menu
// The orientation of this menu.
final Axis orientation;
// The orientation of this menu's parent.

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

@@ -97,7 +97,8 @@ void main() {
onTap: () {
onPressed?.call(TestMenu.outsideButton);
},
child: Text(TestMenu.outsideButton.label)),
child: Text(TestMenu.outsideButton.label),
),
MenuAnchor(
childFocusNode: focusNode,
controller: controller,
@@ -209,7 +210,7 @@ void main() {
equals(const Rect.fromLTRB(257.0, 48.0, 471.0, 208.0)),
);
// Test compact visual density (-2, -2)
// Test compact visual density (-2, -2).
await tester.pumpWidget(Container());
await tester.pumpWidget(buildMenu(visualDensity: VisualDensity.compact));
await tester.pump();
@@ -284,7 +285,7 @@ void main() {
),
);
// menu bar(horizontal menu)
// Menu bar (horizontal menu).
Finder menuMaterial = find
.ancestor(
of: find.byType(TextButton),
@@ -314,7 +315,7 @@ void main() {
expect(material.textStyle?.fontSize, 14.0);
expect(material.textStyle?.height, 1.43);
// vertical menu
// Vertical menu.
await tester.tap(find.text(TestMenu.mainMenu1.label));
await tester.pump();
@@ -374,7 +375,7 @@ void main() {
),
);
// menu bar(horizontal menu)
// Menu bar (horizontal menu).
Finder menuMaterial = find
.ancestor(
of: find.widgetWithText(TextButton, TestMenu.mainMenu5.label),
@@ -402,7 +403,7 @@ void main() {
expect(material.shape, const RoundedRectangleBorder());
expect(material.textStyle?.color, themeData.colorScheme.onSurface.withOpacity(0.38));
// vertical menu
// Vertical menu.
await tester.tap(find.text(TestMenu.mainMenu2.label));
await tester.pump();
@@ -506,14 +507,10 @@ void main() {
),
),
onPressed: () {},
child: const Text(
'Category',
),
child: const Text('Category'),
),
],
child: const Text(
'Main Menu',
),
child: const Text('Main Menu'),
),
],
),
@@ -532,7 +529,7 @@ void main() {
);
}, variant: TargetPlatformVariant.desktop());
testWidgets('focus is returned to previous focus before invoking onPressed', (WidgetTester tester) async {
testWidgets('Focus is returned to previous focus before invoking onPressed', (WidgetTester tester) async {
final FocusNode buttonFocus = FocusNode(debugLabel: 'Button Focus');
addTearDown(buttonFocus.dispose);
FocusNode? focusInOnPressed;
@@ -1073,8 +1070,10 @@ void main() {
);
await tester.tap(find.text('Tap me'));
await tester.pump();
// Test default clip behavior.
expect(getMenuBarMaterial(tester).clipBehavior, equals(Clip.hardEdge));
// Close the menu.
await tester.tapAt(const Offset(10.0, 10.0));
await tester.pumpAndSettle();
@@ -1104,6 +1103,7 @@ void main() {
);
await tester.tap(find.text('Tap me'));
await tester.pump();
// Test custom clip behavior.
expect(getMenuBarMaterial(tester).clipBehavior, equals(Clip.antiAlias));
});
@@ -1457,7 +1457,7 @@ void main() {
await tester.pump();
expect(focusedMenu, equals('SubmenuButton(Text("Sub Menu 11"))'));
// Open the next submenu
// Open the next submenu.
await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight);
await tester.pump();
expect(focusedMenu, equals('MenuItemButton(Text("Sub Sub Menu 110"))'));
@@ -1505,7 +1505,6 @@ void main() {
await tester.pump();
expect(focusedMenu, equals('SubmenuButton(Text("Menu 0"))'));
await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight);
await tester.pump();
expect(focusedMenu, equals('SubmenuButton(Text("Menu 1"))'));
@@ -1577,7 +1576,7 @@ void main() {
await tester.pump();
expect(focusedMenu, equals('SubmenuButton(Text("Sub Menu 11"))'));
// Open the next submenu
// Open the next submenu.
await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft);
await tester.pump();
expect(focusedMenu, equals('MenuItemButton(Text("Sub Sub Menu 110"))'));
@@ -1625,7 +1624,6 @@ void main() {
await tester.pump();
expect(focusedMenu, equals('SubmenuButton(Text("Menu 0"))'));
await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft);
await tester.pump();
expect(focusedMenu, equals('SubmenuButton(Text("Menu 1"))'));
@@ -1841,20 +1839,19 @@ void main() {
expect(focusedMenu, equals('SubmenuButton(Text("Menu 0"))'));
expect(find.text('Sub Menu 00'), findsNothing);
// Open the submenu again
// Open the submenu again.
await tester.sendKeyEvent(LogicalKeyboardKey.space);
await tester.pump();
expect(focusedMenu, equals('SubmenuButton(Text("Menu 0"))'));
expect(find.text('Sub Menu 00'), findsOne);
// Close all menus
// Close all menus.
await tester.sendKeyEvent(LogicalKeyboardKey.escape);
await tester.pump();
expect(focusedMenu, equals(TestMenu.anchorButton.label));
expect(find.byType(MenuItemButton), findsNothing);
});
testWidgets('MenuAnchor RTL directional traversal works', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/119532
final FocusNode buttonFocusNode = FocusNode(debugLabel: TestMenu.anchorButton.label);
@@ -1903,7 +1900,7 @@ void main() {
listenForFocusChanges();
await tester.sendKeyEvent(LogicalKeyboardKey.tab);
await tester.sendKeyEvent(LogicalKeyboardKey.tab);
expect(focusedMenu, equals(TestMenu.anchorButton.label));
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
@@ -1952,13 +1949,13 @@ void main() {
expect(focusedMenu, equals('SubmenuButton(Text("Menu 0"))'));
expect(find.text('Sub Menu 00'), findsNothing);
// Open the submenu again
// Open the submenu again.
await tester.sendKeyEvent(LogicalKeyboardKey.space);
await tester.pump();
expect(focusedMenu, equals('SubmenuButton(Text("Menu 0"))'));
expect(find.text('Sub Menu 00'), findsOne);
// Close all menus
// Close all menus.
await tester.sendKeyEvent(LogicalKeyboardKey.escape);
await tester.pump();
expect(focusedMenu, equals(TestMenu.anchorButton.label));
@@ -2021,9 +2018,8 @@ 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
// Regression test for https://github.com/flutter/flutter/issues/150910.
await tester.pumpWidget(
MaterialApp(
home: Material(
@@ -2050,7 +2046,7 @@ void main() {
await tester.pump();
expect(focusedMenu, equals('MenuItemButton(Text("Sub Menu 12"))'));
// Move pointer to disabled menu
// Move pointer to disabled menu.
await hoverOver(tester, find.text(TestMenu.mainMenu5.label));
await tester.pump();
expect(focusedMenu, equals('MenuItemButton(Text("Sub Menu 12"))'));
@@ -2071,7 +2067,7 @@ void main() {
});
testWidgets('scrolling does not trigger hover traversal', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/150911
// Regression test for https://github.com/flutter/flutter/issues/150911.
final GlobalKey scrolledMenuItemKey = GlobalKey();
await tester.pumpWidget(
MaterialApp(
@@ -2601,7 +2597,7 @@ void main() {
opened.clear();
closed.clear();
// Close menus using the controller
// Close menus using the controller.
controller.close();
await tester.pump();
@@ -2638,41 +2634,29 @@ void main() {
await tester.tap(find.text(TestMenu.subMenu11.label));
await tester.pump();
Text mnemonic0;
Text mnemonic1;
Text mnemonic2;
Text mnemonic3;
Text mnemonic0 = tester.widget(findMnemonic(TestMenu.subSubMenu110.label));
Text mnemonic1 = tester.widget(findMnemonic(TestMenu.subSubMenu111.label));
Text mnemonic2 = tester.widget(findMnemonic(TestMenu.subSubMenu112.label));
Text mnemonic3 = tester.widget(findMnemonic(TestMenu.subSubMenu113.label));
switch (defaultTargetPlatform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
case TargetPlatform.linux:
mnemonic0 = tester.widget(findMnemonic(TestMenu.subSubMenu110.label));
expect(mnemonic0.data, equals('Ctrl+A'));
mnemonic1 = tester.widget(findMnemonic(TestMenu.subSubMenu111.label));
expect(mnemonic1.data, equals('Shift+B'));
mnemonic2 = tester.widget(findMnemonic(TestMenu.subSubMenu112.label));
expect(mnemonic2.data, equals('Alt+C'));
mnemonic3 = tester.widget(findMnemonic(TestMenu.subSubMenu113.label));
expect(mnemonic3.data, equals('Meta+D'));
case TargetPlatform.windows:
mnemonic0 = tester.widget(findMnemonic(TestMenu.subSubMenu110.label));
expect(mnemonic0.data, equals('Ctrl+A'));
mnemonic1 = tester.widget(findMnemonic(TestMenu.subSubMenu111.label));
expect(mnemonic1.data, equals('Shift+B'));
mnemonic2 = tester.widget(findMnemonic(TestMenu.subSubMenu112.label));
expect(mnemonic2.data, equals('Alt+C'));
mnemonic3 = tester.widget(findMnemonic(TestMenu.subSubMenu113.label));
expect(mnemonic3.data, equals('Win+D'));
case TargetPlatform.iOS:
case TargetPlatform.macOS:
mnemonic0 = tester.widget(findMnemonic(TestMenu.subSubMenu110.label));
expect(mnemonic0.data, equals('⌃ A'));
mnemonic1 = tester.widget(findMnemonic(TestMenu.subSubMenu111.label));
expect(mnemonic1.data, equals('⇧ B'));
mnemonic2 = tester.widget(findMnemonic(TestMenu.subSubMenu112.label));
expect(mnemonic2.data, equals('⌥ C'));
mnemonic3 = tester.widget(findMnemonic(TestMenu.subSubMenu113.label));
expect(mnemonic3.data, equals('⌘ D'));
}
@@ -2815,9 +2799,7 @@ void main() {
expect(find.text('leadingIcon'), findsOneWidget);
});
testWidgets('autofocus is used when set and widget is enabled',
(WidgetTester tester) async {
testWidgets('autofocus is used when set and widget is enabled', (WidgetTester tester) async {
listenForFocusChanges();
await tester.pumpWidget(
@@ -3028,7 +3010,7 @@ void main() {
await tester.pump();
expect(find.byType(MenuItemButton), findsNWidgets(1));
// Taps the MenuItemButton which should close the menu
// Taps the MenuItemButton which should close the menu.
await tester.tap(find.text('Button 1'));
await tester.pump();
expect(find.byType(MenuItemButton), findsNWidgets(0));
@@ -3064,7 +3046,7 @@ void main() {
await tester.pump();
expect(find.byType(MenuItemButton), findsNWidgets(1));
// Taps the MenuItemButton which shouldn't close the menu
// Taps the MenuItemButton which shouldn't close the menu.
await tester.tap(find.text('Button 1'));
await tester.pump();
expect(find.byType(MenuItemButton), findsNWidgets(1));
@@ -3159,6 +3141,7 @@ void main() {
);
});
// Regression test for https://github.com/flutter/flutter/issues/147479.
testWidgets('MenuItemButton can build when its child is null', (WidgetTester tester) async {
await tester.pumpWidget(
const MaterialApp(
@@ -3171,7 +3154,6 @@ void main() {
),
);
// exception `Null check operator used on a null value` would be thrown.
expect(tester.takeException(), isNull);
});
});
@@ -3639,8 +3621,7 @@ void main() {
);
});
testWidgets('vertically constrained menus are positioned above the anchor with the provided offset',
(WidgetTester tester) async {
testWidgets('vertically constrained menus are positioned above the anchor with the provided offset', (WidgetTester tester) async {
await changeSurfaceSize(tester, const Size(800, 600));
await tester.pumpWidget(
MaterialApp(
@@ -4038,7 +4019,7 @@ void main() {
),
);
// The flags should not have SemanticsFlag.isButton
// The flags should not have SemanticsFlag.isButton.
expect(
semantics,
hasSemantics(
@@ -4105,7 +4086,7 @@ void main() {
),
);
// The flags should not have SemanticsFlag.isButton
// The flags should not have SemanticsFlag.isButton.
expect(
semantics,
hasSemantics(