From cc2c902bcbce598fa84321186bd263df87905115 Mon Sep 17 00:00:00 2001 From: Taha Tesser Date: Fri, 28 Jan 2022 17:20:24 +0200 Subject: [PATCH] Fix tappable area for `DropdownButtonFormField` & add `InkWell` to `DropdownButton` (#95906) --- .../flutter/lib/src/material/dropdown.dart | 176 +++++++++++------- .../material/dropdown_form_field_test.dart | 26 +-- .../flutter/test/material/dropdown_test.dart | 22 +-- 3 files changed, 133 insertions(+), 91 deletions(-) diff --git a/packages/flutter/lib/src/material/dropdown.dart b/packages/flutter/lib/src/material/dropdown.dart index 76e13ede7a..120a28bc29 100644 --- a/packages/flutter/lib/src/material/dropdown.dart +++ b/packages/flutter/lib/src/material/dropdown.dart @@ -788,6 +788,9 @@ class DropdownButtonHideUnderline extends InheritedWidget { /// shows the currently selected item as well as an arrow that opens a menu for /// selecting another item. /// +/// One ancestor must be a [Material] widget and typically this is +/// provided by the app's [Scaffold]. +/// /// The type `T` is the type of the [value] that each dropdown item represents. /// All the entries in a given menu must represent values with consistent types. /// Typically, an enum is used. Each [DropdownMenuItem] in [items] must be @@ -892,6 +895,61 @@ class DropdownButton extends StatefulWidget { assert(isExpanded != null), assert(autofocus != null), assert(itemHeight == null || itemHeight >= kMinInteractiveDimension), + _inputDecoration = null, + _isEmpty = false, + _isFocused = false, + super(key: key); + + DropdownButton._formField({ + Key? key, + required this.items, + this.selectedItemBuilder, + this.value, + this.hint, + this.disabledHint, + required this.onChanged, + this.onTap, + this.elevation = 8, + this.style, + this.underline, + this.icon, + this.iconDisabledColor, + this.iconEnabledColor, + this.iconSize = 24.0, + this.isDense = false, + this.isExpanded = false, + this.itemHeight = kMinInteractiveDimension, + this.focusColor, + this.focusNode, + this.autofocus = false, + this.dropdownColor, + this.menuMaxHeight, + this.enableFeedback, + this.alignment = AlignmentDirectional.centerStart, + this.borderRadius, + required InputDecoration inputDecoration, + required bool isEmpty, + required bool isFocused, + }) : assert(items == null || items.isEmpty || value == null || + items.where((DropdownMenuItem item) { + return item.value == value; + }).length == 1, + "There should be exactly one item with [DropdownButtonFormField]'s value: " + '$value. \n' + 'Either zero or 2 or more [DropdownMenuItem]s were detected ' + 'with the same value', + ), + assert(elevation != null), + assert(iconSize != null), + assert(isDense != null), + assert(isExpanded != null), + assert(autofocus != null), + assert(itemHeight == null || itemHeight >= kMinInteractiveDimension), + assert(isEmpty != null), + assert(isFocused != null), + _inputDecoration = inputDecoration, + _isEmpty = isEmpty, + _isFocused = isFocused, super(key: key); /// The list of items the user can select. @@ -1101,6 +1159,12 @@ class DropdownButton extends StatefulWidget { /// Defines the corner radii of the menu's rounded rectangle shape. final BorderRadius? borderRadius; + final InputDecoration? _inputDecoration; + + final bool _isEmpty; + + final bool _isFocused; + @override State> createState() => _DropdownButtonState(); } @@ -1113,7 +1177,6 @@ class _DropdownButtonState extends State> with WidgetsBindi FocusNode? get focusNode => widget.focusNode ?? _internalNode; bool _hasPrimaryFocus = false; late Map> _actionMap; - late FocusHighlightMode _focusHighlightMode; // Only used if needed to create _internalNode. FocusNode _createFocusNode() { @@ -1136,16 +1199,12 @@ class _DropdownButtonState extends State> with WidgetsBindi ), }; focusNode!.addListener(_handleFocusChanged); - final FocusManager focusManager = WidgetsBinding.instance!.focusManager; - _focusHighlightMode = focusManager.highlightMode; - focusManager.addHighlightModeListener(_handleFocusHighlightModeChange); } @override void dispose() { WidgetsBinding.instance!.removeObserver(this); _removeDropdownRoute(); - WidgetsBinding.instance!.focusManager.removeHighlightModeListener(_handleFocusHighlightModeChange); focusNode!.removeListener(_handleFocusChanged); _internalNode?.dispose(); super.dispose(); @@ -1165,14 +1224,6 @@ class _DropdownButtonState extends State> with WidgetsBindi } } - void _handleFocusHighlightModeChange(FocusHighlightMode mode) { - if (!mounted) { - return; - } - setState(() { - _focusHighlightMode = mode; - }); - } @override void didUpdateWidget(DropdownButton oldWidget) { @@ -1315,15 +1366,6 @@ class _DropdownButtonState extends State> with WidgetsBindi return result; } - bool get _showHighlight { - switch (_focusHighlightMode) { - case FocusHighlightMode.touch: - return false; - case FocusHighlightMode.traditional: - return _hasPrimaryFocus; - } - } - @override Widget build(BuildContext context) { assert(debugCheckHasMaterial(context)); @@ -1386,12 +1428,6 @@ class _DropdownButtonState extends State> with WidgetsBindi Widget result = DefaultTextStyle( style: _enabled ? _textStyle! : _textStyle!.copyWith(color: Theme.of(context).disabledColor), child: Container( - decoration: _showHighlight - ? BoxDecoration( - color: widget.focusColor ?? Theme.of(context).focusColor, - borderRadius: const BorderRadius.all(Radius.circular(4.0)), - ) - : null, padding: padding.resolve(Directionality.of(context)), height: widget.isDense ? _denseButtonHeight : null, child: Row( @@ -1446,22 +1482,28 @@ class _DropdownButtonState extends State> with WidgetsBindi }, ); + if (widget._inputDecoration != null) { + result = InputDecorator( + decoration: widget._inputDecoration!, + isEmpty: widget._isEmpty, + isFocused: widget._isFocused, + child: result, + ); + } + return Semantics( button: true, child: Actions( actions: _actionMap, - child: Focus( + child: InkWell( + mouseCursor: effectiveMouseCursor, + onTap: _enabled ? _handleTap : null, canRequestFocus: _enabled, focusNode: focusNode, autofocus: widget.autofocus, - child: MouseRegion( - cursor: effectiveMouseCursor, - child: GestureDetector( - onTap: _enabled ? _handleTap : null, - behavior: HitTestBehavior.opaque, - child: result, - ), - ), + focusColor: widget.focusColor ?? Theme.of(context).focusColor, + enableFeedback: false, + child: result, ), ), ); @@ -1570,37 +1612,35 @@ class DropdownButtonFormField extends FormField { canRequestFocus: false, skipTraversal: true, child: Builder(builder: (BuildContext context) { - return InputDecorator( - decoration: effectiveDecoration.copyWith(errorText: field.errorText), - isEmpty: isEmpty, - isFocused: Focus.of(context).hasFocus, - child: DropdownButtonHideUnderline( - child: DropdownButton( - items: items, - selectedItemBuilder: selectedItemBuilder, - value: state.value, - hint: hint, - disabledHint: disabledHint, - onChanged: onChanged == null ? null : state.didChange, - onTap: onTap, - elevation: elevation, - style: style, - icon: icon, - iconDisabledColor: iconDisabledColor, - iconEnabledColor: iconEnabledColor, - iconSize: iconSize, - isDense: isDense, - isExpanded: isExpanded, - itemHeight: itemHeight, - focusColor: focusColor, - focusNode: focusNode, - autofocus: autofocus, - dropdownColor: dropdownColor, - menuMaxHeight: menuMaxHeight, - enableFeedback: enableFeedback, - alignment: alignment, - borderRadius: borderRadius, - ), + return DropdownButtonHideUnderline( + child: DropdownButton._formField( + items: items, + selectedItemBuilder: selectedItemBuilder, + value: state.value, + hint: hint, + disabledHint: disabledHint, + onChanged: onChanged == null ? null : state.didChange, + onTap: onTap, + elevation: elevation, + style: style, + icon: icon, + iconDisabledColor: iconDisabledColor, + iconEnabledColor: iconEnabledColor, + iconSize: iconSize, + isDense: isDense, + isExpanded: isExpanded, + itemHeight: itemHeight, + focusColor: focusColor, + focusNode: focusNode, + autofocus: autofocus, + dropdownColor: dropdownColor, + menuMaxHeight: menuMaxHeight, + enableFeedback: enableFeedback, + alignment: alignment, + borderRadius: borderRadius, + inputDecoration: effectiveDecoration.copyWith(errorText: field.errorText), + isEmpty: isEmpty, + isFocused: Focus.of(context).hasFocus, ), ); }), diff --git a/packages/flutter/test/material/dropdown_form_field_test.dart b/packages/flutter/test/material/dropdown_form_field_test.dart index a8f8bd082b..018745b4f2 100644 --- a/packages/flutter/test/material/dropdown_form_field_test.dart +++ b/packages/flutter/test/material/dropdown_form_field_test.dart @@ -577,24 +577,26 @@ void main() { TestApp( textDirection: TextDirection.ltr, child: Material( - child: DropdownButtonFormField( - key: buttonKey, - value: value, - onChanged: onChanged, - items: menuItems.map>((String item) { - return DropdownMenuItem( - key: ValueKey(item), - value: item, - child: Text(item, key: ValueKey('${item}Text')), - ); - }).toList(), + child: Center( + child: DropdownButtonFormField( + key: buttonKey, + value: value, + onChanged: onChanged, + items: menuItems.map>((String item) { + return DropdownMenuItem( + key: ValueKey(item), + value: item, + child: Text(item, key: ValueKey('${item}Text')), + ); + }).toList(), + ), ), ), ), ); final RenderBox box = tester.renderObject(find.byType(dropdownButtonType)); - expect(box.size.height, 24.0); + expect(box.size.height, 48.0); }); testWidgets('DropdownButtonFormField - custom text style', (WidgetTester tester) async { diff --git a/packages/flutter/test/material/dropdown_test.dart b/packages/flutter/test/material/dropdown_test.dart index 5d1000954e..947d5ecc17 100644 --- a/packages/flutter/test/material/dropdown_test.dart +++ b/packages/flutter/test/material/dropdown_test.dart @@ -2395,13 +2395,13 @@ void main() { final UniqueKey buttonKey = UniqueKey(); final FocusNode focusNode = FocusNode(debugLabel: 'DropdownButton'); await tester.pumpWidget(buildFrame(buttonKey: buttonKey, onChanged: onChanged, focusNode: focusNode, autofocus: true)); - await tester.pump(); // Pump a frame for autofocus to take effect. + await tester.pumpAndSettle(); // Pump a frame for autofocus to take effect. expect(focusNode.hasPrimaryFocus, isTrue); - final Finder buttonFinder = find.byKey(buttonKey); - expect(buttonFinder, paints ..rrect(rrect: const RRect.fromLTRBXY(0.0, 0.0, 104.0, 48.0, 4.0, 4.0), color: const Color(0x1f000000))); + expect(find.byType(Material), paints..rect(rect: const Rect.fromLTRB(348.0, 276.0, 452.0, 324.0), color: const Color(0x1f000000))); await tester.pumpWidget(buildFrame(buttonKey: buttonKey, onChanged: onChanged, focusNode: focusNode, focusColor: const Color(0xff00ff00))); - expect(buttonFinder, paints ..rrect(rrect: const RRect.fromLTRBXY(0.0, 0.0, 104.0, 48.0, 4.0, 4.0), color: const Color(0xff00ff00))); + await tester.pumpAndSettle(); // Pump a frame for autofocus to take effect. + expect(find.byType(Material), paints..rect(rect: const Rect.fromLTRB(348.0, 276.0, 452.0, 324.0), color: const Color(0x1f00ff00))); }); testWidgets('DropdownButtonFormField can be focused, and has focusColor', (WidgetTester tester) async { @@ -2409,13 +2409,13 @@ void main() { final UniqueKey buttonKey = UniqueKey(); final FocusNode focusNode = FocusNode(debugLabel: 'DropdownButtonFormField'); await tester.pumpWidget(buildFrame(isFormField: true, buttonKey: buttonKey, onChanged: onChanged, focusNode: focusNode, autofocus: true)); - await tester.pump(); // Pump a frame for autofocus to take effect. + await tester.pumpAndSettle(); // Pump a frame for autofocus to take effect. expect(focusNode.hasPrimaryFocus, isTrue); - final Finder buttonFinder = find.descendant(of: find.byKey(buttonKey), matching: find.byType(InputDecorator)); - expect(buttonFinder, paints ..rrect(rrect: const RRect.fromLTRBXY(0.0, 12.0, 800.0, 60.0, 4.0, 4.0), color: const Color(0x1f000000))); + expect(find.byType(Material), paints ..rect(rect: const Rect.fromLTRB(0.0, 264.0, 800.0, 336.0), color: const Color(0x1f000000))); await tester.pumpWidget(buildFrame(isFormField: true, buttonKey: buttonKey, onChanged: onChanged, focusNode: focusNode, focusColor: const Color(0xff00ff00))); - expect(buttonFinder, paints ..rrect(rrect: const RRect.fromLTRBXY(0.0, 12.0, 800.0, 60.0, 4.0, 4.0), color: const Color(0xff00ff00))); + await tester.pumpAndSettle(); // Pump a frame for autofocus to take effect. + expect(find.byType(Material), paints ..rect(rect: const Rect.fromLTRB(0.0, 264.0, 800.0, 336.0), color: const Color(0x1f00ff00))); }); testWidgets("DropdownButton won't be focused if not enabled", (WidgetTester tester) async { @@ -3413,7 +3413,7 @@ void main() { await tester.tap(find.text('One')); await tester.pumpAndSettle(); - await tester.tap(find.widgetWithText(InkWell, 'One')); + await tester.tap(find.widgetWithText(InkWell, 'One').last); await tester.pumpAndSettle(); expect(feedback.clickSoundCount, 1); expect(feedback.hapticCount, 0); @@ -3426,7 +3426,7 @@ void main() { await tester.tap(find.text('One')); await tester.pumpAndSettle(); - await tester.tap(find.widgetWithText(InkWell, 'One')); + await tester.tap(find.widgetWithText(InkWell, 'One').last); await tester.pumpAndSettle(); expect(feedback.clickSoundCount, 0); expect(feedback.hapticCount, 0); @@ -3437,7 +3437,7 @@ void main() { await tester.tap(find.text('One')); await tester.pumpAndSettle(); - await tester.tap(find.widgetWithText(InkWell, 'Two')); + await tester.tap(find.widgetWithText(InkWell, 'Two').last); await tester.pumpAndSettle(); expect(feedback.clickSoundCount, 1); expect(feedback.hapticCount, 0);