From eb5ab2cb5cd309986cdf7ed579fae1c5597bdcc5 Mon Sep 17 00:00:00 2001 From: Hans Muller Date: Tue, 15 Sep 2020 10:57:04 -0700 Subject: [PATCH] Buttons animate elevation changes before changing background color (#65662) --- .../lib/src/material/button_style_button.dart | 46 +++++++++++++- .../lib/src/material/elevated_button.dart | 5 +- .../test/material/elevated_button_test.dart | 61 +++++++++++++++++-- 3 files changed, 102 insertions(+), 10 deletions(-) diff --git a/packages/flutter/lib/src/material/button_style_button.dart b/packages/flutter/lib/src/material/button_style_button.dart index 4095c2ce175..853cb027c6c 100644 --- a/packages/flutter/lib/src/material/button_style_button.dart +++ b/packages/flutter/lib/src/material/button_style_button.dart @@ -179,7 +179,10 @@ abstract class ButtonStyleButton extends StatefulWidget { /// * [TextButton], a simple button without a shadow. /// * [ElevatedButton], a filled button whose material elevates when pressed. /// * [OutlinedButton], similar to [TextButton], but with an outline. -class _ButtonStyleState extends State { +class _ButtonStyleState extends State with TickerProviderStateMixin { + AnimationController _controller; + double _elevation; + Color _backgroundColor; final Set _states = {}; bool get _hovered => _states.contains(MaterialState.hovered); @@ -221,6 +224,12 @@ class _ButtonStyleState extends State { _updateState(MaterialState.disabled, !widget.enabled); } + @override + void dispose() { + _controller?.dispose(); + super.dispose(); + } + @override void didUpdateWidget(ButtonStyleButton oldWidget) { super.didUpdateWidget(oldWidget); @@ -254,11 +263,11 @@ class _ButtonStyleState extends State { ); } + final double resolvedElevation = resolve((ButtonStyle style) => style?.elevation); final TextStyle resolvedTextStyle = resolve((ButtonStyle style) => style?.textStyle); - final Color resolvedBackgroundColor = resolve((ButtonStyle style) => style?.backgroundColor); + Color resolvedBackgroundColor = resolve((ButtonStyle style) => style?.backgroundColor); final Color resolvedForegroundColor = resolve((ButtonStyle style) => style?.foregroundColor); final Color resolvedShadowColor = resolve((ButtonStyle style) => style?.shadowColor); - final double resolvedElevation = resolve((ButtonStyle style) => style?.elevation); final EdgeInsetsGeometry resolvedPadding = resolve((ButtonStyle style) => style?.padding); final Size resolvedMinimumSize = resolve((ButtonStyle style) => style?.minimumSize); final BorderSide resolvedSide = resolve((ButtonStyle style) => style?.side); @@ -292,6 +301,37 @@ class _ButtonStyleState extends State { ), ).clamp(EdgeInsets.zero, EdgeInsetsGeometry.infinity); + // If an opaque button's background is becoming translucent while its + // elevation is changing, change the elevation first. Material implicitly + // animates its elevation but not its color. SKIA renders non-zero + // elevations as a shadow colored fill behind the Material's background. + if (resolvedAnimationDuration > Duration.zero + && _elevation != null + && _backgroundColor != null + && _elevation != resolvedElevation + && _backgroundColor.value != resolvedBackgroundColor.value + && _backgroundColor.opacity == 1 + && resolvedBackgroundColor.opacity < 1 + && resolvedElevation == 0) { + if (_controller?.duration != resolvedAnimationDuration) { + _controller?.dispose(); + _controller = AnimationController( + duration: resolvedAnimationDuration, + vsync: this, + ) + ..addStatusListener((AnimationStatus status) { + if (status == AnimationStatus.completed) { + setState(() { }); // Rebuild with the final background color. + } + }); + } + resolvedBackgroundColor = _backgroundColor; // Defer changing the background color. + _controller.value = 0; + _controller.forward(); + } + _elevation = resolvedElevation; + _backgroundColor = resolvedBackgroundColor; + final Widget result = ConstrainedBox( constraints: effectiveConstraints, child: Material( diff --git a/packages/flutter/lib/src/material/elevated_button.dart b/packages/flutter/lib/src/material/elevated_button.dart index e55b80a5762..837e24bd5cb 100644 --- a/packages/flutter/lib/src/material/elevated_button.dart +++ b/packages/flutter/lib/src/material/elevated_button.dart @@ -222,8 +222,9 @@ class ElevatedButton extends ButtonStyleButton { /// * `shadowColor` - Theme.shadowColor /// * `elevation` /// * disabled - 0 - /// * hovered or focused - 2 - /// * pressed - 6 + /// * default - 2 + /// * hovered or focused - 4 + /// * pressed - 8 /// * `padding` /// * textScaleFactor <= 1 - horizontal(16) /// * `1 < textScaleFactor <= 2` - lerp(horizontal(16), horizontal(8)) diff --git a/packages/flutter/test/material/elevated_button_test.dart b/packages/flutter/test/material/elevated_button_test.dart index 7d7710970ab..1a9a40086c4 100644 --- a/packages/flutter/test/material/elevated_button_test.dart +++ b/packages/flutter/test/material/elevated_button_test.dart @@ -14,11 +14,6 @@ import '../widgets/semantics_tester.dart'; void main() { testWidgets('ElevatedButton defaults', (WidgetTester tester) async { - final Finder rawButtonMaterial = find.descendant( - of: find.byType(ElevatedButton), - matching: find.byType(Material), - ); - const ColorScheme colorScheme = ColorScheme.light(); // Enabled ElevatedButton @@ -34,6 +29,12 @@ void main() { ), ); + final Finder rawButtonMaterial = find.descendant( + of: find.byType(ElevatedButton), + matching: find.byType(Material), + ); + + Material material = tester.widget(rawButtonMaterial); expect(material.animationDuration, const Duration(milliseconds: 200)); expect(material.borderOnForeground, true); @@ -88,6 +89,9 @@ void main() { ), ); + // Finish the elevation animation, final background color change. + await tester.pumpAndSettle(); + material = tester.widget(rawButtonMaterial); expect(material.animationDuration, const Duration(milliseconds: 200)); expect(material.borderOnForeground, true); @@ -926,6 +930,53 @@ void main() { ); expect(paddingWidget.padding, const EdgeInsets.all(22)); }); + + testWidgets('Elevated buttons animate elevation before color on disable', (WidgetTester tester) async { + // This is a regression test for https://github.com/flutter/flutter/issues/387 + + const ColorScheme colorScheme = ColorScheme.light(); + final Color backgroundColor = colorScheme.primary; + final Color disabledBackgroundColor = colorScheme.onSurface.withOpacity(0.12); + + Widget buildFrame({ bool enabled }) { + return MaterialApp( + theme: ThemeData.from(colorScheme: colorScheme), + home: Center( + child: ElevatedButton( + onPressed: enabled ? () { } : null, + child: const Text('button'), + ), + ), + ); + } + + PhysicalShape physicalShape() { + return tester.widget( + find.descendant( + of: find.byType(ElevatedButton), + matching: find.byType(PhysicalShape), + ), + ); + } + + // Default elevation is 2, background color is primary. + await tester.pumpWidget(buildFrame(enabled: true)); + expect(physicalShape().elevation, 2); + expect(physicalShape().color, backgroundColor); + + // Disabled elevation animates to 0 over 200ms, THEN the background + // color changes to onSurface.withOpacity(0.12) + await tester.pumpWidget(buildFrame(enabled: false)); + await tester.pump(const Duration(milliseconds: 50)); + expect(physicalShape().elevation, lessThan(2)); + expect(physicalShape().color, backgroundColor); + await tester.pump(const Duration(milliseconds: 150)); + expect(physicalShape().elevation, 0); + expect(physicalShape().color, backgroundColor); + await tester.pumpAndSettle(); + expect(physicalShape().elevation, 0); + expect(physicalShape().color, disabledBackgroundColor); + }); } TextStyle _iconStyle(WidgetTester tester, IconData icon) {