-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Add DropdownMenu.decorationBuilder #176264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,16 @@ typedef FilterCallback<T> = | |
| /// Used by [DropdownMenu.searchCallback]. | ||
| typedef SearchCallback<T> = int? Function(List<DropdownMenuEntry<T>> entries, String query); | ||
|
|
||
| /// The type of builder function used by [DropdownMenu.decorationBuilder] to | ||
| /// build the [InputDecoration] passed to the inner text field. | ||
| /// | ||
| /// The `context` is the context that the decoration is being built in. | ||
| /// | ||
| /// The `controller` is the [MenuController] that can be used to open and close | ||
| /// the menu with and query the current state. | ||
| typedef DropdownMenuDecorationBuilder = | ||
| InputDecoration Function(BuildContext context, MenuController controller); | ||
|
|
||
| const double _kMinimumWidth = 112.0; | ||
|
|
||
| const double _kDefaultHorizontalPadding = 12.0; | ||
|
|
@@ -182,6 +192,7 @@ class DropdownMenu<T extends Object> extends StatefulWidget { | |
| this.textAlign = TextAlign.start, | ||
| // TODO(bleroux): Clean this up once `InputDecorationTheme` is fully normalized. | ||
| Object? inputDecorationTheme, | ||
| this.decorationBuilder, | ||
| this.menuStyle, | ||
| this.controller, | ||
| this.initialSelection, | ||
|
|
@@ -207,6 +218,10 @@ class DropdownMenu<T extends Object> extends StatefulWidget { | |
| inputDecorationTheme is InputDecorationThemeData), | ||
| ), | ||
| assert(trailingIconFocusNode == null || showTrailingIcon), | ||
| assert( | ||
| decorationBuilder == null || | ||
| (label == null && hintText == null && helperText == null && errorText == null), | ||
| ), | ||
| _inputDecorationTheme = inputDecorationTheme; | ||
|
|
||
| /// Determine if the [DropdownMenu] is enabled. | ||
|
|
@@ -369,6 +384,23 @@ class DropdownMenu<T extends Object> extends StatefulWidget { | |
|
|
||
| final Object? _inputDecorationTheme; | ||
|
|
||
| /// The builder function used to create the [InputDecoration] passed to the text field. | ||
| /// | ||
| /// If a value is provided for this property and the resulting [InputDecoration.suffixIcon] | ||
| /// is null, a default [IconButton] is assigned as the suffix icon. This button's icon will | ||
| /// use [trailingIcon] and [selectedTrailingIcon] if those are explicitly defined; otherwise, | ||
| /// it defaults to [Icons.arrow_drop_down] for the collapsed state and [Icons.arrow_drop_up] | ||
| /// for the expanded state. | ||
| /// | ||
| /// If null, the default builder creates a decoration where: | ||
| /// - [InputDecoration.label] is set to [label]. | ||
| /// - [InputDecoration.hintText] is set to [hintText]. | ||
| /// - [InputDecoration.helperText] is set to [helperText]. | ||
| /// - [InputDecoration.errorText] is set to [errorText]. | ||
| /// - [InputDecoration.prefixIcon] is set to [leadingIcon]. | ||
| /// - [InputDecoration.suffixIcon] is set to an [IconButton] which uses [trailingIcon] and [selectedTrailingIcon] if defined, or [Icons.arrow_drop_down] and [Icons.arrow_drop_up] otherwise. | ||
| final DropdownMenuDecorationBuilder? decorationBuilder; | ||
|
|
||
| /// The [MenuStyle] that defines the visual attributes of the menu. | ||
| /// | ||
| /// The default width of the menu is set to the width of the text field. | ||
|
|
@@ -1132,38 +1164,26 @@ class _DropdownMenuState<T extends Object> extends State<DropdownMenu<T>> { | |
| crossAxisUnconstrained: false, | ||
| builder: (BuildContext context, MenuController controller, Widget? child) { | ||
| assert(_initialMenu != null); | ||
| final bool isCollapsed = widget.inputDecorationTheme?.isCollapsed ?? false; | ||
| final Widget trailingButton = widget.showTrailingIcon | ||
| ? Padding( | ||
| padding: isCollapsed ? EdgeInsets.zero : const EdgeInsets.all(4.0), | ||
| child: ExcludeSemantics( | ||
| // When the text field is treated as a button (i.e., it can | ||
| // not be focused), the trailing button should become part of | ||
| // the text field button by excluding semantics. Otherwise, | ||
| // it will inappropriately announce whether this icon button | ||
| // is selected or not. | ||
| excluding: !canRequestFocus(), | ||
| child: IconButton( | ||
| focusNode: _trailingIconButtonFocusNode, | ||
| isSelected: controller.isOpen, | ||
| constraints: widget.inputDecorationTheme?.suffixIconConstraints, | ||
| padding: isCollapsed ? EdgeInsets.zero : null, | ||
| icon: widget.trailingIcon ?? const Icon(Icons.arrow_drop_down), | ||
| selectedIcon: widget.selectedTrailingIcon ?? const Icon(Icons.arrow_drop_up), | ||
| onPressed: !widget.enabled | ||
| ? null | ||
| : () { | ||
| handlePressed(controller); | ||
| }, | ||
| ), | ||
| ), | ||
| ) | ||
| : const SizedBox.shrink(); | ||
|
|
||
| final Widget leadingButton = Padding( | ||
| padding: const EdgeInsets.all(8.0), | ||
| child: widget.leadingIcon ?? const SizedBox.shrink(), | ||
| final DropdownMenuDecorationBuilder decorationBuilder = | ||
| widget.decorationBuilder ?? _buildDefaultDecoration; | ||
| InputDecoration decoration = decorationBuilder(context, controller); | ||
| // If no suffixIcon is provided, the default IconButton is used for convenience. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think if there will be a case where the user doesn't want the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting. Because
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd propose to always add the default icon, also with a custom decoration (when the user adds an additinal trailing icon, then keep it in a The other way around: in the current implementation, if the user defines its own trailing icon, the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Adding a Row fits specific use cases and for these use cases it might be better for us to expose the default button construction if you think this would be interesting and let users add a Row (or something else) if needed (that way they can control padding for instance or anything matching their design).
Yes. It surely should be documented (in fact the best move would probably to rename
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Good point. I personally don't need the default trailing button, as the fuctionality can easily be recreated with the controller. Not sure if folks would notice / search for the constructor of the default icon.
Sounds reasonable. Happy with both variants :) Btw thank you for your efforts!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I see, it makes sense to me. |
||
| if (decoration.suffixIcon == null) { | ||
| decoration = decoration.copyWith( | ||
| suffixIcon: _buildDefaultSuffixIcon(context, controller), | ||
| ); | ||
| } | ||
| final InputDecoration effectiveDecoration = decoration.applyDefaults( | ||
| effectiveInputDecorationTheme, | ||
| ); | ||
| final InputDecoration textFieldDecoration = effectiveDecoration.prefixIcon == null | ||
| ? effectiveDecoration | ||
| : effectiveDecoration.copyWith( | ||
| prefixIcon: SizedBox( | ||
| key: _leadingKey, // Used to query the width in refreshLeadingPadding. | ||
| child: effectiveDecoration.prefixIcon, | ||
| ), | ||
| ); | ||
|
|
||
| final MaterialLocalizations localizations = MaterialLocalizations.of(context); | ||
| final bool isButton = !canRequestFocus(); | ||
|
|
@@ -1223,16 +1243,7 @@ class _DropdownMenuState<T extends Object> extends State<DropdownMenu<T>> { | |
| }); | ||
| }, | ||
| inputFormatters: widget.inputFormatters, | ||
| decoration: InputDecoration( | ||
| label: widget.label, | ||
| hintText: widget.hintText, | ||
| helperText: widget.helperText, | ||
| errorText: widget.errorText, | ||
| prefixIcon: widget.leadingIcon != null | ||
| ? SizedBox(key: _leadingKey, child: widget.leadingIcon) | ||
| : null, | ||
| suffixIcon: widget.showTrailingIcon ? trailingButton : null, | ||
| ).applyDefaults(effectiveInputDecorationTheme), | ||
| decoration: textFieldDecoration, | ||
| restorationId: widget.restorationId, | ||
| ), | ||
| ), | ||
|
|
@@ -1250,6 +1261,11 @@ class _DropdownMenuState<T extends Object> extends State<DropdownMenu<T>> { | |
| // and leadingButton. | ||
| // | ||
| // See _RenderDropdownMenuBody layout logic. | ||
| // | ||
| // TODO(bleroux): find a more accurate way to measure the text field minimum width. | ||
| // The text field width computation is not accurate as it is based only on label, | ||
| // prefixIcon and suffixIcon. Other InputDecoration parameters can have an | ||
| // impact on the total width. | ||
| children: <Widget>[ | ||
| textField, | ||
| ..._initialMenu!.map( | ||
|
|
@@ -1263,8 +1279,14 @@ class _DropdownMenuState<T extends Object> extends State<DropdownMenu<T>> { | |
| child: DefaultTextStyle(style: effectiveTextStyle!, child: widget.label!), | ||
| ), | ||
| ), | ||
| trailingButton, | ||
| leadingButton, | ||
| effectiveDecoration.suffixIcon ?? const SizedBox.shrink(), | ||
| Padding( | ||
| // TODO(bleroux): find a more accurate way to get the correct width. | ||
| // This padding is used to mimic default input decorator padding. | ||
| // It won't be correct if non default values are used. | ||
| padding: const EdgeInsets.all(8.0), | ||
| child: effectiveDecoration.prefixIcon ?? const SizedBox.shrink(), | ||
| ), | ||
| ], | ||
| ); | ||
|
|
||
|
|
@@ -1337,6 +1359,47 @@ class _DropdownMenuState<T extends Object> extends State<DropdownMenu<T>> { | |
| ), | ||
| ); | ||
| } | ||
|
|
||
| InputDecoration _buildDefaultDecoration(BuildContext context, MenuController controller) { | ||
| return InputDecoration( | ||
| label: widget.label, | ||
| hintText: widget.hintText, | ||
| helperText: widget.helperText, | ||
| errorText: widget.errorText, | ||
| prefixIcon: widget.leadingIcon, | ||
| suffixIcon: _buildDefaultSuffixIcon(context, controller), | ||
| ); | ||
| } | ||
|
|
||
| Widget? _buildDefaultSuffixIcon(BuildContext context, MenuController controller) { | ||
| final bool isCollapsed = widget.inputDecorationTheme?.isCollapsed ?? false; | ||
| return widget.showTrailingIcon | ||
| ? Padding( | ||
| padding: isCollapsed ? EdgeInsets.zero : const EdgeInsets.all(4.0), | ||
| child: ExcludeSemantics( | ||
| // When the text field is treated as a button (i.e., it can | ||
| // not be focused), the trailing button should become part of | ||
| // the text field button by excluding semantics. Otherwise, | ||
| // it will inappropriately announce whether this icon button | ||
| // is selected or not. | ||
| excluding: !canRequestFocus(), | ||
| child: IconButton( | ||
| focusNode: _trailingIconButtonFocusNode, | ||
| isSelected: controller.isOpen, | ||
| constraints: widget.inputDecorationTheme?.suffixIconConstraints, | ||
| padding: isCollapsed ? EdgeInsets.zero : null, | ||
| icon: widget.trailingIcon ?? const Icon(Icons.arrow_drop_down), | ||
| selectedIcon: widget.selectedTrailingIcon ?? const Icon(Icons.arrow_drop_up), | ||
| onPressed: !widget.enabled | ||
| ? null | ||
| : () { | ||
| handlePressed(controller); | ||
| }, | ||
| ), | ||
| ), | ||
| ) | ||
| : null; | ||
| } | ||
| } | ||
|
|
||
| // `DropdownMenu` dispatches these private intents on arrow up/down keys. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bleroux I noticed a little naming improvement: Maybe it makes sense to call the parameter
menuController, as in the widget itselfcontrolleris always referenced as theTextEditingController. Maybe it makes sense to rename the default param name to reduce confusion (?)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Gustl22 I usually try to use more precise names. In the context of DropdownMenu, there are multiple controllers so it could help, especially for instance variables.
About this particular case, because the name is just in a typedef and the explicit type is just before, I probably preferred
controlleras the type of controller was explicit.Is this variable visible anywhere else? Maybe when using completion?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this is only a problem of code completion for now.
I just stumbled across this scenario, where I wanted to add a clear button:
I needed to use a StatefulWidget, because the
TextEditingControllerwas not provided by thedecorationBuilder.For me it would make sense to have both controllers available in the callback, or none (so I should pass both the
MenuControllerand theTextEditingControllerto theDropdownMenu).Its not per se bad, how it is right now. Just wanted to hear your opinion, what makes
MenuControllermore valuable to exist in the callback than theTextEditingController, which might be the option to mimic the current behavior of opening and closing.