Unverified Commit 952ace62 authored by Chris Bobbe's avatar Chris Bobbe Committed by GitHub

BottomAppBar: Fix doubled layers of color and shadow (#123294)

Fixes BottomAppBar with translucent colors.
parent c9f23b0b
...@@ -57,7 +57,7 @@ import 'theme.dart'; ...@@ -57,7 +57,7 @@ import 'theme.dart';
class BottomAppBar extends StatefulWidget { class BottomAppBar extends StatefulWidget {
/// Creates a bottom application bar. /// 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. /// Additionally, [elevation] must be non-negative.
/// ///
/// If [color], [elevation], or [shape] are null, their [BottomAppBarTheme] values will be used. /// If [color], [elevation], or [shape] are null, their [BottomAppBarTheme] values will be used.
...@@ -118,7 +118,7 @@ class BottomAppBar extends StatefulWidget { ...@@ -118,7 +118,7 @@ class BottomAppBar extends StatefulWidget {
/// {@macro flutter.material.Material.clipBehavior} /// {@macro flutter.material.Material.clipBehavior}
/// ///
/// Defaults to [Clip.none], and must not be null. /// Defaults to [Clip.none].
final Clip clipBehavior; final Clip clipBehavior;
/// The margin between the [FloatingActionButton] and the [BottomAppBar]'s /// The margin between the [FloatingActionButton] and the [BottomAppBar]'s
...@@ -191,7 +191,9 @@ class _BottomAppBarState extends State<BottomAppBar> { ...@@ -191,7 +191,9 @@ class _BottomAppBarState extends State<BottomAppBar> {
final double? height = widget.height ?? babTheme.height ?? defaults.height; final double? height = widget.height ?? babTheme.height ?? defaults.height;
final Color color = widget.color ?? babTheme.color ?? defaults.color!; final Color color = widget.color ?? babTheme.color ?? defaults.color!;
final Color surfaceTintColor = widget.surfaceTintColor ?? babTheme.surfaceTintColor ?? defaults.surfaceTintColor!; 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 Color shadowColor = widget.shadowColor ?? babTheme.shadowColor ?? defaults.shadowColor!;
final Widget child = SizedBox( final Widget child = SizedBox(
...@@ -204,11 +206,7 @@ class _BottomAppBarState extends State<BottomAppBar> { ...@@ -204,11 +206,7 @@ class _BottomAppBarState extends State<BottomAppBar> {
final Material material = Material( final Material material = Material(
key: materialKey, key: materialKey,
type: isMaterial3 ? MaterialType.canvas : MaterialType.transparency, type: MaterialType.transparency,
elevation: elevation,
color: isMaterial3 ? effectiveColor : null,
surfaceTintColor: surfaceTintColor,
shadowColor: shadowColor,
child: SafeArea(child: child), child: SafeArea(child: child),
); );
......
...@@ -11,7 +11,74 @@ import 'package:flutter/material.dart'; ...@@ -11,7 +11,74 @@ import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import '../rendering/mock_canvas.dart';
void main() { 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 { testWidgets('no overlap with floating action button', (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
const MaterialApp( const MaterialApp(
...@@ -218,6 +285,7 @@ void main() { ...@@ -218,6 +285,7 @@ void main() {
), ),
bottomNavigationBar: BottomAppBar( bottomNavigationBar: BottomAppBar(
color: Color(0xff0000ff), color: Color(0xff0000ff),
surfaceTintColor: Colors.transparent,
), ),
); );
}, },
...@@ -225,12 +293,10 @@ void main() { ...@@ -225,12 +293,10 @@ void main() {
), ),
); );
final PhysicalShape physicalShape = final PhysicalShape physicalShape = tester.widget(
tester.widget(find.byType(PhysicalShape).at(0)); find.descendant(of: find.byType(BottomAppBar), matching: find.byType(PhysicalShape)));
final Material material = tester.widget(find.byType(Material).at(1));
expect(physicalShape.color, const Color(0xff0000ff)); expect(physicalShape.color, const Color(0xff0000ff));
expect(material.color, const Color(0xff0000ff));
}); });
testWidgets('Shadow color is transparent in Material 3', (WidgetTester tester) async { testWidgets('Shadow color is transparent in Material 3', (WidgetTester tester) async {
...@@ -249,9 +315,10 @@ void main() { ...@@ -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 { testWidgets('dark theme applies an elevation overlay color', (WidgetTester tester) async {
......
...@@ -114,15 +114,6 @@ void main() { ...@@ -114,15 +114,6 @@ void main() {
}); });
group('Material 3 tests', () { group('Material 3 tests', () {
Material getBabRenderObject(WidgetTester tester) {
return tester.widget<Material>(
find.descendant(
of: find.byType(BottomAppBar),
matching: find.byType(Material),
),
);
}
testWidgets('BAB theme overrides color - M3', (WidgetTester tester) async { testWidgets('BAB theme overrides color - M3', (WidgetTester tester) async {
const Color themedColor = Colors.black87; const Color themedColor = Colors.black87;
const BottomAppBarTheme theme = BottomAppBarTheme( const BottomAppBarTheme theme = BottomAppBarTheme(
...@@ -147,7 +138,7 @@ void main() { ...@@ -147,7 +138,7 @@ void main() {
bottomAppBarTheme: theme, bottomAppBarTheme: theme,
bottomAppBarColor: themeColor bottomAppBarColor: themeColor
), ),
home: const Scaffold(body: BottomAppBar(color: babColor)), home: const Scaffold(body: BottomAppBar(color: babColor, surfaceTintColor: Colors.transparent)),
)); ));
final PhysicalShape widget = _getBabRenderObject(tester); final PhysicalShape widget = _getBabRenderObject(tester);
...@@ -165,7 +156,7 @@ void main() { ...@@ -165,7 +156,7 @@ void main() {
bottomAppBarTheme: theme, bottomAppBarTheme: theme,
bottomAppBarColor: themeColor bottomAppBarColor: themeColor
), ),
home: const Scaffold(body: BottomAppBar()), home: const Scaffold(body: BottomAppBar(surfaceTintColor: Colors.transparent)),
)); ));
final PhysicalShape widget = _getBabRenderObject(tester); final PhysicalShape widget = _getBabRenderObject(tester);
...@@ -176,7 +167,7 @@ void main() { ...@@ -176,7 +167,7 @@ void main() {
final ThemeData theme = ThemeData(useMaterial3: true); final ThemeData theme = ThemeData(useMaterial3: true);
await tester.pumpWidget(MaterialApp( await tester.pumpWidget(MaterialApp(
theme: theme, theme: theme,
home: const Scaffold(body: BottomAppBar()), home: const Scaffold(body: BottomAppBar(surfaceTintColor: Colors.transparent)),
)); ));
final PhysicalShape widget = _getBabRenderObject(tester); final PhysicalShape widget = _getBabRenderObject(tester);
...@@ -186,14 +177,15 @@ void main() { ...@@ -186,14 +177,15 @@ void main() {
}); });
testWidgets('BAB theme overrides surfaceTintColor - M3', (WidgetTester tester) async { 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 Color babThemeSurfaceTintColor = Colors.black87;
const BottomAppBarTheme theme = BottomAppBarTheme( const BottomAppBarTheme theme = BottomAppBarTheme(
surfaceTintColor: babThemeSurfaceTintColor, elevation: 0 color: color, surfaceTintColor: babThemeSurfaceTintColor, elevation: 0,
); );
await tester.pumpWidget(_withTheme(theme, true)); await tester.pumpWidget(_withTheme(theme, true));
final Material widget = getBabRenderObject(tester); final PhysicalShape widget = _getBabRenderObject(tester);
expect(widget.surfaceTintColor, babThemeSurfaceTintColor); expect(widget.color, ElevationOverlay.applySurfaceTint(color, babThemeSurfaceTintColor, 0));
}); });
testWidgets('BAB theme overrides shadowColor - M3', (WidgetTester tester) async { testWidgets('BAB theme overrides shadowColor - M3', (WidgetTester tester) async {
...@@ -203,11 +195,12 @@ void main() { ...@@ -203,11 +195,12 @@ void main() {
); );
await tester.pumpWidget(_withTheme(theme, true)); await tester.pumpWidget(_withTheme(theme, true));
final Material widget = getBabRenderObject(tester); final PhysicalShape widget = _getBabRenderObject(tester);
expect(widget.shadowColor, babThemeShadowColor); expect(widget.shadowColor, babThemeShadowColor);
}); });
testWidgets('BAB surfaceTintColor - Widget - M3', (WidgetTester tester) async { 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 themeSurfaceTintColor = Colors.white10;
const Color babThemeSurfaceTintColor = Colors.black87; const Color babThemeSurfaceTintColor = Colors.black87;
const Color babSurfaceTintColor = Colors.pink; const Color babSurfaceTintColor = Colors.pink;
...@@ -221,15 +214,16 @@ void main() { ...@@ -221,15 +214,16 @@ void main() {
bottomAppBarColor: themeSurfaceTintColor bottomAppBarColor: themeSurfaceTintColor
), ),
home: const Scaffold( home: const Scaffold(
body: BottomAppBar(surfaceTintColor: babSurfaceTintColor) body: BottomAppBar(color: color, surfaceTintColor: babSurfaceTintColor)
), ),
)); ));
final Material widget = getBabRenderObject(tester); final PhysicalShape widget = _getBabRenderObject(tester);
expect(widget.surfaceTintColor, babSurfaceTintColor); expect(widget.color, ElevationOverlay.applySurfaceTint(color, babSurfaceTintColor, 3.0));
}); });
testWidgets('BAB surfaceTintColor - BabTheme - M3', (WidgetTester tester) async { 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 themeColor = Colors.white10;
const Color babThemeColor = Colors.black87; const Color babThemeColor = Colors.black87;
const BottomAppBarTheme theme = BottomAppBarTheme( const BottomAppBarTheme theme = BottomAppBarTheme(
...@@ -242,11 +236,11 @@ void main() { ...@@ -242,11 +236,11 @@ void main() {
bottomAppBarTheme: theme, bottomAppBarTheme: theme,
bottomAppBarColor: themeColor bottomAppBarColor: themeColor
), ),
home: const Scaffold(body: BottomAppBar()), home: const Scaffold(body: BottomAppBar(color: color)),
)); ));
final Material widget = getBabRenderObject(tester); final PhysicalShape widget = _getBabRenderObject(tester);
expect(widget.surfaceTintColor, babThemeColor); expect(widget.color, ElevationOverlay.applySurfaceTint(color, babThemeColor, 3.0));
}); });
}); });
} }
......
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