mirror of
https://github.com/flutter/flutter.git
synced 2026-02-20 02:29:02 +08:00
Fix Selected DropdownMenuItem isn't highlighted on mobile (#167874)
Fix https://github.com/flutter/flutter/issues/70294 ### Description The bug is about the previously selected DropdownMenuItem isn't highlighted on mobile while it works on Web and desktop. The reason is that `FocusManager.instance.highlightMode` has fallen to `FocusHighlightMode.touch` on mobile by default (or touch device without mouse in general):6f220d3ec0/packages/flutter/lib/src/material/dropdown.dart (L172-L175)7d56be4c8d/packages/flutter/lib/src/widgets/focus_manager.dart (L2335-L2353)7d56be4c8d/packages/flutter/lib/src/widgets/focus_manager.dart (L1741-L1748)We actually can work around this by setting `FocusManager.instance.highlightStrategy` to `FocusHighlightStrategy.alwaysTraditional`, see https://github.com/flutter/flutter/pull/70442#issuecomment-727036571. Nevertheless, this approach will may affect the entire scope of the app, potentially resulting in unforeseen behavior for other widgets. In this PR, I propose a straightforward solution that utilizes an ancestor widget (Ink) to manage the color state corresponding to `InkWell.overlayColor`. ### Demo the fix https://github.com/user-attachments/assets/b6476732-716c-4f39-ab82-7ab5accb7182 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md Signed-off-by: huycozy <huy@nevercode.io>
This commit is contained in:
Huy
GitHub
parent
05b71be8bc
commit
1ca93dec90
@ -23,6 +23,7 @@ import 'colors.dart';
|
||||
import 'constants.dart';
|
||||
import 'debug.dart';
|
||||
import 'icons.dart';
|
||||
import 'ink_decoration.dart';
|
||||
import 'ink_well.dart';
|
||||
import 'input_decorator.dart';
|
||||
import 'material.dart';
|
||||
@ -217,14 +218,22 @@ class _DropdownMenuItemButtonState<T> extends State<_DropdownMenuItemButton<T>>
|
||||
child = Padding(padding: padding, child: child);
|
||||
}
|
||||
child = SizedBox(height: widget.route.itemHeight, child: child);
|
||||
|
||||
final bool isSelected = widget.itemIndex == widget.route.selectedIndex;
|
||||
final FocusHighlightMode highlightMode = FocusManager.instance.highlightMode;
|
||||
// An [InkWell] is added to the item only if it is enabled
|
||||
if (dropdownMenuItem.enabled) {
|
||||
child = InkWell(
|
||||
autofocus: widget.itemIndex == widget.route.selectedIndex,
|
||||
autofocus: isSelected,
|
||||
enableFeedback: widget.enableFeedback,
|
||||
onTap: _handleOnTap,
|
||||
onFocusChange: _handleFocusChange,
|
||||
child: child,
|
||||
// When highlightMode is traditional, the InkWell draws the selected item background color.
|
||||
// When highlightMode is touch, insert an Ink to force the selected item background color.
|
||||
child:
|
||||
highlightMode == FocusHighlightMode.touch
|
||||
? Ink(color: isSelected ? Theme.of(context).focusColor : null, child: child)
|
||||
: child,
|
||||
);
|
||||
}
|
||||
child = FadeTransition(opacity: _opacityAnimation, child: child);
|
||||
|
||||
@ -4449,4 +4449,67 @@ void main() {
|
||||
// The dropdown should still be open, i.e., there should be one widget with 'second' text.
|
||||
expect(find.text('second'), findsOneWidget);
|
||||
});
|
||||
|
||||
// This is a regression test for https://github.com/flutter/flutter/issues/70294.
|
||||
testWidgets(
|
||||
'The previous selected item should be highlighted when reopening dropdown on mobile',
|
||||
(WidgetTester tester) async {
|
||||
final Color selectedColor = Colors.black.withValues(alpha: 0.12);
|
||||
String currentValue = 'one';
|
||||
await tester.pumpWidget(
|
||||
MaterialApp(
|
||||
theme: ThemeData(focusColor: selectedColor),
|
||||
home: Scaffold(
|
||||
body: Center(
|
||||
child: StatefulBuilder(
|
||||
builder: (BuildContext context, StateSetter setState) {
|
||||
return DropdownButton<String>(
|
||||
value: currentValue,
|
||||
items:
|
||||
menuItems
|
||||
.map(
|
||||
(String item) =>
|
||||
DropdownMenuItem<String>(value: item, child: Text(item)),
|
||||
)
|
||||
.toList(),
|
||||
onChanged: (String? newValue) {
|
||||
setState(() {
|
||||
currentValue = newValue!;
|
||||
});
|
||||
},
|
||||
);
|
||||
},
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
// Make sure the current value of dropdown is the first one of items list menuItems.
|
||||
expect(find.text('one'), findsOne);
|
||||
|
||||
// Tap to open the dropdown.
|
||||
await tester.tap(find.text('one'));
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
// Select the second item from the dropdown list.
|
||||
await tester.tap(find.text('two'));
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
// Make sure the current item of dropdown is the second item of items list menuItems.
|
||||
expect(find.text('two'), findsOneWidget);
|
||||
|
||||
// Tap to reopen the dropdown.
|
||||
await tester.tap(find.text('two'));
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
// Make sure the current selected item is highlighted with selectedColor.
|
||||
final Ink selectedItemInk = tester.widget<Ink>(
|
||||
find.ancestor(of: find.text('two'), matching: find.byType(Ink)).first,
|
||||
);
|
||||
final BoxDecoration decoration = selectedItemInk.decoration! as BoxDecoration;
|
||||
expect(decoration.color, selectedColor);
|
||||
},
|
||||
variant: TargetPlatformVariant.mobile(),
|
||||
);
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user