From 62674cee3d61b170bfba227c503953adeec4bc0a Mon Sep 17 00:00:00 2001 From: Justin McCandless Date: Thu, 8 Aug 2019 13:29:27 -0700 Subject: [PATCH] Interactive size const (#36964) (Breaking Change) Move some hardcoded pixel values to reusable constants. --- .../flutter/lib/src/cupertino/button.dart | 13 +++----- .../flutter/lib/src/cupertino/constants.dart | 17 ++++++++++ .../flutter/lib/src/cupertino/nav_bar.dart | 3 +- packages/flutter/lib/src/material/chip.dart | 3 +- .../flutter/lib/src/material/constants.dart | 16 +++++++++- .../flutter/lib/src/material/data_table.dart | 6 ++-- .../flutter/lib/src/material/dropdown.dart | 2 +- .../lib/src/material/expansion_panel.dart | 3 +- .../flutter/lib/src/material/icon_button.dart | 11 ++++--- .../src/material/paginated_data_table.dart | 6 ++-- .../flutter/lib/src/material/popup_menu.dart | 15 +++++---- .../lib/src/material/range_slider.dart | 4 +-- .../flutter/lib/src/widgets/constants.dart | 16 ++++++++++ .../lib/src/widgets/editable_text.dart | 3 +- .../lib/src/widgets/text_selection.dart | 8 ++--- .../test/widgets/editable_text_test.dart | 32 +++++++++---------- 16 files changed, 104 insertions(+), 54 deletions(-) create mode 100644 packages/flutter/lib/src/cupertino/constants.dart create mode 100644 packages/flutter/lib/src/widgets/constants.dart diff --git a/packages/flutter/lib/src/cupertino/button.dart b/packages/flutter/lib/src/cupertino/button.dart index 0227b75f0c..b7203e7119 100644 --- a/packages/flutter/lib/src/cupertino/button.dart +++ b/packages/flutter/lib/src/cupertino/button.dart @@ -5,6 +5,7 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; +import 'constants.dart'; import 'theme.dart'; const Color _kDisabledBackground = Color(0xFFA9A9A9); @@ -33,7 +34,7 @@ class CupertinoButton extends StatefulWidget { this.padding, this.color, this.disabledColor, - this.minSize = 44.0, + this.minSize = kMinInteractiveDimensionCupertino, this.pressedOpacity = 0.1, this.borderRadius = const BorderRadius.all(Radius.circular(8.0)), @required this.onPressed, @@ -52,7 +53,7 @@ class CupertinoButton extends StatefulWidget { @required this.child, this.padding, this.disabledColor, - this.minSize = 44.0, + this.minSize = kMinInteractiveDimensionCupertino, this.pressedOpacity = 0.1, this.borderRadius = const BorderRadius.all(Radius.circular(8.0)), @required this.onPressed, @@ -94,12 +95,8 @@ class CupertinoButton extends StatefulWidget { /// Minimum size of the button. /// - /// Defaults to 44.0 which the iOS Human Interface Guideline recommends as the - /// minimum tappable area - /// - /// See also: - /// - /// * + /// Defaults to kMinInteractiveDimensionCupertino which the iOS Human + /// Interface Guidelines recommends as the minimum tappable area. final double minSize; /// The opacity that the button will fade to when it is pressed. diff --git a/packages/flutter/lib/src/cupertino/constants.dart b/packages/flutter/lib/src/cupertino/constants.dart new file mode 100644 index 0000000000..e3ec740281 --- /dev/null +++ b/packages/flutter/lib/src/cupertino/constants.dart @@ -0,0 +1,17 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +/// The minimum dimension of any interactive region according to the iOS Human +/// Interface Guidelines. +/// +/// This is used to avoid small regions that are hard for the user to interact +/// with. It applies to both dimensions of a region, so a square of size +/// kMinInteractiveDimension x kMinInteractiveDimension is the smallest +/// acceptable region that should respond to gestures. +/// +/// See also: +/// +/// * [kMinInteractiveDimension] +/// * +const double kMinInteractiveDimensionCupertino = 44.0; diff --git a/packages/flutter/lib/src/cupertino/nav_bar.dart b/packages/flutter/lib/src/cupertino/nav_bar.dart index fb4b94259e..6f25123a20 100644 --- a/packages/flutter/lib/src/cupertino/nav_bar.dart +++ b/packages/flutter/lib/src/cupertino/nav_bar.dart @@ -11,6 +11,7 @@ import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; import 'button.dart'; +import 'constants.dart'; import 'icons.dart'; import 'page_scaffold.dart'; import 'route.dart'; @@ -19,7 +20,7 @@ import 'theme.dart'; /// Standard iOS navigation bar height without the status bar. /// /// This height is constant and independent of accessibility as it is in iOS. -const double _kNavBarPersistentHeight = 44.0; +const double _kNavBarPersistentHeight = kMinInteractiveDimensionCupertino; /// Size increase from expanding the navigation bar into an iOS-11-style large title /// form in a [CustomScrollView]. diff --git a/packages/flutter/lib/src/material/chip.dart b/packages/flutter/lib/src/material/chip.dart index 367f84d752..ca321bac12 100644 --- a/packages/flutter/lib/src/material/chip.dart +++ b/packages/flutter/lib/src/material/chip.dart @@ -11,6 +11,7 @@ import 'package:flutter/widgets.dart'; import 'chip_theme.dart'; import 'colors.dart'; +import 'constants.dart'; import 'debug.dart'; import 'icons.dart'; import 'ink_well.dart'; @@ -1731,7 +1732,7 @@ class _RawChipState extends State with TickerProviderStateMixin. +const double kMinInteractiveDimension = 48.0; + /// The height of the toolbar component of the [AppBar]. const double kToolbarHeight = 56.0; @@ -11,7 +25,7 @@ const double kToolbarHeight = 56.0; const double kBottomNavigationBarHeight = 56.0; /// The height of a tab bar containing text. -const double kTextTabBarHeight = 48.0; +const double kTextTabBarHeight = kMinInteractiveDimension; /// The amount of time theme change animations should last. const Duration kThemeChangeDuration = Duration(milliseconds: 200); diff --git a/packages/flutter/lib/src/material/data_table.dart b/packages/flutter/lib/src/material/data_table.dart index 6082209531..cccd97b558 100644 --- a/packages/flutter/lib/src/material/data_table.dart +++ b/packages/flutter/lib/src/material/data_table.dart @@ -10,6 +10,7 @@ import 'package:flutter/widgets.dart'; import 'checkbox.dart'; import 'colors.dart'; +import 'constants.dart'; import 'debug.dart'; import 'divider.dart'; import 'dropdown.dart'; @@ -262,7 +263,7 @@ class DataTable extends StatelessWidget { this.sortColumnIndex, this.sortAscending = true, this.onSelectAll, - this.dataRowHeight = 48.0, + this.dataRowHeight = kMinInteractiveDimension, this.headingRowHeight = 56.0, this.horizontalMargin = 24.0, this.columnSpacing = 56.0, @@ -321,7 +322,8 @@ class DataTable extends StatelessWidget { /// The height of each row (excluding the row that contains column headings). /// - /// This value defaults to 48.0 to adhere to the Material Design specifications. + /// This value defaults to kMinInteractiveDimension to adhere to the Material + /// Design specifications. final double dataRowHeight; /// The height of the heading row. diff --git a/packages/flutter/lib/src/material/dropdown.dart b/packages/flutter/lib/src/material/dropdown.dart index 98baac9e28..f12b67ef7d 100644 --- a/packages/flutter/lib/src/material/dropdown.dart +++ b/packages/flutter/lib/src/material/dropdown.dart @@ -20,7 +20,7 @@ import 'shadows.dart'; import 'theme.dart'; const Duration _kDropdownMenuDuration = Duration(milliseconds: 300); -const double _kMenuItemHeight = 48.0; +const double _kMenuItemHeight = kMinInteractiveDimension; const double _kDenseButtonHeight = 24.0; const EdgeInsets _kMenuItemPadding = EdgeInsets.symmetric(horizontal: 16.0); const EdgeInsetsGeometry _kAlignedButtonPadding = EdgeInsetsDirectional.only(start: 16.0, end: 4.0); diff --git a/packages/flutter/lib/src/material/expansion_panel.dart b/packages/flutter/lib/src/material/expansion_panel.dart index 8e2fdeb992..73a43dd06c 100644 --- a/packages/flutter/lib/src/material/expansion_panel.dart +++ b/packages/flutter/lib/src/material/expansion_panel.dart @@ -5,13 +5,14 @@ import 'package:flutter/rendering.dart'; import 'package:flutter/widgets.dart'; +import 'constants.dart'; import 'expand_icon.dart'; import 'ink_well.dart'; import 'material_localizations.dart'; import 'mergeable_material.dart'; import 'theme.dart'; -const double _kPanelHeaderCollapsedHeight = 48.0; +const double _kPanelHeaderCollapsedHeight = kMinInteractiveDimension; const double _kPanelHeaderExpandedHeight = 64.0; class _SaltedKey extends LocalKey { diff --git a/packages/flutter/lib/src/material/icon_button.dart b/packages/flutter/lib/src/material/icon_button.dart index c0e518b904..331a019f05 100644 --- a/packages/flutter/lib/src/material/icon_button.dart +++ b/packages/flutter/lib/src/material/icon_button.dart @@ -7,6 +7,7 @@ import 'dart:math' as math; import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; +import 'constants.dart'; import 'debug.dart'; import 'icons.dart'; import 'ink_well.dart'; @@ -15,8 +16,8 @@ import 'theme.dart'; import 'tooltip.dart'; // Minimum logical pixel size of the IconButton. -// See: -const double _kMinButtonSize = 48.0; +// See: . +const double _kMinButtonSize = kMinInteractiveDimension; /// A material design icon button. /// @@ -31,9 +32,9 @@ const double _kMinButtonSize = 48.0; /// /// Requires one of its ancestors to be a [Material] widget. /// -/// The hit region of an icon button will, if possible, be at least 48.0 pixels -/// in size, regardless of the actual [iconSize], to satisfy the [touch target -/// size](https://material.io/guidelines/layout/metrics-keylines.html#metrics-keylines-touch-target-size) +/// The hit region of an icon button will, if possible, be at least +/// kMinInteractiveDimension pixels in size, regardless of the actual +/// [iconSize], to satisfy the [touch target size](https://material.io/guidelines/layout/metrics-keylines.html#metrics-keylines-touch-target-size) /// requirements in the Material Design specification. The [alignment] controls /// how the icon itself is positioned within the hit region. /// diff --git a/packages/flutter/lib/src/material/paginated_data_table.dart b/packages/flutter/lib/src/material/paginated_data_table.dart index 8f61b35038..0627791f4d 100644 --- a/packages/flutter/lib/src/material/paginated_data_table.dart +++ b/packages/flutter/lib/src/material/paginated_data_table.dart @@ -11,6 +11,7 @@ import 'package:flutter/gestures.dart' show DragStartBehavior; import 'button_bar.dart'; import 'button_theme.dart'; import 'card.dart'; +import 'constants.dart'; import 'data_table.dart'; import 'data_table_source.dart'; import 'debug.dart'; @@ -70,7 +71,7 @@ class PaginatedDataTable extends StatefulWidget { this.sortColumnIndex, this.sortAscending = true, this.onSelectAll, - this.dataRowHeight = 48.0, + this.dataRowHeight = kMinInteractiveDimension, this.headingRowHeight = 56.0, this.horizontalMargin = 24.0, this.columnSpacing = 56.0, @@ -141,7 +142,8 @@ class PaginatedDataTable extends StatefulWidget { /// The height of each row (excluding the row that contains column headings). /// - /// This value is optional and defaults to 48.0 if not specified. + /// This value is optional and defaults to kMinInteractiveDimension if not + /// specified. final double dataRowHeight; /// The height of the heading row. diff --git a/packages/flutter/lib/src/material/popup_menu.dart b/packages/flutter/lib/src/material/popup_menu.dart index 11218f80b6..f0e121540a 100644 --- a/packages/flutter/lib/src/material/popup_menu.dart +++ b/packages/flutter/lib/src/material/popup_menu.dart @@ -30,7 +30,7 @@ const Duration _kMenuDuration = Duration(milliseconds: 300); const double _kBaselineOffsetFromBottom = 20.0; const double _kMenuCloseIntervalEnd = 2.0 / 3.0; const double _kMenuHorizontalPadding = 16.0; -const double _kMenuItemHeight = 48.0; +const double _kMenuItemHeight = kMinInteractiveDimension; const double _kMenuDividerHeight = 16.0; const double _kMenuMaxWidth = 5.0 * _kMenuWidthStep; const double _kMenuMinWidth = 2.0 * _kMenuWidthStep; @@ -132,8 +132,8 @@ class _PopupMenuDividerState extends State { /// /// Typically the [child] of a [PopupMenuItem] is a [Text] widget. More /// elaborate menus with icons can use a [ListTile]. By default, a -/// [PopupMenuItem] is 48 pixels high. If you use a widget with a different -/// height, it must be specified in the [height] property. +/// [PopupMenuItem] is kMinInteractiveDimension pixels high. If you use a widget +/// with a different height, it must be specified in the [height] property. /// /// {@tool sample} /// @@ -189,7 +189,7 @@ class PopupMenuItem extends PopupMenuEntry { /// The height of the entry. /// - /// Defaults to 48 pixels. + /// Defaults to kMinInteractiveDimension pixels. @override final double height; @@ -292,9 +292,10 @@ class PopupMenuItemState> extends State { /// To show a popup menu, use the [showMenu] function. To create a button that /// shows a popup menu, consider using [PopupMenuButton]. /// -/// A [CheckedPopupMenuItem] is 48 pixels high, which matches the default height -/// of a [PopupMenuItem]. The horizontal layout uses a [ListTile]; the checkmark -/// is an [Icons.done] icon, shown in the [ListTile.leading] position. +/// A [CheckedPopupMenuItem] is kMinInteractiveDimension pixels high, which +/// matches the default height of a [PopupMenuItem]. The horizontal layout uses +/// a [ListTile]; the checkmark is an [Icons.done] icon, shown in the +/// [ListTile.leading] position. /// /// {@tool sample} /// diff --git a/packages/flutter/lib/src/material/range_slider.dart b/packages/flutter/lib/src/material/range_slider.dart index 5e2220f315..45b11842bb 100644 --- a/packages/flutter/lib/src/material/range_slider.dart +++ b/packages/flutter/lib/src/material/range_slider.dart @@ -333,7 +333,7 @@ class RangeSlider extends StatefulWidget { final RangeSemanticFormatterCallback semanticFormatterCallback; // Touch width for the tap boundary of the slider thumbs. - static const double _minTouchTargetWidth = 48; + static const double _minTouchTargetWidth = kMinInteractiveDimension; @override _RangeSliderState createState() => _RangeSliderState(); @@ -1387,4 +1387,4 @@ class _RenderRangeSlider extends RenderBox { double _decreaseValue(double value) { return (value - _semanticActionUnit).clamp(0.0, 1.0); } -} \ No newline at end of file +} diff --git a/packages/flutter/lib/src/widgets/constants.dart b/packages/flutter/lib/src/widgets/constants.dart new file mode 100644 index 0000000000..837283eab6 --- /dev/null +++ b/packages/flutter/lib/src/widgets/constants.dart @@ -0,0 +1,16 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +/// The minimum dimension of any interactive region according to the Material +/// guidelines. +/// +/// This is used to avoid small regions that are hard for the user to interact +/// with. It applies to both dimensions of a region, so a square of size +/// kMinInteractiveDimension x kMinInteractiveDimension is the smallest +/// acceptable region that should respond to gestures. +/// +/// See also: +/// +/// * [kMinInteractiveDimensionCupertino] +const double kMinInteractiveDimension = 48.0; diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index 435b5a17cd..150e0e864a 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -16,6 +16,7 @@ import 'package:flutter/gestures.dart' show DragStartBehavior; import 'automatic_keep_alive.dart'; import 'basic.dart'; import 'binding.dart'; +import 'constants.dart'; import 'debug.dart'; import 'focus_manager.dart'; import 'focus_scope.dart'; @@ -1446,7 +1447,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien .getHandleSize(renderEditable.preferredLineHeight).height; final double interactiveHandleHeight = math.max( handleHeight, - kMinInteractiveSize, + kMinInteractiveDimension, ); final Offset anchor = _selectionOverlay.selectionControls .getHandleAnchor( diff --git a/packages/flutter/lib/src/widgets/text_selection.dart b/packages/flutter/lib/src/widgets/text_selection.dart index 8afb0d56a3..d92e30d83c 100644 --- a/packages/flutter/lib/src/widgets/text_selection.dart +++ b/packages/flutter/lib/src/widgets/text_selection.dart @@ -12,6 +12,7 @@ import 'package:flutter/scheduler.dart'; import 'package:flutter/services.dart'; import 'basic.dart'; +import 'constants.dart'; import 'container.dart'; import 'editable_text.dart'; import 'framework.dart'; @@ -618,11 +619,6 @@ class _TextSelectionHandleOverlay extends StatefulWidget { } } -/// The minimum size that a widget should be in order to be easily interacted -/// with by the user. -@visibleForTesting -const double kMinInteractiveSize = 48.0; - class _TextSelectionHandleOverlayState extends State<_TextSelectionHandleOverlay> with SingleTickerProviderStateMixin { Offset _dragPosition; @@ -749,7 +745,7 @@ class _TextSelectionHandleOverlayState // Make sure the GestureDetector is big enough to be easily interactive. final Rect interactiveRect = handleRect.expandToInclude( - Rect.fromCircle(center: handleRect.center, radius: kMinInteractiveSize / 2), + Rect.fromCircle(center: handleRect.center, radius: kMinInteractiveDimension/ 2), ); final RelativeRect padding = RelativeRect.fromLTRB( math.max((interactiveRect.width - handleRect.width) / 2, 0), diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index f75e7f4808..2298383e72 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -2268,8 +2268,8 @@ void main() { expect( pos, inExclusiveRange( - 0 - kMinInteractiveSize, - 0 + kMinInteractiveSize, + 0 - kMinInteractiveDimension, + 0 + kMinInteractiveDimension, ), ); break; @@ -2277,8 +2277,8 @@ void main() { expect( pos, inExclusiveRange( - viewport.width - kMinInteractiveSize, - viewport.width + kMinInteractiveSize, + viewport.width - kMinInteractiveDimension, + viewport.width + kMinInteractiveDimension, ), ); break; @@ -2286,8 +2286,8 @@ void main() { expect( pos, inExclusiveRange( - 0 - kMinInteractiveSize, - viewport.width + kMinInteractiveSize, + 0 - kMinInteractiveDimension, + viewport.width + kMinInteractiveDimension, ), ); break; @@ -2381,15 +2381,15 @@ void main() { expect( handles[0].localToGlobal(Offset.zero).dx, inExclusiveRange( - -kMinInteractiveSize, - kMinInteractiveSize, + -kMinInteractiveDimension, + kMinInteractiveDimension, ), ); expect( handles[1].localToGlobal(Offset.zero).dx, inExclusiveRange( - 70.0 - kMinInteractiveSize, - 70.0 + kMinInteractiveSize, + 70.0 - kMinInteractiveDimension, + 70.0 + kMinInteractiveDimension, ), ); expect(state.selectionOverlay.handlesAreVisible, isTrue); @@ -2497,8 +2497,8 @@ void main() { expect( pos, inExclusiveRange( - 0 - kMinInteractiveSize, - 0 + kMinInteractiveSize, + 0 - kMinInteractiveDimension, + 0 + kMinInteractiveDimension, ), ); break; @@ -2506,8 +2506,8 @@ void main() { expect( pos, inExclusiveRange( - viewport.width - kMinInteractiveSize, - viewport.width + kMinInteractiveSize, + viewport.width - kMinInteractiveDimension, + viewport.width + kMinInteractiveDimension, ), ); break; @@ -2515,8 +2515,8 @@ void main() { expect( pos, inExclusiveRange( - 0 - kMinInteractiveSize, - viewport.width + kMinInteractiveSize, + 0 - kMinInteractiveDimension, + viewport.width + kMinInteractiveDimension, ), ); break;