From 952ace627f4c41469fe528506c53014bf3dbbd62 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 11 Apr 2023 11:21:13 -0700 Subject: [PATCH] BottomAppBar: Fix doubled layers of color and shadow (#123294) Fixes BottomAppBar with translucent colors. --- .../lib/src/material/bottom_app_bar.dart | 14 ++-- .../test/material/bottom_app_bar_test.dart | 79 +++++++++++++++++-- .../material/bottom_app_bar_theme_test.dart | 38 ++++----- 3 files changed, 95 insertions(+), 36 deletions(-) diff --git a/packages/flutter/lib/src/material/bottom_app_bar.dart b/packages/flutter/lib/src/material/bottom_app_bar.dart index 56297c2862..d5b89dd699 100644 --- a/packages/flutter/lib/src/material/bottom_app_bar.dart +++ b/packages/flutter/lib/src/material/bottom_app_bar.dart @@ -57,7 +57,7 @@ import 'theme.dart'; class BottomAppBar extends StatefulWidget { /// Creates a bottom application bar. /// - /// The [clipBehavior] argument defaults to [Clip.none] and must not be null. + /// The [clipBehavior] argument defaults to [Clip.none]. /// Additionally, [elevation] must be non-negative. /// /// If [color], [elevation], or [shape] are null, their [BottomAppBarTheme] values will be used. @@ -118,7 +118,7 @@ class BottomAppBar extends StatefulWidget { /// {@macro flutter.material.Material.clipBehavior} /// - /// Defaults to [Clip.none], and must not be null. + /// Defaults to [Clip.none]. final Clip clipBehavior; /// The margin between the [FloatingActionButton] and the [BottomAppBar]'s @@ -191,7 +191,9 @@ class _BottomAppBarState extends State { final double? height = widget.height ?? babTheme.height ?? defaults.height; final Color color = widget.color ?? babTheme.color ?? defaults.color!; final Color surfaceTintColor = widget.surfaceTintColor ?? babTheme.surfaceTintColor ?? defaults.surfaceTintColor!; - final Color effectiveColor = isMaterial3 ? color : ElevationOverlay.applyOverlay(context, color, elevation); + final Color effectiveColor = isMaterial3 + ? ElevationOverlay.applySurfaceTint(color, surfaceTintColor, elevation) + : ElevationOverlay.applyOverlay(context, color, elevation); final Color shadowColor = widget.shadowColor ?? babTheme.shadowColor ?? defaults.shadowColor!; final Widget child = SizedBox( @@ -204,11 +206,7 @@ class _BottomAppBarState extends State { final Material material = Material( key: materialKey, - type: isMaterial3 ? MaterialType.canvas : MaterialType.transparency, - elevation: elevation, - color: isMaterial3 ? effectiveColor : null, - surfaceTintColor: surfaceTintColor, - shadowColor: shadowColor, + type: MaterialType.transparency, child: SafeArea(child: child), ); diff --git a/packages/flutter/test/material/bottom_app_bar_test.dart b/packages/flutter/test/material/bottom_app_bar_test.dart index 6a74ce129d..6e5075b535 100644 --- a/packages/flutter/test/material/bottom_app_bar_test.dart +++ b/packages/flutter/test/material/bottom_app_bar_test.dart @@ -11,7 +11,74 @@ import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter_test/flutter_test.dart'; +import '../rendering/mock_canvas.dart'; + void main() { + testWidgets('shadow effect is not doubled', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/123064 + debugDisableShadows = false; + + const double elevation = 1; + const Color shadowColor = Colors.black; + + await tester.pumpWidget( + MaterialApp( + theme: ThemeData.light(useMaterial3: true), + home: const Scaffold( + bottomNavigationBar: BottomAppBar( + elevation: elevation, + shadowColor: shadowColor, + ), + ), + ), + ); + + final Finder finder = find.byType(BottomAppBar); + expect(finder, paints..shadow(color: shadowColor, elevation: elevation)); + expect(finder, paintsExactlyCountTimes(#drawShadow, 1)); + + debugDisableShadows = true; + }); + + testWidgets('only one layer with `color` is painted', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/122667 + const Color bottomAppBarColor = Colors.black45; + + await tester.pumpWidget( + MaterialApp( + theme: ThemeData.light(useMaterial3: true), + home: const Scaffold( + bottomNavigationBar: BottomAppBar( + color: bottomAppBarColor, + + // Avoid getting a surface tint color, to keep the color check below simple + elevation: 0, + ), + ), + ), + ); + + // There should be just one color layer, and with the specified color. + final Finder finder = find.descendant( + of: find.byType(BottomAppBar), + matching: find.byWidgetPredicate((Widget widget) { + // A color layer is probably a [PhysicalShape] or [PhysicalModel], + // either used directly or backing a [Material] (one without + // [MaterialType.transparency]). + return widget is PhysicalShape || widget is PhysicalModel; + }), + ); + final Widget widget = tester.widgetList(finder).single; + if (widget is PhysicalShape) { + expect(widget.color, bottomAppBarColor); + } else if (widget is PhysicalModel) { + expect(widget.color, bottomAppBarColor); + } else { + // Should be unreachable: compare with the finder. + assert(false); + } + }); + testWidgets('no overlap with floating action button', (WidgetTester tester) async { await tester.pumpWidget( const MaterialApp( @@ -218,6 +285,7 @@ void main() { ), bottomNavigationBar: BottomAppBar( color: Color(0xff0000ff), + surfaceTintColor: Colors.transparent, ), ); }, @@ -225,12 +293,10 @@ void main() { ), ); - final PhysicalShape physicalShape = - tester.widget(find.byType(PhysicalShape).at(0)); - final Material material = tester.widget(find.byType(Material).at(1)); + final PhysicalShape physicalShape = tester.widget( + find.descendant(of: find.byType(BottomAppBar), matching: find.byType(PhysicalShape))); expect(physicalShape.color, const Color(0xff0000ff)); - expect(material.color, const Color(0xff0000ff)); }); testWidgets('Shadow color is transparent in Material 3', (WidgetTester tester) async { @@ -249,9 +315,10 @@ void main() { ) ); - final Material material = tester.widget(find.byType(Material).at(1)); + final PhysicalShape physicalShape = tester.widget( + find.descendant(of: find.byType(BottomAppBar), matching: find.byType(PhysicalShape))); - expect(material.shadowColor, Colors.transparent); + expect(physicalShape.shadowColor, Colors.transparent); }); testWidgets('dark theme applies an elevation overlay color', (WidgetTester tester) async { diff --git a/packages/flutter/test/material/bottom_app_bar_theme_test.dart b/packages/flutter/test/material/bottom_app_bar_theme_test.dart index 7c922d5548..e3a811f5a5 100644 --- a/packages/flutter/test/material/bottom_app_bar_theme_test.dart +++ b/packages/flutter/test/material/bottom_app_bar_theme_test.dart @@ -114,15 +114,6 @@ void main() { }); group('Material 3 tests', () { - Material getBabRenderObject(WidgetTester tester) { - return tester.widget( - find.descendant( - of: find.byType(BottomAppBar), - matching: find.byType(Material), - ), - ); - } - testWidgets('BAB theme overrides color - M3', (WidgetTester tester) async { const Color themedColor = Colors.black87; const BottomAppBarTheme theme = BottomAppBarTheme( @@ -147,7 +138,7 @@ void main() { bottomAppBarTheme: theme, bottomAppBarColor: themeColor ), - home: const Scaffold(body: BottomAppBar(color: babColor)), + home: const Scaffold(body: BottomAppBar(color: babColor, surfaceTintColor: Colors.transparent)), )); final PhysicalShape widget = _getBabRenderObject(tester); @@ -165,7 +156,7 @@ void main() { bottomAppBarTheme: theme, bottomAppBarColor: themeColor ), - home: const Scaffold(body: BottomAppBar()), + home: const Scaffold(body: BottomAppBar(surfaceTintColor: Colors.transparent)), )); final PhysicalShape widget = _getBabRenderObject(tester); @@ -176,7 +167,7 @@ void main() { final ThemeData theme = ThemeData(useMaterial3: true); await tester.pumpWidget(MaterialApp( theme: theme, - home: const Scaffold(body: BottomAppBar()), + home: const Scaffold(body: BottomAppBar(surfaceTintColor: Colors.transparent)), )); final PhysicalShape widget = _getBabRenderObject(tester); @@ -186,14 +177,15 @@ void main() { }); testWidgets('BAB theme overrides surfaceTintColor - M3', (WidgetTester tester) async { + const Color color = Colors.blue; // base color that the surface tint will be applied to const Color babThemeSurfaceTintColor = Colors.black87; const BottomAppBarTheme theme = BottomAppBarTheme( - surfaceTintColor: babThemeSurfaceTintColor, elevation: 0 + color: color, surfaceTintColor: babThemeSurfaceTintColor, elevation: 0, ); await tester.pumpWidget(_withTheme(theme, true)); - final Material widget = getBabRenderObject(tester); - expect(widget.surfaceTintColor, babThemeSurfaceTintColor); + final PhysicalShape widget = _getBabRenderObject(tester); + expect(widget.color, ElevationOverlay.applySurfaceTint(color, babThemeSurfaceTintColor, 0)); }); testWidgets('BAB theme overrides shadowColor - M3', (WidgetTester tester) async { @@ -203,11 +195,12 @@ void main() { ); await tester.pumpWidget(_withTheme(theme, true)); - final Material widget = getBabRenderObject(tester); + final PhysicalShape widget = _getBabRenderObject(tester); expect(widget.shadowColor, babThemeShadowColor); }); testWidgets('BAB surfaceTintColor - Widget - M3', (WidgetTester tester) async { + const Color color = Colors.white10; // base color that the surface tint will be applied to const Color themeSurfaceTintColor = Colors.white10; const Color babThemeSurfaceTintColor = Colors.black87; const Color babSurfaceTintColor = Colors.pink; @@ -221,15 +214,16 @@ void main() { bottomAppBarColor: themeSurfaceTintColor ), home: const Scaffold( - body: BottomAppBar(surfaceTintColor: babSurfaceTintColor) + body: BottomAppBar(color: color, surfaceTintColor: babSurfaceTintColor) ), )); - final Material widget = getBabRenderObject(tester); - expect(widget.surfaceTintColor, babSurfaceTintColor); + final PhysicalShape widget = _getBabRenderObject(tester); + expect(widget.color, ElevationOverlay.applySurfaceTint(color, babSurfaceTintColor, 3.0)); }); testWidgets('BAB surfaceTintColor - BabTheme - M3', (WidgetTester tester) async { + const Color color = Colors.blue; // base color that the surface tint will be applied to const Color themeColor = Colors.white10; const Color babThemeColor = Colors.black87; const BottomAppBarTheme theme = BottomAppBarTheme( @@ -242,11 +236,11 @@ void main() { bottomAppBarTheme: theme, bottomAppBarColor: themeColor ), - home: const Scaffold(body: BottomAppBar()), + home: const Scaffold(body: BottomAppBar(color: color)), )); - final Material widget = getBabRenderObject(tester); - expect(widget.surfaceTintColor, babThemeColor); + final PhysicalShape widget = _getBabRenderObject(tester); + expect(widget.color, ElevationOverlay.applySurfaceTint(color, babThemeColor, 3.0)); }); }); }