From 21381d843f3feb32640ae385aedd8e5c4003696b Mon Sep 17 00:00:00 2001 From: Bruno Leroux Date: Wed, 2 Oct 2024 14:03:20 +0200 Subject: [PATCH] Fix DropdownMenu does not rematch initialSelection when entries have changed (#155757) ## Description This PR makes DropdownMenu rematching the initialSelection when the entries are updated. If the new entries contains one entry whose value matches `initialSelection` this entry's label is used to initialize the inner text field, if no entries matches `initialSelection` the text field is emptied. ## Related Issue Fixes [DropdownMenu.didUpdateWidget should re-match initialSelection when dropdownMenuEntries have changed](https://github.com/flutter/flutter/issues/155660). ## Tests Adds 3 tests. --- .../dropdown_menu/dropdown_menu.0.dart | 56 ++++---- .../dropdown_menu/dropdown_menu.1.dart | 13 +- .../dropdown_menu/dropdown_menu.2.dart | 26 ++-- .../lib/src/material/dropdown_menu.dart | 35 +++-- .../test/material/dropdown_menu_test.dart | 129 ++++++++++++++++++ 5 files changed, 198 insertions(+), 61 deletions(-) diff --git a/examples/api/lib/material/dropdown_menu/dropdown_menu.0.dart b/examples/api/lib/material/dropdown_menu/dropdown_menu.0.dart index e7b1ea9b3d..2a5e0b9071 100644 --- a/examples/api/lib/material/dropdown_menu/dropdown_menu.0.dart +++ b/examples/api/lib/material/dropdown_menu/dropdown_menu.0.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; // Flutter code sample for [DropdownMenu]s. The first dropdown menu @@ -14,6 +15,8 @@ void main() { runApp(const DropdownMenuExample()); } +typedef ColorEntry = DropdownMenuEntry; + // DropdownMenuEntry labels and values for the first dropdown menu. enum ColorLabel { blue('Blue', Colors.blue), @@ -25,21 +28,43 @@ enum ColorLabel { const ColorLabel(this.label, this.color); final String label; final Color color; + + static final List entries = UnmodifiableListView( + values.map( + (ColorLabel color) => ColorEntry( + value: color, + label: color.label, + enabled: color.label != 'Grey', + style: MenuItemButton.styleFrom( + foregroundColor: color.color, + ), + ), + ), + ); } +typedef IconEntry = DropdownMenuEntry; + // DropdownMenuEntry labels and values for the second dropdown menu. enum IconLabel { smile('Smile', Icons.sentiment_satisfied_outlined), - cloud( - 'Cloud', - Icons.cloud_outlined, - ), + cloud('Cloud', Icons.cloud_outlined), brush('Brush', Icons.brush_outlined), heart('Heart', Icons.favorite); const IconLabel(this.label, this.icon); final String label; final IconData icon; + + static final List entries = UnmodifiableListView( + values.map( + (IconLabel icon) => IconEntry( + value: icon, + label: icon.label, + leadingIcon: Icon(icon.icon), + ), + ), + ); } class DropdownMenuExample extends StatefulWidget { @@ -85,18 +110,7 @@ class _DropdownMenuExampleState extends State { selectedColor = color; }); }, - dropdownMenuEntries: ColorLabel.values.map>( - (ColorLabel color) { - return DropdownMenuEntry( - value: color, - label: color.label, - enabled: color.label != 'Grey', - style: MenuItemButton.styleFrom( - foregroundColor: color.color, - ), - ); - } - ).toList(), + dropdownMenuEntries: ColorLabel.entries, ), const SizedBox(width: 24), DropdownMenu( @@ -114,15 +128,7 @@ class _DropdownMenuExampleState extends State { selectedIcon = icon; }); }, - dropdownMenuEntries: IconLabel.values.map>( - (IconLabel icon) { - return DropdownMenuEntry( - value: icon, - label: icon.label, - leadingIcon: Icon(icon.icon), - ); - }, - ).toList(), + dropdownMenuEntries: IconLabel.entries, ), ], ), diff --git a/examples/api/lib/material/dropdown_menu/dropdown_menu.1.dart b/examples/api/lib/material/dropdown_menu/dropdown_menu.1.dart index dbb22177f3..6de6d9a7a1 100644 --- a/examples/api/lib/material/dropdown_menu/dropdown_menu.1.dart +++ b/examples/api/lib/material/dropdown_menu/dropdown_menu.1.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; /// Flutter code sample for [DropdownMenu]. @@ -34,7 +35,12 @@ class DropdownMenuExample extends StatefulWidget { State createState() => _DropdownMenuExampleState(); } +typedef MenuEntry = DropdownMenuEntry; + class _DropdownMenuExampleState extends State { + static final List menuEntries = UnmodifiableListView( + list.map((String name) => MenuEntry(value: name, label: name)), + ); String dropdownValue = list.first; @override @@ -47,12 +53,7 @@ class _DropdownMenuExampleState extends State { dropdownValue = value!; }); }, - dropdownMenuEntries: list.map>((String value) { - return DropdownMenuEntry( - value: value, - label: value - ); - }).toList(), + dropdownMenuEntries: menuEntries, ); } } diff --git a/examples/api/lib/material/dropdown_menu/dropdown_menu.2.dart b/examples/api/lib/material/dropdown_menu/dropdown_menu.2.dart index 4fbe137cb5..4b07bbaf37 100644 --- a/examples/api/lib/material/dropdown_menu/dropdown_menu.2.dart +++ b/examples/api/lib/material/dropdown_menu/dropdown_menu.2.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; /// Flutter code sample for [DropdownMenu]. @@ -33,7 +34,12 @@ class DropdownMenuExample extends StatefulWidget { State createState() => _DropdownMenuExampleState(); } +typedef MenuEntry = DropdownMenuEntry; + class _DropdownMenuExampleState extends State { + static final List menuEntries = UnmodifiableListView( + list.map((String name) => MenuEntry(value: name, label: name)), + ); String dropdownValue = list.first; @override @@ -62,10 +68,7 @@ class _DropdownMenuExampleState extends State { dropdownValue = value!; }); }, - dropdownMenuEntries: - list.map>((String value) { - return DropdownMenuEntry(value: value, label: value); - }).toList(), + dropdownMenuEntries: menuEntries, ), const Text('Text cursor is shown when hovering over the DropdownMenu.'), ], @@ -92,10 +95,7 @@ class _DropdownMenuExampleState extends State { dropdownValue = value!; }); }, - dropdownMenuEntries: - list.map>((String value) { - return DropdownMenuEntry(value: value, label: value); - }).toList(), + dropdownMenuEntries: menuEntries, ), const Text('Clickable cursor is shown when hovering over the DropdownMenu.'), ], @@ -123,10 +123,7 @@ class _DropdownMenuExampleState extends State { dropdownValue = value!; }); }, - dropdownMenuEntries: - list.map>((String value) { - return DropdownMenuEntry(value: value, label: value); - }).toList(), + dropdownMenuEntries: menuEntries, ), const Text('Default cursor is shown when hovering over the DropdownMenu.'), ], @@ -154,10 +151,7 @@ class _DropdownMenuExampleState extends State { dropdownValue = value!; }); }, - dropdownMenuEntries: - list.map>((String value) { - return DropdownMenuEntry(value: value, label: value); - }).toList(), + dropdownMenuEntries: menuEntries, ), const Text('Default cursor is shown when hovering over the DropdownMenu.'), ], diff --git a/packages/flutter/lib/src/material/dropdown_menu.dart b/packages/flutter/lib/src/material/dropdown_menu.dart index 7050a4e161..394a8b61e8 100644 --- a/packages/flutter/lib/src/material/dropdown_menu.dart +++ b/packages/flutter/lib/src/material/dropdown_menu.dart @@ -497,6 +497,18 @@ class _DropdownMenuState extends State> { bool _menuHasEnabledItem = false; TextEditingController? _localTextEditingController; + TextEditingValue get _initialTextEditingValue { + for (final DropdownMenuEntry entry in filteredEntries) { + if (entry.value == widget.initialSelection) { + return TextEditingValue( + text: entry.label, + selection: TextSelection.collapsed(offset: entry.label.length), + ); + } + } + return TextEditingValue.empty; + } + @override void initState() { super.initState(); @@ -509,14 +521,8 @@ class _DropdownMenuState extends State> { filteredEntries = widget.dropdownMenuEntries; buttonItemKeys = List.generate(filteredEntries.length, (int index) => GlobalKey()); _menuHasEnabledItem = filteredEntries.any((DropdownMenuEntry entry) => entry.enabled); + _localTextEditingController?.value = _initialTextEditingValue; - final int index = filteredEntries.indexWhere((DropdownMenuEntry entry) => entry.value == widget.initialSelection); - if (index != -1) { - _localTextEditingController?.value = TextEditingValue( - text: filteredEntries[index].label, - selection: TextSelection.collapsed(offset: filteredEntries[index].label.length), - ); - } refreshLeadingPadding(); } @@ -554,18 +560,19 @@ class _DropdownMenuState extends State> { filteredEntries = widget.dropdownMenuEntries; buttonItemKeys = List.generate(filteredEntries.length, (int index) => GlobalKey()); _menuHasEnabledItem = filteredEntries.any((DropdownMenuEntry entry) => entry.enabled); + // If the text field content matches one of the new entries do not rematch the initialSelection. + final bool isCurrentSelectionValid = filteredEntries.any( + (DropdownMenuEntry entry) => entry.label == _localTextEditingController?.text + ); + if (!isCurrentSelectionValid) { + _localTextEditingController?.value = _initialTextEditingValue; + } } if (oldWidget.leadingIcon != widget.leadingIcon) { refreshLeadingPadding(); } if (oldWidget.initialSelection != widget.initialSelection) { - final int index = filteredEntries.indexWhere((DropdownMenuEntry entry) => entry.value == widget.initialSelection); - if (index != -1) { - _localTextEditingController?.value = TextEditingValue( - text: filteredEntries[index].label, - selection: TextSelection.collapsed(offset: filteredEntries[index].label.length), - ); - } + _localTextEditingController?.value = _initialTextEditingValue; } } diff --git a/packages/flutter/test/material/dropdown_menu_test.dart b/packages/flutter/test/material/dropdown_menu_test.dart index f394799360..bbe12ea6cf 100644 --- a/packages/flutter/test/material/dropdown_menu_test.dart +++ b/packages/flutter/test/material/dropdown_menu_test.dart @@ -1996,6 +1996,135 @@ void main() { expect(itemMaterial.color, themeData.colorScheme.onSurface.withOpacity(0.12)); }); + testWidgets( + 'When the initial selection matches a menu entry, the text field displays the corresponding value', + (WidgetTester tester) async { + final TextEditingController controller = TextEditingController(); + addTearDown(controller.dispose); + + await tester.pumpWidget(MaterialApp( + home: StatefulBuilder( + builder: (BuildContext context, StateSetter setState) { + return Scaffold( + body: DropdownMenu( + initialSelection: TestMenu.mainMenu3, + dropdownMenuEntries: menuChildren, + controller: controller, + ), + ); + } + ), + )); + + expect(controller.text, TestMenu.mainMenu3.label); + }, + ); + + testWidgets( + 'Text field is empty when the initial selection does not match any menu entries', + (WidgetTester tester) async { + final TextEditingController controller = TextEditingController(); + addTearDown(controller.dispose); + + await tester.pumpWidget(MaterialApp( + home: StatefulBuilder( + builder: (BuildContext context, StateSetter setState) { + return Scaffold( + body: DropdownMenu( + initialSelection: TestMenu.mainMenu3, + // Use a menu entries which does not contain TestMenu.mainMenu3. + dropdownMenuEntries: menuChildren.getRange(0, 1).toList(), + controller: controller, + ), + ); + } + ), + )); + + expect(controller.text, isEmpty); + }, + ); + + // Regression test for https://github.com/flutter/flutter/issues/155660. + testWidgets('Updating the menu entries refreshes the initial selection', (WidgetTester tester) async { + final TextEditingController controller = TextEditingController(); + addTearDown(controller.dispose); + + Widget boilerplate(List> entries) { + return MaterialApp( + home: StatefulBuilder( + builder: (BuildContext context, StateSetter setState) { + return Scaffold( + body: DropdownMenu( + initialSelection: TestMenu.mainMenu3, + dropdownMenuEntries: entries, + controller: controller, + ), + ); + } + ), + ); + } + + // The text field should be empty when the initial selection does not match + // any menu items. + await tester.pumpWidget(boilerplate(menuChildren.getRange(0, 1).toList())); + expect(controller.text, ''); + + // When the menu entries is updated the initial selection should be rematched. + await tester.pumpWidget(boilerplate(menuChildren)); + expect(controller.text, TestMenu.mainMenu3.label); + + // Update the entries with none matching the initial selection. + await tester.pumpWidget(boilerplate(menuChildren.getRange(0, 1).toList())); + expect(controller.text, ''); + }); + + // Regression test for https://github.com/flutter/flutter/issues/155660. + testWidgets( + 'Updating the menu entries refreshes the initial selection only if the current selection is no more valid', + (WidgetTester tester) async { + final TextEditingController controller = TextEditingController(); + addTearDown(controller.dispose); + + Widget boilerplate(List> entries) { + return MaterialApp( + home: StatefulBuilder( + builder: (BuildContext context, StateSetter setState) { + return Scaffold( + body: DropdownMenu( + initialSelection: TestMenu.mainMenu3, + dropdownMenuEntries: entries, + controller: controller, + ), + ); + } + ), + ); + } + + await tester.pumpWidget(boilerplate(menuChildren)); + expect(controller.text, TestMenu.mainMenu3.label); + + // Open the menu. + await tester.tap(find.byType(DropdownMenu)); + await tester.pump(); + + // Select another item. + final Finder item2 = find.widgetWithText(MenuItemButton, 'Item 2').last; + await tester.tap(item2); + await tester.pumpAndSettle(); + expect(controller.text, TestMenu.mainMenu2.label); + + // Update the menu entries with another instance of list containing the + // same entries. + await tester.pumpWidget(boilerplate( + List>.from(menuChildren) + )); + expect(controller.text, TestMenu.mainMenu2.label); + }, + ); + testWidgets('The default text input field should not be focused on mobile platforms ' 'when it is tapped', (WidgetTester tester) async { final ThemeData themeData = ThemeData();