Unverified Commit 7ff5f81a authored by Qun Cheng's avatar Qun Cheng Committed by GitHub

Fix `SegmentedButton` default size and default tappable size (#142243)

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
parent a6c3ad23
...@@ -3,14 +3,17 @@ ...@@ -3,14 +3,17 @@
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:math' as math; import 'dart:math' as math;
import 'dart:math';
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
import 'package:flutter/widgets.dart'; import 'package:flutter/widgets.dart';
import 'button_style.dart'; import 'button_style.dart';
import 'button_style_button.dart'; import 'button_style_button.dart';
import 'color_scheme.dart'; import 'color_scheme.dart';
import 'colors.dart'; import 'colors.dart';
import 'constants.dart';
import 'icons.dart'; import 'icons.dart';
import 'ink_well.dart'; import 'ink_well.dart';
import 'material.dart'; import 'material.dart';
...@@ -511,18 +514,33 @@ class SegmentedButtonState<T> extends State<SegmentedButton<T>> { ...@@ -511,18 +514,33 @@ class SegmentedButtonState<T> extends State<SegmentedButton<T>> {
final BorderSide disabledSide = resolve<BorderSide?>((ButtonStyle? style) => style?.side, disabledState) ?? BorderSide.none; final BorderSide disabledSide = resolve<BorderSide?>((ButtonStyle? style) => style?.side, disabledState) ?? BorderSide.none;
final OutlinedBorder enabledBorder = resolvedEnabledBorder.copyWith(side: enabledSide); final OutlinedBorder enabledBorder = resolvedEnabledBorder.copyWith(side: enabledSide);
final OutlinedBorder disabledBorder = resolvedDisabledBorder.copyWith(side: disabledSide); final OutlinedBorder disabledBorder = resolvedDisabledBorder.copyWith(side: disabledSide);
final VisualDensity resolvedVisualDensity = segmentStyle.visualDensity ?? segmentThemeStyle.visualDensity ?? Theme.of(context).visualDensity;
final EdgeInsetsGeometry resolvedPadding = resolve<EdgeInsetsGeometry?>((ButtonStyle? style) => style?.padding, enabledState) ?? EdgeInsets.zero;
final MaterialTapTargetSize resolvedTapTargetSize = segmentStyle.tapTargetSize ?? segmentThemeStyle.tapTargetSize ?? Theme.of(context).materialTapTargetSize;
final double fontSize = resolve<TextStyle?>((ButtonStyle? style) => style?.textStyle, enabledState)?.fontSize ?? 20.0;
final List<Widget> buttons = widget.segments.map(buttonFor).toList(); final List<Widget> 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( return Material(
type: MaterialType.transparency, type: MaterialType.transparency,
shape: enabledBorder.copyWith(side: BorderSide.none),
elevation: resolve<double?>((ButtonStyle? style) => style?.elevation)!, elevation: resolve<double?>((ButtonStyle? style) => style?.elevation)!,
shadowColor: resolve<Color?>((ButtonStyle? style) => style?.shadowColor), shadowColor: resolve<Color?>((ButtonStyle? style) => style?.shadowColor),
surfaceTintColor: resolve<Color?>((ButtonStyle? style) => style?.surfaceTintColor), surfaceTintColor: resolve<Color?>((ButtonStyle? style) => style?.surfaceTintColor),
child: TextButtonTheme( child: TextButtonTheme(
data: TextButtonThemeData(style: segmentThemeStyle), data: TextButtonThemeData(style: segmentThemeStyle),
child: _SegmentedButtonRenderWidget<T>( child: _SegmentedButtonRenderWidget<T>(
tapTargetVerticalPadding: tapTargetVerticalPadding,
segments: widget.segments, segments: widget.segments,
enabledBorder: _enabled ? enabledBorder : disabledBorder, enabledBorder: _enabled ? enabledBorder : disabledBorder,
disabledBorder: disabledBorder, disabledBorder: disabledBorder,
...@@ -569,6 +587,7 @@ class _SegmentedButtonRenderWidget<T> extends MultiChildRenderObjectWidget { ...@@ -569,6 +587,7 @@ class _SegmentedButtonRenderWidget<T> extends MultiChildRenderObjectWidget {
required this.enabledBorder, required this.enabledBorder,
required this.disabledBorder, required this.disabledBorder,
required this.direction, required this.direction,
required this.tapTargetVerticalPadding,
required super.children, required super.children,
}) : assert(children.length == segments.length); }) : assert(children.length == segments.length);
...@@ -576,6 +595,7 @@ class _SegmentedButtonRenderWidget<T> extends MultiChildRenderObjectWidget { ...@@ -576,6 +595,7 @@ class _SegmentedButtonRenderWidget<T> extends MultiChildRenderObjectWidget {
final OutlinedBorder enabledBorder; final OutlinedBorder enabledBorder;
final OutlinedBorder disabledBorder; final OutlinedBorder disabledBorder;
final TextDirection direction; final TextDirection direction;
final double tapTargetVerticalPadding;
@override @override
RenderObject createRenderObject(BuildContext context) { RenderObject createRenderObject(BuildContext context) {
...@@ -584,6 +604,7 @@ class _SegmentedButtonRenderWidget<T> extends MultiChildRenderObjectWidget { ...@@ -584,6 +604,7 @@ class _SegmentedButtonRenderWidget<T> extends MultiChildRenderObjectWidget {
enabledBorder: enabledBorder, enabledBorder: enabledBorder,
disabledBorder: disabledBorder, disabledBorder: disabledBorder,
textDirection: direction, textDirection: direction,
tapTargetVerticalPadding: tapTargetVerticalPadding,
); );
} }
...@@ -611,10 +632,12 @@ class _RenderSegmentedButton<T> extends RenderBox with ...@@ -611,10 +632,12 @@ class _RenderSegmentedButton<T> extends RenderBox with
required OutlinedBorder enabledBorder, required OutlinedBorder enabledBorder,
required OutlinedBorder disabledBorder, required OutlinedBorder disabledBorder,
required TextDirection textDirection, required TextDirection textDirection,
required double tapTargetVerticalPadding,
}) : _segments = segments, }) : _segments = segments,
_enabledBorder = enabledBorder, _enabledBorder = enabledBorder,
_disabledBorder = disabledBorder, _disabledBorder = disabledBorder,
_textDirection = textDirection; _textDirection = textDirection,
_tapTargetVerticalPadding = tapTargetVerticalPadding;
List<ButtonSegment<T>> get segments => _segments; List<ButtonSegment<T>> get segments => _segments;
List<ButtonSegment<T>> _segments; List<ButtonSegment<T>> _segments;
...@@ -656,6 +679,16 @@ class _RenderSegmentedButton<T> extends RenderBox with ...@@ -656,6 +679,16 @@ class _RenderSegmentedButton<T> extends RenderBox with
markNeedsLayout(); markNeedsLayout();
} }
double get tapTargetVerticalPadding => _tapTargetVerticalPadding;
double _tapTargetVerticalPadding;
set tapTargetVerticalPadding(double value) {
if (value == _tapTargetVerticalPadding) {
return;
}
_tapTargetVerticalPadding = value;
markNeedsLayout();
}
@override @override
double computeMinIntrinsicWidth(double height) { double computeMinIntrinsicWidth(double height) {
RenderBox? child = firstChild; RenderBox? child = firstChild;
...@@ -799,7 +832,8 @@ class _RenderSegmentedButton<T> extends RenderBox with ...@@ -799,7 +832,8 @@ class _RenderSegmentedButton<T> extends RenderBox with
@override @override
void paint(PaintingContext context, Offset offset) { 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); final Path borderClipPath = enabledBorder.getInnerPath(borderRect, textDirection: textDirection);
RenderBox? child = firstChild; RenderBox? child = firstChild;
RenderBox? previousChild; RenderBox? previousChild;
......
...@@ -751,7 +751,6 @@ void main() { ...@@ -751,7 +751,6 @@ void main() {
of: find.byType(SegmentedButton<int>), of: find.byType(SegmentedButton<int>),
matching: find.byType(Material), matching: find.byType(Material),
).first); ).first);
expect(material.shape, styleFromStyle.shape?.resolve(enabled)?.copyWith(side: BorderSide.none));
expect(material.elevation, styleFromStyle.elevation?.resolve(enabled)); expect(material.elevation, styleFromStyle.elevation?.resolve(enabled));
expect(material.shadowColor, styleFromStyle.shadowColor?.resolve(enabled)); expect(material.shadowColor, styleFromStyle.shadowColor?.resolve(enabled));
expect(material.surfaceTintColor, styleFromStyle.surfaceTintColor?.resolve(enabled)); expect(material.surfaceTintColor, styleFromStyle.surfaceTintColor?.resolve(enabled));
...@@ -813,6 +812,49 @@ void main() { ...@@ -813,6 +812,49 @@ void main() {
state = tester.state(find.byType(SegmentedButton<int>)); state = tester.state(find.byType(SegmentedButton<int>));
expect(state.statesControllers.values.first.value, states); 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: <Widget>[
SegmentedButton<int>(
segments: const <ButtonSegment<int>>[
ButtonSegment<int>(value: 0, label: Text('Day'), icon: Icon(Icons.calendar_view_day)),
ButtonSegment<int>(value: 1, label: Text('Week'), icon: Icon(Icons.calendar_view_week)),
ButtonSegment<int>(value: 2, label: Text('Month'), icon: Icon(Icons.calendar_view_month)),
ButtonSegment<int>(value: 3, label: Text('Year'), icon: Icon(Icons.calendar_today)),
],
selected: const <int>{0},
onSelectionChanged: (Set<int> value) {},
),
],
),
),
),
),
);
expect(theme.visualDensity, VisualDensity.standard);
expect(theme.materialTapTargetSize, MaterialTapTargetSize.padded);
final Finder button = find.byType(SegmentedButton<int>);
expect(tester.getSize(button).height, 48.0);
expect(
find.byType(SegmentedButton<int>),
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<MaterialState> enabled = const <MaterialState>{}; Set<MaterialState> enabled = const <MaterialState>{};
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment