Skip to content

Commit

Permalink
Remove extra padding if a dropdown menu entry also has a leading icon…
Browse files Browse the repository at this point in the history
… (#135004)

Fixes #131350

This PR is to remove the extra padding in `DropdownMenuEntry` if both the text field and the dropdown menu entry have leading icons.

**After** fix:
<img width="300" alt="Screenshot 2023-09-19 at 4 35 24 PM" src="https://github.com/flutter/flutter/assets/36861262/ed7d92a5-3f96-4106-a03e-09258ea3709f">

**Before** fix:
<img width="300" alt="Screenshot 2023-09-19 at 4 37 58 PM" src="https://github.com/flutter/flutter/assets/36861262/fdbfef54-6c93-48fb-bd64-41fa31dde531">
  • Loading branch information
QuncCccccc authored Sep 22, 2023
1 parent f388559 commit 09f7d7c
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 13 deletions.
33 changes: 20 additions & 13 deletions packages/flutter/lib/src/material/dropdown_menu.dart
Original file line number Diff line number Diff line change
Expand Up @@ -432,21 +432,28 @@ class _DropdownMenuState<T> extends State<DropdownMenu<T>> {
{ int? focusedIndex, bool enableScrollToHighlight = true}
) {
final List<Widget> result = <Widget>[];
final double padding = leadingPadding ?? _kDefaultHorizontalPadding;
final ButtonStyle defaultStyle;
switch (textDirection) {
case TextDirection.rtl:
defaultStyle = MenuItemButton.styleFrom(
padding: EdgeInsets.only(left: _kDefaultHorizontalPadding, right: padding),
);
case TextDirection.ltr:
defaultStyle = MenuItemButton.styleFrom(
padding: EdgeInsets.only(left: padding, right: _kDefaultHorizontalPadding),
);
}

for (int i = 0; i < filteredEntries.length; i++) {
final DropdownMenuEntry<T> entry = filteredEntries[i];

// By default, when the text field has a leading icon but a menu entry doesn't
// have one, the label of the entry should have extra padding to be aligned
// with the text in the text input field. When both the text field and the
// menu entry have leading icons, the menu entry should remove the extra
// paddings so its leading icon will be aligned with the leading icon of
// the text field.
final double padding = entry.leadingIcon == null ? (leadingPadding ?? _kDefaultHorizontalPadding) : _kDefaultHorizontalPadding;
final ButtonStyle defaultStyle;
switch (textDirection) {
case TextDirection.rtl:
defaultStyle = MenuItemButton.styleFrom(
padding: EdgeInsets.only(left: _kDefaultHorizontalPadding, right: padding),
);
case TextDirection.ltr:
defaultStyle = MenuItemButton.styleFrom(
padding: EdgeInsets.only(left: padding, right: _kDefaultHorizontalPadding),
);
}

ButtonStyle effectiveStyle = entry.style ?? defaultStyle;
final Color focusedBackgroundColor = effectiveStyle.foregroundColor?.resolve(<MaterialState>{MaterialState.focused})
?? Theme.of(context).colorScheme.onSurface;
Expand Down
34 changes: 34 additions & 0 deletions packages/flutter/test/material/dropdown_menu_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1680,6 +1680,40 @@ void main() {
await tester.pumpAndSettle();
expect(controller.text, ''); // nothing selected
});

// Regression test for https://github.com/flutter/flutter/issues/131350.
testWidgets('DropdownMenuEntry.leadingIcon default layout', (WidgetTester tester) async {
// The DropdownMenu should not get extra padding in DropdownMenuEntry items
// when both text field and DropdownMenuEntry have leading icons.
await tester.pumpWidget(const MaterialApp(
home: Scaffold(
body: DropdownMenu<int>(
leadingIcon: Icon(Icons.search),
hintText: 'Hint',
dropdownMenuEntries: <DropdownMenuEntry<int>>[
DropdownMenuEntry<int>(
value: 0,
label: 'Item 0',
leadingIcon: Icon(Icons.alarm)
),
DropdownMenuEntry<int>(value: 1, label: 'Item 1'),
],
),
)
));
await tester.tap(find.byType(DropdownMenu<int>));
await tester.pumpAndSettle();

// Check text location in text field.
expect(tester.getTopLeft(find.text('Hint')).dx, 48.0);

// By default, the text of item 0 should be aligned with the text of the text field.
expect(tester.getTopLeft(find.text('Item 0').last).dx, 48.0);

// By default, the text of item 1 should be aligned with the text of the text field,
// so there are some extra padding before "Item 1".
expect(tester.getTopLeft(find.text('Item 1').last).dx, 48.0);
});
}

enum TestMenu {
Expand Down

0 comments on commit 09f7d7c

Please sign in to comment.