From a9c05a72f75c34d4bf62a2da94c408ab380d62f9 Mon Sep 17 00:00:00 2001 From: Hans Muller Date: Wed, 6 Feb 2019 15:53:56 -0800 Subject: [PATCH] Update OutlineButton default border width and highlight elevation (#27575) These changes are **backwards incompatible**. Tests that verify OutlineButton visuals, for example golden image tests, will need to be updated. --- .../lib/src/material/button_theme.dart | 7 +- .../lib/src/material/outline_button.dart | 90 ++++++++++--------- .../test/material/outline_button_test.dart | 10 ++- .../test/material/theme_defaults_test.dart | 33 ++++++- 4 files changed, 91 insertions(+), 49 deletions(-) diff --git a/packages/flutter/lib/src/material/button_theme.dart b/packages/flutter/lib/src/material/button_theme.dart index 4fe12b5545..11b8ab70fe 100644 --- a/packages/flutter/lib/src/material/button_theme.dart +++ b/packages/flutter/lib/src/material/button_theme.dart @@ -632,16 +632,15 @@ class ButtonThemeData extends Diagnosticable { /// /// Returns the button's [MaterialButton.highlightElevation] if it is non-null. /// - /// If button is a [FlatButton] then the highlight elevation is 0.0, if it's - /// a [OutlineButton] then the highlight elevation is 2.0, otherise the - /// highlight elevation is 8.0. + /// If button is a [FlatButton] or an [OutlineButton] then the highlight + /// elevation is 0.0, otherise the highlight elevation is 8.0. double getHighlightElevation(MaterialButton button) { if (button.highlightElevation != null) return button.highlightElevation; if (button is FlatButton) return 0.0; if (button is OutlineButton) - return 2.0; + return 0.0; return 8.0; } diff --git a/packages/flutter/lib/src/material/outline_button.dart b/packages/flutter/lib/src/material/outline_button.dart index 4a0b6c1f0e..90d21da70f 100644 --- a/packages/flutter/lib/src/material/outline_button.dart +++ b/packages/flutter/lib/src/material/outline_button.dart @@ -12,28 +12,31 @@ import 'raised_button.dart'; import 'theme.dart'; // The total time to make the button's fill color opaque and change -// its elevation. +// its elevation. Only applies when highlightElevation > 0.0. const Duration _kPressDuration = Duration(milliseconds: 150); // Half of _kPressDuration: just the time to change the button's -// elevation. +// elevation. Only applies when highlightElevation > 0.0. const Duration _kElevationDuration = Duration(milliseconds: 75); -/// A cross between [RaisedButton] and [FlatButton]: a bordered button whose -/// elevation increases and whose background becomes opaque when the button -/// is pressed. +/// Similar to a [FlatButton] with a thin grey rounded rectangle border. /// -/// An outline button's elevation is initially 0.0 and its background [color] -/// is transparent. When the button is pressed its background becomes opaque -/// and then its elevation increases to [highlightElevation]. -/// -/// The outline button has a border whose shape is defined by [shape] -/// and whose appearance is defined by [borderSide], [disabledBorderColor], -/// and [highlightedBorderColor]. +/// The outline button's border shape is defined by [shape] +/// and its appearance is defined by [borderSide], [disabledBorderColor], +/// and [highlightedBorderColor]. By default the border is a one pixel +/// wide grey rounded rectangle that does not change when the button is +/// pressed or disabled. By default the button's background is transparent. /// /// If the [onPressed] callback is null, then the button will be disabled and by /// default will resemble a flat button in the [disabledColor]. /// +/// The button's [highlightElevation], which defines the size of the +/// drop shadow when the button is pressed, is 0.0 (no shadow) by default. +/// If [highlightElevation] is given a value greater than 0.0 then the button +/// becomes a cross between [RaisedButton] and [FlatButton]: a bordered +/// button whose elevation increases and whose background becomes opaque +/// when the button is pressed. +/// /// If you want an ink-splash effect for taps, but don't want to use a button, /// consider using [InkWell] directly. /// @@ -50,10 +53,10 @@ const Duration _kElevationDuration = Duration(milliseconds: 75); /// * [InkWell], which implements the ink splash part of a flat button. /// * class OutlineButton extends MaterialButton { - /// Create a filled button. + /// Create an outline button. /// - /// The [highlightElevation], [borderWidth], and [clipBehavior] - /// arguments must not be null. + /// The [highlightElevation] argument must be null or a positive value + /// and the [clipBehavior] argument must not be null. const OutlineButton({ Key key, @required VoidCallback onPressed, @@ -94,8 +97,8 @@ class OutlineButton extends MaterialButton { /// The icon and label are arranged in a row and padded by 12 logical pixels /// at the start, and 16 at the end, with an 8 pixel gap in between. /// - /// The [highlightElevation], [icon], [label], and [clipBehavior] must not be - /// null. + /// The [highlightElevation] argument must be null or a positive value. The + /// [icon], [label], and [clipBehavior] arguments must not be null. factory OutlineButton.icon({ Key key, @required VoidCallback onPressed, @@ -118,15 +121,14 @@ class OutlineButton extends MaterialButton { /// The outline border's color when the button is [enabled] and pressed. /// - /// If null this value defaults to the theme's primary color, - /// [ThemeData.primaryColor]. + /// By default the border's color does not change when the button + /// is pressed. final Color highlightedBorderColor; /// The outline border's color when the button is not [enabled]. /// - /// If null this value defaults to a very light shade of grey for light - /// themes (see [ThemeData.brightness]), and a very dark shade of grey for - /// dark themes. + /// By default the outline border's color does not change when the + /// button is disabled. final Color disabledBorderColor; /// Defines the color of the border when the button is enabled but not @@ -136,7 +138,7 @@ class OutlineButton extends MaterialButton { /// an outline is not drawn. /// /// If null the default border's style is [BorderStyle.solid], its - /// [BorderSide.width] is 2.0, and its color is a light shade of grey. + /// [BorderSide.width] is 1.0, and its color is a light shade of grey. final BorderSide borderSide; @override @@ -181,10 +183,10 @@ class OutlineButton extends MaterialButton { } } -// The type of of OutlineButtons created with [OutlineButton.icon]. +// The type of of OutlineButtons created with OutlineButton.icon. // -// This class only exists to give RaisedButtons created with [RaisedButton.icon] -// a distinct class for the sake of [ButtonTheme]. It can not be instantiated. +// This class only exists to give OutlineButtons created with OutlineButton.icon +// a distinct class for the sake of ButtonTheme. It can not be instantiated. class _OutlineButtonWithIcon extends OutlineButton with MaterialButtonWithIconMixin { _OutlineButtonWithIcon({ Key key, @@ -291,12 +293,13 @@ class _OutlineButtonState extends State<_OutlineButton> with SingleTickerProvide void initState() { super.initState(); - // The Material widget animates its shape (which includes the outline - // border) and elevation over _kElevationDuration. When pressed, the - // button makes its fill color opaque white first, and then sets - // its highlightElevation. We can't change the elevation while the - // button's fill is translucent, because the shadow fills the interior - // of the button. + // When highlightElevation > 0.0, the Material widget animates its + // shape (which includes the outline border) and elevation over + // _kElevationDuration. When pressed, the button makes its fill + // color opaque white first, and then sets its + // highlightElevation. We can't change the elevation while the + // button's fill is translucent, because the shadow fills the + // interior of the button. _controller = AnimationController( duration: _kPressDuration, @@ -343,6 +346,8 @@ class _OutlineButtonState extends State<_OutlineButton> with SingleTickerProvide } Color _getFillColor() { + if (widget.highlightElevation == null || widget.highlightElevation == 0.0) + return Colors.transparent; final Color color = widget.color ?? Theme.of(context).canvasColor; final Tween colorTween = ColorTween( begin: color.withAlpha(0x00), @@ -352,28 +357,27 @@ class _OutlineButtonState extends State<_OutlineButton> with SingleTickerProvide } BorderSide _getOutline() { - final bool isDark = widget.brightness == Brightness.dark; if (widget.borderSide?.style == BorderStyle.none) return widget.borderSide; - final Color color = widget.enabled - ? (_pressed - ? widget.highlightedBorderColor - : (widget.borderSide?.color ?? - (isDark ? Colors.grey[600] : Colors.grey[200]))) - : (widget.disabledBorderColor ?? - (isDark ? Colors.grey[800] : Colors.grey[100])); + final Color specifiedColor = widget.enabled + ? (_pressed ? widget.highlightedBorderColor : null) ?? widget.borderSide?.color + : widget.disabledBorderColor; + + final Color themeColor = Theme.of(context).colorScheme.onSurface.withOpacity(0.12); return BorderSide( - color: color, - width: widget.borderSide?.width ?? 2.0, + color: specifiedColor ?? themeColor, + width: widget.borderSide?.width ?? 1.0, ); } double _getHighlightElevation() { + if (widget.highlightElevation == null || widget.highlightElevation == 0.0) + return 0.0; return Tween( begin: 0.0, - end: widget.highlightElevation ?? 2.0, + end: widget.highlightElevation, ).evaluate(_elevationAnimation); } diff --git a/packages/flutter/test/material/outline_button_test.dart b/packages/flutter/test/material/outline_button_test.dart index e977919217..e15d6f7206 100644 --- a/packages/flutter/test/material/outline_button_test.dart +++ b/packages/flutter/test/material/outline_button_test.dart @@ -81,6 +81,10 @@ void main() { shape: const RoundedRectangleBorder(), // default border radius is 0 clipBehavior: Clip.antiAlias, color: fillColor, + // Causes the button to be filled with the theme's canvasColor + // instead of Colors.transparent before the button material's + // elevation is animated to 2.0. + highlightElevation: 2.0, highlightedBorderColor: highlightedBorderColor, disabledBorderColor: disabledBorderColor, borderSide: const BorderSide( @@ -108,7 +112,7 @@ void main() { // Expect that the button is disabled and painted with the disabled border color. expect(tester.widget(outlineButton).enabled, false); expect( - outlineButton, //find.byType(OutlineButton), + outlineButton, paints ..clipPath(pathMatcher: coversSameAreaAs(clipPath, areaToCompare: clipRect.inflate(10.0))) ..path(color: disabledBorderColor, strokeWidth: borderWidth)); @@ -323,6 +327,10 @@ void main() { body: Center( child: OutlineButton( onPressed: () {}, + // Causes the button to be filled with the theme's canvasColor + // instead of Colors.transparent before the button material's + // elevation is animated to 2.0. + highlightElevation: 2.0, child: const Text('Hello'), ), ), diff --git a/packages/flutter/test/material/theme_defaults_test.dart b/packages/flutter/test/material/theme_defaults_test.dart index 32a7610ec5..d04b4addb0 100644 --- a/packages/flutter/test/material/theme_defaults_test.dart +++ b/packages/flutter/test/material/theme_defaults_test.dart @@ -128,6 +128,37 @@ void main() { }); group('OutlineButton', () { + testWidgets('theme: ThemeData.light(), enabled: true, highlightElevation: 2.0', (WidgetTester tester) async { + await tester.pumpWidget( + MaterialApp( + theme: ThemeData.light(), + home: Center( + child: OutlineButton( + onPressed: () { }, // button.enabled == true + // Causes the button to be filled with the theme's canvasColor + // instead of Colors.transparent before the button material's + // elevation is animated to 2.0. + highlightElevation: 2.0, + child: const Text('button'), + ) + ), + ), + ); + + final RawMaterialButton raw = tester.widget(find.byType(RawMaterialButton)); + expect(raw.textStyle.color, const Color(0xdd000000)); + expect(raw.fillColor, const Color(0x00fafafa)); + expect(raw.highlightColor, const Color(0x29000000)); // Was Color(0x66bcbcbc) + expect(raw.splashColor, const Color(0x1f000000)); // Was Color(0x66c8c8c8) + expect(raw.elevation, 0.0); + expect(raw.highlightElevation, 0.0); + expect(raw.disabledElevation, 0.0); + expect(raw.constraints, defaultButtonConstraints); + expect(raw.padding, defaultButtonPadding); + // animationDuration can't be configed by the theme/constructor + expect(raw.materialTapTargetSize, MaterialTapTargetSize.padded); + }); + testWidgets('theme: ThemeData.light(), enabled: true', (WidgetTester tester) async { await tester.pumpWidget( MaterialApp( @@ -143,7 +174,7 @@ void main() { final RawMaterialButton raw = tester.widget(find.byType(RawMaterialButton)); expect(raw.textStyle.color, const Color(0xdd000000)); - expect(raw.fillColor, const Color(0x00fafafa)); + expect(raw.fillColor, Colors.transparent); expect(raw.highlightColor, const Color(0x29000000)); // Was Color(0x66bcbcbc) expect(raw.splashColor, const Color(0x1f000000)); // Was Color(0x66c8c8c8) expect(raw.elevation, 0.0);