From 7ff5f81a2ea7190a48131c52e8a14dc65cf9f500 Mon Sep 17 00:00:00 2001 From: Qun Cheng <36861262+QuncCccccc@users.noreply.github.com> Date: Fri, 26 Jan 2024 00:20:21 +0000 Subject: [PATCH] Fix `SegmentedButton` default size and default tappable size (#142243) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix https://github.com/flutter/flutter/issues/121493 `SegmentedButton` uses `TextButton` for each segments. When we have `MaterialTapTargetSize.padded` for `TextButton`, we make sure the minimum tap target size is 48.0( this value can be adjusted by visual density), even tough the actual button size is smaller. When `SegmentedButton` paints segments by using `MultiChildRenderObjectWidget`, it also includes the tap target size so the button that it actually draws always has the same height as the height of the tap target size. To fix it, this PR firstly calculate the actual height of a text button in `SegmentedButton` class, then we can get the height delta if there is. Then the the value of (Segmented button render box height - the delta) would be the actual button size that we should see. For now, we are not able to customize the min, max, fixed size in [`SegmentedButton` style](https://api.flutter.dev/flutter/material/SegmentedButton/style.html). So the standard button height is always 40 and can only be customized by `style.visualDensity` and `style.tapTargetSize`; `SegmentedButton` only simulates the `TextButton` behavior when `TextButton`'s height is its default value. ![Screenshot 2024-01-25 at 11 45 42 AM](https://github.com/flutter/flutter/assets/36861262/7451fa96-6d45-4cd3-a894-ca71e776c8ef) https://github.com/flutter/flutter/assets/36861262/15ca6034-e6e0-4cc6-8fe3-808b4bd6a920 --- .../lib/src/material/segmented_button.dart | 40 +++++++++++++++-- .../test/material/segmented_button_test.dart | 44 ++++++++++++++++++- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/packages/flutter/lib/src/material/segmented_button.dart b/packages/flutter/lib/src/material/segmented_button.dart index 84b45a3f39..b2dd0e1239 100644 --- a/packages/flutter/lib/src/material/segmented_button.dart +++ b/packages/flutter/lib/src/material/segmented_button.dart @@ -3,14 +3,17 @@ // found in the LICENSE file. import 'dart:math' as math; +import 'dart:math'; import 'package:flutter/foundation.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/widgets.dart'; + import 'button_style.dart'; import 'button_style_button.dart'; import 'color_scheme.dart'; import 'colors.dart'; +import 'constants.dart'; import 'icons.dart'; import 'ink_well.dart'; import 'material.dart'; @@ -511,18 +514,33 @@ class SegmentedButtonState extends State> { final BorderSide disabledSide = resolve((ButtonStyle? style) => style?.side, disabledState) ?? BorderSide.none; final OutlinedBorder enabledBorder = resolvedEnabledBorder.copyWith(side: enabledSide); final OutlinedBorder disabledBorder = resolvedDisabledBorder.copyWith(side: disabledSide); + final VisualDensity resolvedVisualDensity = segmentStyle.visualDensity ?? segmentThemeStyle.visualDensity ?? Theme.of(context).visualDensity; + final EdgeInsetsGeometry resolvedPadding = resolve((ButtonStyle? style) => style?.padding, enabledState) ?? EdgeInsets.zero; + final MaterialTapTargetSize resolvedTapTargetSize = segmentStyle.tapTargetSize ?? segmentThemeStyle.tapTargetSize ?? Theme.of(context).materialTapTargetSize; + final double fontSize = resolve((ButtonStyle? style) => style?.textStyle, enabledState)?.fontSize ?? 20.0; final List buttons = widget.segments.map(buttonFor).toList(); + final Offset densityAdjustment = resolvedVisualDensity.baseSizeAdjustment; + const double textButtonMinHeight = 40.0; + + final double adjustButtonMinHeight = textButtonMinHeight + densityAdjustment.dy; + final double effectiveVerticalPadding = resolvedPadding.vertical + densityAdjustment.dy * 2; + final double effectedButtonHeight = max(fontSize + effectiveVerticalPadding, adjustButtonMinHeight); + final double tapTargetVerticalPadding = switch (resolvedTapTargetSize) { + MaterialTapTargetSize.shrinkWrap => 0.0, + MaterialTapTargetSize.padded => max(0, kMinInteractiveDimension + densityAdjustment.dy - effectedButtonHeight) + }; + return Material( type: MaterialType.transparency, - shape: enabledBorder.copyWith(side: BorderSide.none), elevation: resolve((ButtonStyle? style) => style?.elevation)!, shadowColor: resolve((ButtonStyle? style) => style?.shadowColor), surfaceTintColor: resolve((ButtonStyle? style) => style?.surfaceTintColor), child: TextButtonTheme( data: TextButtonThemeData(style: segmentThemeStyle), child: _SegmentedButtonRenderWidget( + tapTargetVerticalPadding: tapTargetVerticalPadding, segments: widget.segments, enabledBorder: _enabled ? enabledBorder : disabledBorder, disabledBorder: disabledBorder, @@ -569,6 +587,7 @@ class _SegmentedButtonRenderWidget extends MultiChildRenderObjectWidget { required this.enabledBorder, required this.disabledBorder, required this.direction, + required this.tapTargetVerticalPadding, required super.children, }) : assert(children.length == segments.length); @@ -576,6 +595,7 @@ class _SegmentedButtonRenderWidget extends MultiChildRenderObjectWidget { final OutlinedBorder enabledBorder; final OutlinedBorder disabledBorder; final TextDirection direction; + final double tapTargetVerticalPadding; @override RenderObject createRenderObject(BuildContext context) { @@ -584,6 +604,7 @@ class _SegmentedButtonRenderWidget extends MultiChildRenderObjectWidget { enabledBorder: enabledBorder, disabledBorder: disabledBorder, textDirection: direction, + tapTargetVerticalPadding: tapTargetVerticalPadding, ); } @@ -611,10 +632,12 @@ class _RenderSegmentedButton extends RenderBox with required OutlinedBorder enabledBorder, required OutlinedBorder disabledBorder, required TextDirection textDirection, + required double tapTargetVerticalPadding, }) : _segments = segments, _enabledBorder = enabledBorder, _disabledBorder = disabledBorder, - _textDirection = textDirection; + _textDirection = textDirection, + _tapTargetVerticalPadding = tapTargetVerticalPadding; List> get segments => _segments; List> _segments; @@ -656,6 +679,16 @@ class _RenderSegmentedButton extends RenderBox with markNeedsLayout(); } + double get tapTargetVerticalPadding => _tapTargetVerticalPadding; + double _tapTargetVerticalPadding; + set tapTargetVerticalPadding(double value) { + if (value == _tapTargetVerticalPadding) { + return; + } + _tapTargetVerticalPadding = value; + markNeedsLayout(); + } + @override double computeMinIntrinsicWidth(double height) { RenderBox? child = firstChild; @@ -799,7 +832,8 @@ class _RenderSegmentedButton extends RenderBox with @override void paint(PaintingContext context, Offset offset) { - final Rect borderRect = offset & size; + + final Rect borderRect = (offset + Offset(0, tapTargetVerticalPadding / 2)) & (Size(size.width, size.height - tapTargetVerticalPadding)); final Path borderClipPath = enabledBorder.getInnerPath(borderRect, textDirection: textDirection); RenderBox? child = firstChild; RenderBox? previousChild; diff --git a/packages/flutter/test/material/segmented_button_test.dart b/packages/flutter/test/material/segmented_button_test.dart index 89ce760bdf..ce4dda4d2f 100644 --- a/packages/flutter/test/material/segmented_button_test.dart +++ b/packages/flutter/test/material/segmented_button_test.dart @@ -751,7 +751,6 @@ void main() { of: find.byType(SegmentedButton), matching: find.byType(Material), ).first); - expect(material.shape, styleFromStyle.shape?.resolve(enabled)?.copyWith(side: BorderSide.none)); expect(material.elevation, styleFromStyle.elevation?.resolve(enabled)); expect(material.shadowColor, styleFromStyle.shadowColor?.resolve(enabled)); expect(material.surfaceTintColor, styleFromStyle.surfaceTintColor?.resolve(enabled)); @@ -813,6 +812,49 @@ void main() { state = tester.state(find.byType(SegmentedButton)); expect(state.statesControllers.values.first.value, states); }); + + testWidgets('Min button hit target height is 48.0 and min (painted) button height is 40 ' + 'by default with standard density and MaterialTapTargetSize.padded', (WidgetTester tester) async { + final ThemeData theme = ThemeData(); + await tester.pumpWidget( + MaterialApp( + theme: theme, + home: Scaffold( + body: Center( + child: Column( + children: [ + SegmentedButton( + segments: const >[ + ButtonSegment(value: 0, label: Text('Day'), icon: Icon(Icons.calendar_view_day)), + ButtonSegment(value: 1, label: Text('Week'), icon: Icon(Icons.calendar_view_week)), + ButtonSegment(value: 2, label: Text('Month'), icon: Icon(Icons.calendar_view_month)), + ButtonSegment(value: 3, label: Text('Year'), icon: Icon(Icons.calendar_today)), + ], + selected: const {0}, + onSelectionChanged: (Set value) {}, + ), + ], + ), + ), + ), + ), + ); + + expect(theme.visualDensity, VisualDensity.standard); + expect(theme.materialTapTargetSize, MaterialTapTargetSize.padded); + + final Finder button = find.byType(SegmentedButton); + expect(tester.getSize(button).height, 48.0); + expect( + find.byType(SegmentedButton), + paints..rrect( + style: PaintingStyle.stroke, + strokeWidth: 1.0, + // Button border height is button.bottom(43.5) - button.top(4.5) + stoke width(1) = 40. + rrect: RRect.fromLTRBR(0.5, 4.5, 497.5, 43.5, const Radius.circular(19.5)) + ) + ); + }); } Set enabled = const {};