Unverified Commit 13a0d475 authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Convert menus to use OverlayPortal (#130534)

## Description

This converts the `MenuAnchor` class to use `OverlayPortal` instead of directly using the overlay.

## Related Issues
 - Fixes https://github.com/flutter/flutter/issues/124830

## Tests
 - No tests yet (hence it is a draft)
parent 1599cbeb
...@@ -6,11 +6,12 @@ import 'package:flutter/material.dart'; ...@@ -6,11 +6,12 @@ import 'package:flutter/material.dart';
/// Flutter code sample for [MenuAnchor]. /// Flutter code sample for [MenuAnchor].
// This is the type used by the menu below.
enum SampleItem { itemOne, itemTwo, itemThree }
void main() => runApp(const MenuAnchorApp()); void main() => runApp(const MenuAnchorApp());
// This is the type used by the menu below.
enum SampleItem { itemOne, itemTwo, itemThree }
class MenuAnchorApp extends StatelessWidget { class MenuAnchorApp extends StatelessWidget {
const MenuAnchorApp({super.key}); const MenuAnchorApp({super.key});
...@@ -36,7 +37,10 @@ class _MenuAnchorExampleState extends State<MenuAnchorExample> { ...@@ -36,7 +37,10 @@ class _MenuAnchorExampleState extends State<MenuAnchorExample> {
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
return Scaffold( return Scaffold(
appBar: AppBar(title: const Text('MenuAnchorButton')), appBar: AppBar(
title: const Text('MenuAnchorButton'),
backgroundColor: Theme.of(context).primaryColorLight,
),
body: Center( body: Center(
child: MenuAnchor( child: MenuAnchor(
builder: builder:
......
...@@ -51,7 +51,18 @@ void main() { ...@@ -51,7 +51,18 @@ void main() {
expect(find.text('Background Color'), findsOneWidget); expect(find.text('Background Color'), findsOneWidget);
// Focusing the background color item with the keyboard caused the submenu
// to open. Tapping it should cause it to close.
await tester.tap(find.text('Background Color')); await tester.tap(find.text('Background Color'));
await tester.pump();
await tester.pumpAndSettle();
expect(find.text(example.MenuEntry.colorRed.label), findsNothing);
expect(find.text(example.MenuEntry.colorGreen.label), findsNothing);
expect(find.text(example.MenuEntry.colorBlue.label), findsNothing);
await tester.tap(find.text('Background Color'));
await tester.pump();
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(find.text(example.MenuEntry.colorRed.label), findsOneWidget); expect(find.text(example.MenuEntry.colorRed.label), findsOneWidget);
......
...@@ -257,7 +257,7 @@ abstract class ViewportOffset extends ChangeNotifier { ...@@ -257,7 +257,7 @@ abstract class ViewportOffset extends ChangeNotifier {
/// Whether a viewport is allowed to change [pixels] implicitly to respond to /// Whether a viewport is allowed to change [pixels] implicitly to respond to
/// a call to [RenderObject.showOnScreen]. /// a call to [RenderObject.showOnScreen].
/// ///
/// [RenderObject.showOnScreen] is for example used to bring a text field /// [RenderObject.showOnScreen] is, for example, used to bring a text field
/// fully on screen after it has received focus. This property controls /// fully on screen after it has received focus. This property controls
/// whether the viewport associated with this offset is allowed to change the /// whether the viewport associated with this offset is allowed to change the
/// offset's [pixels] value to fulfill such a request. /// offset's [pixels] value to fulfill such a request.
......
...@@ -189,7 +189,8 @@ abstract class FocusTraversalPolicy with Diagnosticable { ...@@ -189,7 +189,8 @@ abstract class FocusTraversalPolicy with Diagnosticable {
}) { }) {
node.requestFocus(); node.requestFocus();
Scrollable.ensureVisible( Scrollable.ensureVisible(
node.context!, alignment: alignment ?? 1.0, node.context!,
alignment: alignment ?? 1,
alignmentPolicy: alignmentPolicy ?? ScrollPositionAlignmentPolicy.explicit, alignmentPolicy: alignmentPolicy ?? ScrollPositionAlignmentPolicy.explicit,
duration: duration ?? Duration.zero, duration: duration ?? Duration.zero,
curve: curve ?? Curves.ease, curve: curve ?? Curves.ease,
...@@ -467,7 +468,6 @@ abstract class FocusTraversalPolicy with Diagnosticable { ...@@ -467,7 +468,6 @@ abstract class FocusTraversalPolicy with Diagnosticable {
groups[key]!.members.addAll(sortedMembers); groups[key]!.members.addAll(sortedMembers);
} }
// Traverse the group tree, adding the children of members in the order they // Traverse the group tree, adding the children of members in the order they
// appear in the member lists. // appear in the member lists.
final List<FocusNode> sortedDescendants = <FocusNode>[]; final List<FocusNode> sortedDescendants = <FocusNode>[];
...@@ -504,9 +504,9 @@ abstract class FocusTraversalPolicy with Diagnosticable { ...@@ -504,9 +504,9 @@ abstract class FocusTraversalPolicy with Diagnosticable {
// The scope.traversalDescendants will not contain currentNode if it // The scope.traversalDescendants will not contain currentNode if it
// skips traversal or not focusable. // skips traversal or not focusable.
assert( assert(
difference.length == 1 && difference.contains(currentNode), difference.isEmpty || (difference.length == 1 && difference.contains(currentNode)),
'Sorted descendants contains different nodes than FocusScopeNode.traversalDescendants would. ' 'Difference between sorted descendants and FocusScopeNode.traversalDescendants contains '
'These are the different nodes: ${difference.where((FocusNode node) => node != currentNode)}', 'something other than the current skipped node. This is the difference: $difference',
); );
return true; return true;
} }
......
...@@ -1414,7 +1414,7 @@ class OverlayPortalController { ...@@ -1414,7 +1414,7 @@ class OverlayPortalController {
: _zOrderIndex != null; : _zOrderIndex != null;
} }
/// Conventience method for toggling the current [isShowing] status. /// Convenience method for toggling the current [isShowing] status.
/// ///
/// This method should typically not be called while the widget tree is being /// This method should typically not be called while the widget tree is being
/// rebuilt. /// rebuilt.
......
...@@ -795,9 +795,13 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { ...@@ -795,9 +795,13 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
Curve curve = Curves.ease, Curve curve = Curves.ease,
ScrollPositionAlignmentPolicy alignmentPolicy = ScrollPositionAlignmentPolicy.explicit, ScrollPositionAlignmentPolicy alignmentPolicy = ScrollPositionAlignmentPolicy.explicit,
RenderObject? targetRenderObject, RenderObject? targetRenderObject,
}) { }) async {
assert(object.attached); assert(object.attached);
final RenderAbstractViewport viewport = RenderAbstractViewport.of(object); final RenderAbstractViewport? viewport = RenderAbstractViewport.maybeOf(object);
// If no viewport is found, return.
if (viewport == null) {
return;
}
Rect? targetRect; Rect? targetRect;
if (targetRenderObject != null && targetRenderObject != object) { if (targetRenderObject != null && targetRenderObject != object) {
...@@ -842,12 +846,12 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { ...@@ -842,12 +846,12 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
} }
if (target == pixels) { if (target == pixels) {
return Future<void>.value(); return;
} }
if (duration == Duration.zero) { if (duration == Duration.zero) {
jumpTo(target); jumpTo(target);
return Future<void>.value(); return;
} }
return animateTo(target, duration: duration, curve: curve); return animateTo(target, duration: duration, curve: curve);
......
...@@ -461,11 +461,12 @@ class Scrollable extends StatefulWidget { ...@@ -461,11 +461,12 @@ class Scrollable extends StatefulWidget {
}) { }) {
final List<Future<void>> futures = <Future<void>>[]; final List<Future<void>> futures = <Future<void>>[];
// The `targetRenderObject` is used to record the first target renderObject. // The targetRenderObject is used to record the first target renderObject.
// If there are multiple scrollable widgets nested, we should let // If there are multiple scrollable widgets nested, the targetRenderObject
// the `targetRenderObject` as visible as possible to improve the user experience. // is made to be as visible as possible to improve the user experience. If
// Otherwise, let the outer renderObject as visible as possible maybe cause // the targetRenderObject is already visible, then let the outer
// the `targetRenderObject` invisible. // renderObject be as visible as possible.
//
// Also see https://github.com/flutter/flutter/issues/65100 // Also see https://github.com/flutter/flutter/issues/65100
RenderObject? targetRenderObject; RenderObject? targetRenderObject;
ScrollableState? scrollable = Scrollable.maybeOf(context); ScrollableState? scrollable = Scrollable.maybeOf(context);
...@@ -481,7 +482,7 @@ class Scrollable extends StatefulWidget { ...@@ -481,7 +482,7 @@ class Scrollable extends StatefulWidget {
); );
futures.addAll(newFutures); futures.addAll(newFutures);
targetRenderObject = targetRenderObject ?? context.findRenderObject(); targetRenderObject ??= context.findRenderObject();
context = scrollable.context; context = scrollable.context;
scrollable = Scrollable.maybeOf(context); scrollable = Scrollable.maybeOf(context);
} }
......
...@@ -61,7 +61,7 @@ void main() { ...@@ -61,7 +61,7 @@ void main() {
final Finder menuMaterial = find.ancestor( final Finder menuMaterial = find.ancestor(
of: find.widgetWithText(TextButton, TestMenu.mainMenu0.label), of: find.widgetWithText(TextButton, TestMenu.mainMenu0.label),
matching: find.byType(Material), matching: find.byType(Material),
).last; ).at(1);
Material material = tester.widget<Material>(menuMaterial); Material material = tester.widget<Material>(menuMaterial);
expect(material.color, themeData.colorScheme.surface); expect(material.color, themeData.colorScheme.surface);
expect(material.shadowColor, themeData.colorScheme.shadow); expect(material.shadowColor, themeData.colorScheme.shadow);
...@@ -143,7 +143,7 @@ void main() { ...@@ -143,7 +143,7 @@ void main() {
final Finder menuMaterial = find.ancestor( final Finder menuMaterial = find.ancestor(
of: find.byType(SingleChildScrollView), of: find.byType(SingleChildScrollView),
matching: find.byType(Material), matching: find.byType(Material),
); ).first;
final Size menuSize = tester.getSize(menuMaterial); final Size menuSize = tester.getSize(menuMaterial);
expect(menuSize, const Size(180.0, 304.0)); expect(menuSize, const Size(180.0, 304.0));
...@@ -161,7 +161,7 @@ void main() { ...@@ -161,7 +161,7 @@ void main() {
final Finder updatedMenu = find.ancestor( final Finder updatedMenu = find.ancestor(
of: find.byType(SingleChildScrollView), of: find.byType(SingleChildScrollView),
matching: find.byType(Material), matching: find.byType(Material),
); ).first;
final double updatedMenuWidth = tester.getSize(updatedMenu).width; final double updatedMenuWidth = tester.getSize(updatedMenu).width;
expect(updatedMenuWidth, 200.0); expect(updatedMenuWidth, 200.0);
}); });
...@@ -192,7 +192,7 @@ void main() { ...@@ -192,7 +192,7 @@ void main() {
final Finder menuMaterial = find.ancestor( final Finder menuMaterial = find.ancestor(
of: find.byType(SingleChildScrollView), of: find.byType(SingleChildScrollView),
matching: find.byType(Material), matching: find.byType(Material),
); ).first;
final double menuWidth = tester.getSize(menuMaterial).width; final double menuWidth = tester.getSize(menuMaterial).width;
expect(menuWidth, closeTo(180.5, 0.1)); expect(menuWidth, closeTo(180.5, 0.1));
...@@ -210,7 +210,7 @@ void main() { ...@@ -210,7 +210,7 @@ void main() {
final Finder updatedMenu = find.ancestor( final Finder updatedMenu = find.ancestor(
of: find.byType(SingleChildScrollView), of: find.byType(SingleChildScrollView),
matching: find.byType(Material), matching: find.byType(Material),
); ).first;
final double updatedMenuWidth = tester.getSize(updatedMenu).width; final double updatedMenuWidth = tester.getSize(updatedMenu).width;
expect(updatedMenuWidth, 200.0); expect(updatedMenuWidth, 200.0);
}); });
...@@ -644,8 +644,8 @@ void main() { ...@@ -644,8 +644,8 @@ void main() {
final Finder menuMaterial = find.ancestor( final Finder menuMaterial = find.ancestor(
of: find.widgetWithText(MenuItemButton, TestMenu.mainMenu0.label), of: find.widgetWithText(MenuItemButton, TestMenu.mainMenu0.label),
matching: find.byType(Material), matching: find.byType(Material),
).last; );
expect(menuMaterial, findsOneWidget); expect(menuMaterial, findsNWidgets(3));
// didChangeMetrics // didChangeMetrics
tester.view.physicalSize = const Size(700.0, 700.0); tester.view.physicalSize = const Size(700.0, 700.0);
......
...@@ -80,7 +80,7 @@ void main() { ...@@ -80,7 +80,7 @@ void main() {
final Finder menuMaterial = find.ancestor( final Finder menuMaterial = find.ancestor(
of: find.widgetWithText(TextButton, 'Item 0'), of: find.widgetWithText(TextButton, 'Item 0'),
matching: find.byType(Material), matching: find.byType(Material),
).last; ).at(1);
Material material = tester.widget<Material>(menuMaterial); Material material = tester.widget<Material>(menuMaterial);
expect(material.color, themeData.colorScheme.surface); expect(material.color, themeData.colorScheme.surface);
expect(material.shadowColor, themeData.colorScheme.shadow); expect(material.shadowColor, themeData.colorScheme.shadow);
...@@ -159,7 +159,7 @@ void main() { ...@@ -159,7 +159,7 @@ void main() {
final Finder menuMaterial = find.ancestor( final Finder menuMaterial = find.ancestor(
of: find.widgetWithText(TextButton, 'Item 0'), of: find.widgetWithText(TextButton, 'Item 0'),
matching: find.byType(Material), matching: find.byType(Material),
).last; ).at(1);
Material material = tester.widget<Material>(menuMaterial); Material material = tester.widget<Material>(menuMaterial);
expect(material.color, Colors.grey); expect(material.color, Colors.grey);
expect(material.shadowColor, Colors.brown); expect(material.shadowColor, Colors.brown);
...@@ -262,7 +262,7 @@ void main() { ...@@ -262,7 +262,7 @@ void main() {
final Finder menuMaterial = find.ancestor( final Finder menuMaterial = find.ancestor(
of: find.widgetWithText(TextButton, 'Item 0'), of: find.widgetWithText(TextButton, 'Item 0'),
matching: find.byType(Material), matching: find.byType(Material),
).last; ).at(1);
Material material = tester.widget<Material>(menuMaterial); Material material = tester.widget<Material>(menuMaterial);
expect(material.color, Colors.yellow); expect(material.color, Colors.yellow);
expect(material.shadowColor, Colors.green); expect(material.shadowColor, Colors.green);
...@@ -383,7 +383,7 @@ void main() { ...@@ -383,7 +383,7 @@ void main() {
final Finder menuMaterial = find.ancestor( final Finder menuMaterial = find.ancestor(
of: find.widgetWithText(TextButton, 'Item 0'), of: find.widgetWithText(TextButton, 'Item 0'),
matching: find.byType(Material), matching: find.byType(Material),
).last; ).at(1);
Material material = tester.widget<Material>(menuMaterial); Material material = tester.widget<Material>(menuMaterial);
expect(material.color, Colors.limeAccent); expect(material.color, Colors.limeAccent);
expect(material.shadowColor, Colors.deepOrangeAccent); expect(material.shadowColor, Colors.deepOrangeAccent);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment