Skip to content

Commit

Permalink
The initial/selected item on popup menu should always be visible (#14…
Browse files Browse the repository at this point in the history
…3118)

Fixes #142895

With the change of #143121, this PR is to add auto scroll to `PopupMenuButton` so when we open the menu, it will automatically scroll to the selected item.

https://github.com/flutter/flutter/assets/36861262/c2bc0395-0641-4e7a-a54d-57a8e62ee26f
  • Loading branch information
QuncCccccc authored Feb 14, 2024
1 parent db83bc6 commit 9bc8393
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 0 deletions.
20 changes: 20 additions & 0 deletions packages/flutter/lib/src/material/popup_menu.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import 'package:flutter/foundation.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/scheduler.dart';
import 'package:flutter/widgets.dart';

import 'color_scheme.dart';
Expand Down Expand Up @@ -570,12 +571,14 @@ class _CheckedPopupMenuItemState<T> extends PopupMenuItemState<T, CheckedPopupMe
class _PopupMenu<T> extends StatelessWidget {
const _PopupMenu({
super.key,
required this.itemKeys,
required this.route,
required this.semanticLabel,
this.constraints,
required this.clipBehavior,
});

final List<GlobalKey> itemKeys;
final _PopupMenuRoute<T> route;
final String? semanticLabel;
final BoxConstraints? constraints;
Expand Down Expand Up @@ -609,6 +612,7 @@ class _PopupMenu<T> extends StatelessWidget {
route.itemSizes[i] = size;
},
child: FadeTransition(
key: itemKeys[i],
opacity: opacity,
child: item,
),
Expand Down Expand Up @@ -791,6 +795,7 @@ class _PopupMenuRoute<T> extends PopupRoute<T> {
_PopupMenuRoute({
required this.position,
required this.items,
required this.itemKeys,
this.initialValue,
this.elevation,
this.surfaceTintColor,
Expand All @@ -811,6 +816,7 @@ class _PopupMenuRoute<T> extends PopupRoute<T> {

final RelativeRect position;
final List<PopupMenuEntry<T>> items;
final List<GlobalKey> itemKeys;
final List<Size?> itemSizes;
final T? initialValue;
final double? elevation;
Expand All @@ -836,6 +842,14 @@ class _PopupMenuRoute<T> extends PopupRoute<T> {
return super.createAnimation();
}

void scrollTo(int selectedItemIndex) {
SchedulerBinding.instance.addPostFrameCallback((_) {
if (itemKeys[selectedItemIndex].currentContext != null) {
Scrollable.ensureVisible(itemKeys[selectedItemIndex].currentContext!);
}
});
}

@override
Duration get transitionDuration => popUpAnimationStyle?.duration ?? _kMenuDuration;

Expand All @@ -859,9 +873,13 @@ class _PopupMenuRoute<T> extends PopupRoute<T> {
}
}
}
if (selectedItemIndex != null) {
scrollTo(selectedItemIndex);
}

final Widget menu = _PopupMenu<T>(
route: this,
itemKeys: itemKeys,
semanticLabel: semanticLabel,
constraints: constraints,
clipBehavior: clipBehavior,
Expand Down Expand Up @@ -985,10 +1003,12 @@ Future<T?> showMenu<T>({
semanticLabel ??= MaterialLocalizations.of(context).popupMenuLabel;
}

final List<GlobalKey> menuItemKeys = List<GlobalKey>.generate(items.length, (int index) => GlobalKey());
final NavigatorState navigator = Navigator.of(context, rootNavigator: useRootNavigator);
return navigator.push(_PopupMenuRoute<T>(
position: position,
items: items,
itemKeys: menuItemKeys,
initialValue: initialValue,
elevation: elevation,
shadowColor: shadowColor,
Expand Down
53 changes: 53 additions & 0 deletions packages/flutter/test/material/popup_menu_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3956,6 +3956,59 @@ void main() {
expect(tester.getSize(find.byType(Material).last), within(distance: 0.1, from: const Size(112.0, 160.0)));
});

testWidgets('PopupMenuButton scrolls initial value/selected value to visible', (WidgetTester tester) async {
const int length = 50;
const int selectedValue = length - 1;
await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: Align(
alignment: Alignment.bottomCenter,
child: PopupMenuButton<int>(
itemBuilder: (BuildContext context) {
return List<PopupMenuEntry<int>>.generate(length, (int index) {
return PopupMenuItem<int>(value: index, child: Text('item #$index'));
});
},
popUpAnimationStyle: AnimationStyle.noAnimation,
initialValue: selectedValue,
child: const Text('click here'),
),
),
),
),
);
await tester.tap(find.text('click here'));
await tester.pump();

// Set up finder and verify basic widget structure.
final Finder item49 = find.text('item #49');
expect(item49, findsOneWidget);

// The initially selected menu item should be positioned on screen.
final RenderBox initialItem = tester.renderObject<RenderBox>(item49);
final Rect initialItemBounds = initialItem.localToGlobal(Offset.zero) & initialItem.size;
final Size windowSize = tester.view.physicalSize / tester.view.devicePixelRatio;
expect(initialItemBounds.bottomRight.dy, lessThanOrEqualTo(windowSize.height));

// Select item 20.
final Finder item20 = find.text('item #20');
await tester.scrollUntilVisible(item20, 500);
expect(item20, findsOneWidget);
await tester.tap(item20);
await tester.pump();

// Open menu again.
await tester.tap(find.text('click here'));
await tester.pump();
expect(item20, findsOneWidget);

// The selected menu item should be positioned on screen.
final RenderBox selectedItem = tester.renderObject<RenderBox>(item20);
final Rect selectedItemBounds = selectedItem.localToGlobal(Offset.zero) & selectedItem.size;
expect(selectedItemBounds.bottomRight.dy, lessThanOrEqualTo(windowSize.height));
});

testWidgets('PopupMenuButton properly positions a constrained-size popup', (WidgetTester tester) async {
final Size windowSize = tester.view.physicalSize / tester.view.devicePixelRatio;
const int length = 50;
Expand Down

0 comments on commit 9bc8393

Please sign in to comment.