From b0046b1811132cec74a29e2ebd3e8c0064b1d4ea Mon Sep 17 00:00:00 2001 From: Natalie Sampsell Date: Tue, 7 Aug 2018 13:01:15 -0700 Subject: [PATCH] Segmented control fixes (#20202) Segment width now determined by width of widest child + children widgets now centered within segments --- bin/internal/goldens.version | 3 +- .../lib/src/cupertino/segmented_control.dart | 35 ++-- .../cupertino/segmented_control_test.dart | 195 +++++++++++------- 3 files changed, 137 insertions(+), 96 deletions(-) diff --git a/bin/internal/goldens.version b/bin/internal/goldens.version index e717d09a45..59df7b34e3 100644 --- a/bin/internal/goldens.version +++ b/bin/internal/goldens.version @@ -1,2 +1 @@ -64b7a3a7aef2fea9c7529d4834bf9eb3d85602d8 - +46cf554baf4840c326bbceaa51b11534069bb557 \ No newline at end of file diff --git a/packages/flutter/lib/src/cupertino/segmented_control.dart b/packages/flutter/lib/src/cupertino/segmented_control.dart index b6c7866dc2..99071a6530 100644 --- a/packages/flutter/lib/src/cupertino/segmented_control.dart +++ b/packages/flutter/lib/src/cupertino/segmented_control.dart @@ -46,10 +46,11 @@ const Duration _kFadeDuration = Duration(milliseconds: 165); /// The [children] will be displayed in the order of the keys in the [Map]. /// The height of the segmented control is determined by the height of the /// tallest widget provided as a value in the [Map] of [children]. -/// The width of the segmented control is determined by the horizontal -/// constraints on its parent. The available horizontal space is divided by -/// the number of provided [children] to determine the width of each widget. -/// The selection area for each of the widgets in the [Map] of +/// The width of each child in the segmented control will be equal to the width +/// of widest child, unless the combined width of the children is wider than +/// the available horizontal space. In this case, the available horizontal space +/// is divided by the number of provided [children] to determine the width of +/// each widget. The selection area for each of the widgets in the [Map] of /// [children] will then be expanded to fill the calculated space, so each /// widget will appear to have the same dimensions. /// @@ -75,10 +76,10 @@ class SegmentedControl extends StatefulWidget { /// in the [onValueChanged] callback when a new value from the [children] map /// is selected. /// - /// The [groupValue] must be one of the keys in the [children] map. /// The [groupValue] is the currently selected value for the segmented control. /// If no [groupValue] is provided, or the [groupValue] is null, no widget will - /// appear as selected. + /// appear as selected. The [groupValue] must be either null or one of the keys + /// in the [children] map. SegmentedControl({ Key key, @required this.children, @@ -91,7 +92,8 @@ class SegmentedControl extends StatefulWidget { }) : assert(children != null), assert(children.length >= 2), assert(onValueChanged != null), - assert(groupValue == null || children.keys.any((T child) => child == groupValue)), + assert(groupValue == null || children.keys.any((T child) => child == groupValue), + 'The groupValue must be either null or one of the keys in the children map.'), assert(unselectedColor != null), assert(selectedColor != null), assert(borderColor != null), @@ -189,7 +191,7 @@ class SegmentedControl extends StatefulWidget { /// This attribute must not be null. /// /// If this attribute is unspecified, this color will default to - /// 'Color(0x33007AFF)', a light, partially-transparent blue color. + /// `Color(0x33007AFF)`, a light, partially-transparent blue color. final Color pressedColor; @override @@ -346,7 +348,10 @@ class _SegmentedControlState extends State> color: getTextColor(index, currentKey), ); - Widget child = widget.children[currentKey]; + Widget child = new Center( + child: widget.children[currentKey], + ); + child = new GestureDetector( onTapDown: (TapDownDetails event) { _onTapDown(currentKey); @@ -599,15 +604,11 @@ class _RenderSegmentedControl extends RenderBox void performLayout() { double maxHeight = _kMinSegmentedControlHeight; - double childWidth; - if (constraints.maxWidth.isFinite) { - childWidth = constraints.maxWidth / childCount; - } else { - childWidth = constraints.minWidth / childCount; - for (RenderBox child in getChildrenAsList()) { - childWidth = math.max(childWidth, child.getMaxIntrinsicWidth(double.infinity)); - } + double childWidth = constraints.minWidth / childCount; + for (RenderBox child in getChildrenAsList()) { + childWidth = math.max(childWidth, child.getMaxIntrinsicWidth(double.infinity)); } + childWidth = math.min(childWidth, constraints.maxWidth / childCount); RenderBox child = firstChild; while (child != null) { diff --git a/packages/flutter/test/cupertino/segmented_control_test.dart b/packages/flutter/test/cupertino/segmented_control_test.dart index 40c9f23687..f4257e9bbf 100644 --- a/packages/flutter/test/cupertino/segmented_control_test.dart +++ b/packages/flutter/test/cupertino/segmented_control_test.dart @@ -218,8 +218,6 @@ void main() { ), ); - await tester.pumpAndSettle(); - DefaultTextStyle textStyle = tester.widget(find.widgetWithText(DefaultTextStyle, 'Child 1')); IconTheme iconTheme = tester.widget(find.widgetWithIcon(IconTheme, const IconData(1))); @@ -238,63 +236,88 @@ void main() { testWidgets('SegmentedControl is correct when user provides custom colors', (WidgetTester tester) async { - final Map children = {}; - children[0] = const Text('Child 1'); - children[1] = const Icon(IconData(1)); + final Map children = {}; + children[0] = const Text('Child 1'); + children[1] = const Icon(IconData(1)); - int sharedValue = 0; + int sharedValue = 0; - await tester.pumpWidget( - new StatefulBuilder( - builder: (BuildContext context, StateSetter setState) { - return boilerplate( - child: new SegmentedControl( - children: children, - onValueChanged: (int newValue) { - setState(() { - sharedValue = newValue; - }); - }, - groupValue: sharedValue, - unselectedColor: CupertinoColors.lightBackgroundGray, - selectedColor: CupertinoColors.activeGreen, - borderColor: CupertinoColors.black, - pressedColor: const Color(0x638CFC7B), - ), - ); - }, + await tester.pumpWidget( + new StatefulBuilder( + builder: (BuildContext context, StateSetter setState) { + return boilerplate( + child: new SegmentedControl( + children: children, + onValueChanged: (int newValue) { + setState(() { + sharedValue = newValue; + }); + }, + groupValue: sharedValue, + unselectedColor: CupertinoColors.lightBackgroundGray, + selectedColor: CupertinoColors.activeGreen, + borderColor: CupertinoColors.black, + pressedColor: const Color(0x638CFC7B), + ), + ); + }, + ), + ); + + DefaultTextStyle textStyle = tester.widget(find.widgetWithText(DefaultTextStyle, 'Child 1')); + IconTheme iconTheme = tester.widget(find.widgetWithIcon(IconTheme, const IconData(1))); + + expect(getRenderSegmentedControl(tester).borderColor, CupertinoColors.black); + expect(textStyle.style.color, CupertinoColors.lightBackgroundGray); + expect(iconTheme.data.color, CupertinoColors.activeGreen); + expect(getBackgroundColor(tester, 0), CupertinoColors.activeGreen); + expect(getBackgroundColor(tester, 1), CupertinoColors.lightBackgroundGray); + + await tester.tap(find.widgetWithIcon(IconTheme, const IconData(1))); + await tester.pumpAndSettle(); + + textStyle = tester.widget(find.widgetWithText(DefaultTextStyle, 'Child 1')); + iconTheme = tester.widget(find.widgetWithIcon(IconTheme, const IconData(1))); + + expect(textStyle.style.color, CupertinoColors.activeGreen); + expect(iconTheme.data.color, CupertinoColors.lightBackgroundGray); + expect(getBackgroundColor(tester, 0), CupertinoColors.lightBackgroundGray); + expect(getBackgroundColor(tester, 1), CupertinoColors.activeGreen); + + final Offset center = tester.getCenter(find.text('Child 1')); + await tester.startGesture(center); + await tester.pumpAndSettle(); + + expect(getBackgroundColor(tester, 0), const Color(0x638CFC7B)); + expect(getBackgroundColor(tester, 1), CupertinoColors.activeGreen); + }); + + testWidgets('Widgets are centered within segments', (WidgetTester tester) async { + final Map children = {}; + children[0] = const Text('Child 1'); + children[1] = const Text('Child 2'); + + await tester.pumpWidget( + new Directionality( + textDirection: TextDirection.ltr, + child: new Align( + alignment: Alignment.topLeft, + child: new SizedBox( + width: 200.0, + height: 200.0, + child: new SegmentedControl( + children: children, + onValueChanged: (int newValue) {}, + ), ), - ); + ), + ), + ); - await tester.pumpAndSettle(); - - DefaultTextStyle textStyle = tester.widget(find.widgetWithText(DefaultTextStyle, 'Child 1')); - IconTheme iconTheme = tester.widget(find.widgetWithIcon(IconTheme, const IconData(1))); - - expect(getRenderSegmentedControl(tester).borderColor, CupertinoColors.black); - expect(textStyle.style.color, CupertinoColors.lightBackgroundGray); - expect(iconTheme.data.color, CupertinoColors.activeGreen); - expect(getBackgroundColor(tester, 0), CupertinoColors.activeGreen); - expect(getBackgroundColor(tester, 1), CupertinoColors.lightBackgroundGray); - - await tester.tap(find.widgetWithIcon(IconTheme, const IconData(1))); - await tester.pumpAndSettle(); - - textStyle = tester.widget(find.widgetWithText(DefaultTextStyle, 'Child 1')); - iconTheme = tester.widget(find.widgetWithIcon(IconTheme, const IconData(1))); - - expect(textStyle.style.color, CupertinoColors.activeGreen); - expect(iconTheme.data.color, CupertinoColors.lightBackgroundGray); - expect(getBackgroundColor(tester, 0), CupertinoColors.lightBackgroundGray); - expect(getBackgroundColor(tester, 1), CupertinoColors.activeGreen); - - final Offset center = tester.getCenter(find.text('Child 1')); - await tester.startGesture(center); - await tester.pumpAndSettle(); - - expect(getBackgroundColor(tester, 0), const Color(0x638CFC7B)); - expect(getBackgroundColor(tester, 1), CupertinoColors.activeGreen); - }); + // Widgets are centered taking into account 16px of horizontal padding + expect(tester.getCenter(find.text('Child 1')), const Offset(58.0, 100.0)); + expect(tester.getCenter(find.text('Child 2')), const Offset(142.0, 100.0)); + }); testWidgets('Tap calls onValueChanged', (WidgetTester tester) async { final Map children = {}; @@ -510,16 +533,21 @@ void main() { final RenderBox buttonBox = tester.renderObject( find.byKey(const ValueKey('Segmented Control'))); - // Default height of Placeholder is 400.0px, which is greater than heights - // of other child widgets. expect(buttonBox.size.height, 400.0); }); - testWidgets('Width of each child widget is the same', (WidgetTester tester) async { + testWidgets('Width of each segmented control segment is determined by widest widget', + (WidgetTester tester) async { final Map children = {}; - children[0] = new Container(); - children[1] = const Placeholder(); - children[2] = new Container(); + children[0] = new Container( + constraints: const BoxConstraints.tightFor(width: 50.0), + ); + children[1] = new Container( + constraints: const BoxConstraints.tightFor(width: 100.0), + ); + children[2] = new Container( + constraints: const BoxConstraints.tightFor(width: 200.0), + ); await tester.pumpWidget( new StatefulBuilder( @@ -542,6 +570,8 @@ void main() { // to each child equally. final double childWidth = (segmentedControl.size.width - 32.0) / 3; + expect(childWidth, 200.0); + expect(childWidth, getRenderSegmentedControl(tester).getChildrenAsList()[0].parentData.surroundingRect.width); expect(childWidth, @@ -748,8 +778,8 @@ void main() { testWidgets('Non-centered taps work on smaller widgets', (WidgetTester tester) async { final Map children = {}; - children[0] = const Text('A'); - children[1] = const Text('B'); + children[0] = const Text('Child 1'); + children[1] = const Text('Child 2'); int sharedValue = 1; @@ -775,10 +805,15 @@ void main() { expect(sharedValue, 1); final double childWidth = getRenderSegmentedControl(tester).firstChild.size.width; - final Offset centerOfSegmentedControl = tester.getCenter(find.text('A')); + final Offset centerOfSegmentedControl = tester.getCenter(find.text('Child 1')); // Tap just inside segment bounds - await tester.tapAt(new Offset(childWidth - 10.0, centerOfSegmentedControl.dy)); + await tester.tapAt( + new Offset( + centerOfSegmentedControl.dx + (childWidth / 2) - 10.0, + centerOfSegmentedControl.dy, + ), + ); expect(sharedValue, 0); }); @@ -1257,11 +1292,14 @@ void main() { child: new StatefulBuilder( builder: (BuildContext context, StateSetter setState) { return boilerplate( - child: new SegmentedControl( - key: const ValueKey('Segmented Control'), - children: children, - onValueChanged: (int newValue) {}, - groupValue: currentValue, + child: new SizedBox( + width: 800.0, + child: new SegmentedControl( + key: const ValueKey('Segmented Control'), + children: children, + onValueChanged: (int newValue) {}, + groupValue: currentValue, + ), ), ); }, @@ -1273,7 +1311,7 @@ void main() { find.byType(RepaintBoundary), matchesGoldenFile('segmented_control_test.0.0.png'), ); - }, skip: !Platform.isLinux); + }, skip: !Platform.isMacOS); testWidgets('Golden Test Pressed State', (WidgetTester tester) async { final Map children = {}; @@ -1288,11 +1326,14 @@ void main() { child: new StatefulBuilder( builder: (BuildContext context, StateSetter setState) { return boilerplate( - child: new SegmentedControl( - key: const ValueKey('Segmented Control'), - children: children, - onValueChanged: (int newValue) {}, - groupValue: currentValue, + child: new SizedBox( + width: 800.0, + child: new SegmentedControl( + key: const ValueKey('Segmented Control'), + children: children, + onValueChanged: (int newValue) {}, + groupValue: currentValue, + ), ), ); }, @@ -1308,5 +1349,5 @@ void main() { find.byType(RepaintBoundary), matchesGoldenFile('segmented_control_test.1.0.png'), ); - }, skip: !Platform.isLinux); + }, skip: !Platform.isMacOS); }