Skip to content

F
Front-End
Unverified Commit 56e1bddc authored by Taha Tesser's avatar Taha Tesser Committed by GitHub
Browse files
Options

Fix `SliverAppBar.medium` & `SliverAppBar.large` title overlap with... 

Fix `SliverAppBar.medium` & `SliverAppBar.large` title overlap with leading/actions widgets, leading width, and title spacing (#120780)

* Fix `SliverAppBar.medium` & `SliverAppBar.large` title overlap with leading/actions widgets, leading width, and title spacing

* Add `titleSpacing` theme tests and consolidate multiple tests for the same widgets
parent af347d66
Expand all Show whitespace changes
Inline Side-by-side
Showing with 595 additions and 72 deletions
dev/tools/gen_defaults/lib/app_bar_template.dart
View file @ 56e1bddc
......@@ -79,10 +79,10 @@ class _MediumScrollUnderFlexibleConfig with _ScrollUnderFlexibleConfig {
${textStyle('md.comp.top-app-bar.medium.headline')}?.apply(color: ${color('md.comp.top-app-bar.medium.headline.color')});
@override
EdgeInsetsGeometry? get collapsedTitlePadding => const EdgeInsetsDirectional.fromSTEB(48, 0, 16, 0);
EdgeInsetsGeometry? get collapsedTitlePadding => const EdgeInsetsDirectional.only(start: 40);
@override
EdgeInsetsGeometry? get collapsedCenteredTitlePadding => const EdgeInsets.fromLTRB(16, 0, 16, 0);
EdgeInsetsGeometry? get collapsedCenteredTitlePadding => const EdgeInsetsDirectional.only(start: 40);
@override
EdgeInsetsGeometry? get expandedTitlePadding => const EdgeInsets.fromLTRB(16, 0, 16, 20);
......@@ -108,10 +108,10 @@ class _LargeScrollUnderFlexibleConfig with _ScrollUnderFlexibleConfig {
${textStyle('md.comp.top-app-bar.large.headline')}?.apply(color: ${color('md.comp.top-app-bar.large.headline.color')});
@override
EdgeInsetsGeometry? get collapsedTitlePadding => const EdgeInsetsDirectional.fromSTEB(48, 0, 16, 0);
EdgeInsetsGeometry? get collapsedTitlePadding => const EdgeInsetsDirectional.only(start: 40);
@override
EdgeInsetsGeometry? get collapsedCenteredTitlePadding => const EdgeInsets.fromLTRB(16, 0, 16, 0);
EdgeInsetsGeometry? get collapsedCenteredTitlePadding => const EdgeInsetsDirectional.only(start: 40);
@override
EdgeInsetsGeometry? get expandedTitlePadding => const EdgeInsets.fromLTRB(16, 0, 16, 28);
......
packages/flutter/lib/src/material/app_bar.dart
View file @ 56e1bddc
......@@ -1528,13 +1528,16 @@ class SliverAppBar extends StatefulWidget {
key: key,
leading: leading,
automaticallyImplyLeading: automaticallyImplyLeading,
actions: actions,
flexibleSpace: flexibleSpace ?? _ScrollUnderFlexibleSpace(
hasLeading: leading != null,
title: title,
actions: actions,
foregroundColor: foregroundColor,
variant: _ScrollUnderFlexibleVariant.medium,
centerCollapsedTitle: centerTitle,
primary: primary,
leadingWidth: leadingWidth,
titleSpacing: titleSpacing,
),
bottom: bottom,
elevation: elevation,
......@@ -1630,13 +1633,16 @@ class SliverAppBar extends StatefulWidget {
key: key,
leading: leading,
automaticallyImplyLeading: automaticallyImplyLeading,
actions: actions,
flexibleSpace: flexibleSpace ?? _ScrollUnderFlexibleSpace(
hasLeading: leading != null,
title: title,
actions: actions,
foregroundColor: foregroundColor,
variant: _ScrollUnderFlexibleVariant.large,
centerCollapsedTitle: centerTitle,
primary: primary,
leadingWidth: leadingWidth,
titleSpacing: titleSpacing,
),
bottom: bottom,
elevation: elevation,
......@@ -2077,18 +2083,26 @@ enum _ScrollUnderFlexibleVariant { medium, large }
class _ScrollUnderFlexibleSpace extends StatelessWidget {
const _ScrollUnderFlexibleSpace({
required this.hasLeading,
this.title,
this.actions,
this.foregroundColor,
required this.variant,
this.centerCollapsedTitle,
this.primary = true,
this.leadingWidth,
this.titleSpacing,
});
final bool hasLeading;
final Widget? title;
final List<Widget>? actions;
final Color? foregroundColor;
final _ScrollUnderFlexibleVariant variant;
final bool? centerCollapsedTitle;
final bool primary;
final double? leadingWidth;
final double? titleSpacing;
@override
Widget build(BuildContext context) {
......@@ -2142,6 +2156,14 @@ class _ScrollUnderFlexibleSpace extends StatelessWidget {
centerTitle = centerCollapsedTitle ?? appBarTheme.centerTitle ?? platformCenter();
}
EdgeInsetsGeometry effectiveCollapsedTitlePadding = EdgeInsets.zero;
if (hasLeading && leadingWidth == null) {
effectiveCollapsedTitlePadding = centerTitle
? config.collapsedCenteredTitlePadding!
: config.collapsedTitlePadding!;
} else if (hasLeading && leadingWidth != null) {
effectiveCollapsedTitlePadding = EdgeInsetsDirectional.only(start: leadingWidth!);
}
final bool isCollapsed = settings.isScrolledUnder ?? false;
return Column(
children: <Widget>[
......@@ -2149,17 +2171,20 @@ class _ScrollUnderFlexibleSpace extends StatelessWidget {
padding: EdgeInsets.only(top: topPadding),
child: Container(
height: collapsedHeight,
padding: centerTitle ? config.collapsedCenteredTitlePadding : config.collapsedTitlePadding,
child: AnimatedOpacity(
padding: effectiveCollapsedTitlePadding,
child: NavigationToolbar(
centerMiddle: centerTitle,
middleSpacing: titleSpacing ?? appBarTheme.titleSpacing ?? NavigationToolbar.kMiddleSpacing,
middle: AnimatedOpacity(
opacity: isCollapsed ? 1 : 0,
duration: const Duration(milliseconds: 500),
curve: const Cubic(0.2, 0.0, 0.0, 1.0),
child: Align(
alignment: centerTitle
? Alignment.center
: AlignmentDirectional.centerStart,
child: collapsedTitle,
),
trailing: actions != null ? Row(
mainAxisSize: MainAxisSize.min,
children: actions!,
) : null,
),
),
),
......@@ -2295,10 +2320,10 @@ class _MediumScrollUnderFlexibleConfig with _ScrollUnderFlexibleConfig {
_textTheme.headlineSmall?.apply(color: _colors.onSurface);
@override
EdgeInsetsGeometry? get collapsedTitlePadding => const EdgeInsetsDirectional.fromSTEB(48, 0, 16, 0);
EdgeInsetsGeometry? get collapsedTitlePadding => const EdgeInsetsDirectional.only(start: 40);
@override
EdgeInsetsGeometry? get collapsedCenteredTitlePadding => const EdgeInsets.fromLTRB(16, 0, 16, 0);
EdgeInsetsGeometry? get collapsedCenteredTitlePadding => const EdgeInsetsDirectional.only(start: 40);
@override
EdgeInsetsGeometry? get expandedTitlePadding => const EdgeInsets.fromLTRB(16, 0, 16, 20);
......@@ -2324,10 +2349,10 @@ class _LargeScrollUnderFlexibleConfig with _ScrollUnderFlexibleConfig {
_textTheme.headlineMedium?.apply(color: _colors.onSurface);
@override
EdgeInsetsGeometry? get collapsedTitlePadding => const EdgeInsetsDirectional.fromSTEB(48, 0, 16, 0);
EdgeInsetsGeometry? get collapsedTitlePadding => const EdgeInsetsDirectional.only(start: 40);
@override
EdgeInsetsGeometry? get collapsedCenteredTitlePadding => const EdgeInsets.fromLTRB(16, 0, 16, 0);
EdgeInsetsGeometry? get collapsedCenteredTitlePadding => const EdgeInsetsDirectional.only(start: 40);
@override
EdgeInsetsGeometry? get expandedTitlePadding => const EdgeInsets.fromLTRB(16, 0, 16, 28);
......
packages/flutter/test/material/app_bar_theme_test.dart
View file @ 56e1bddc
......@@ -9,6 +9,10 @@ import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
void main() {
ScrollController primaryScrollController(WidgetTester tester) {
return PrimaryScrollController.of(tester.element(find.byType(CustomScrollView)));
}
test('AppBarTheme copyWith, ==, hashCode basics', () {
expect(const AppBarTheme(), const AppBarTheme().copyWith());
expect(const AppBarTheme().hashCode, const AppBarTheme().copyWith().hashCode);
......@@ -676,14 +680,28 @@ void main() {
expect(navToolbar.middleSpacing, 40);
});
testWidgets("SliverAppBar.medium's title uses AppBarTheme.foregroundColor", (WidgetTester tester) async {
testWidgets('SliverAppBar.medium uses AppBarTheme properties', (WidgetTester tester) async {
const String title = 'Medium SliverAppBar Title';
const Color foregroundColor = Color(0xff00ff00);
const double titleSpacing = 10.0;
await tester.pumpWidget(MaterialApp(
theme: ThemeData(appBarTheme: const AppBarTheme(foregroundColor: foregroundColor)),
theme: ThemeData(
appBarTheme: const AppBarTheme(
foregroundColor: foregroundColor,
titleSpacing: titleSpacing,
centerTitle: false,
),
),
home: CustomScrollView(
primary: true,
slivers: <Widget>[
SliverAppBar.medium(
title: const Text('Medium Title'),
leading: IconButton(
onPressed: () {},
icon: const Icon(Icons.menu),
),
title: const Text(title),
),
],
),
......@@ -691,18 +709,43 @@ void main() {
final RichText text = tester.firstWidget(find.byType(RichText));
expect(text.text.style!.color, foregroundColor);
// Scroll to collapse the SliverAppBar.
final ScrollController controller = primaryScrollController(tester);
controller.jumpTo(45);
await tester.pumpAndSettle();
final Offset titleOffset = tester.getTopLeft(find.text(title).first);
final Offset iconOffset = tester.getTopRight(find.byIcon(Icons.menu));
// Title spacing should be 10.0.
expect(titleOffset.dx, iconOffset.dx + titleSpacing);
});
testWidgets(
"SliverAppBar.medium's foregroundColor takes priority over AppBarTheme.foregroundColor", (WidgetTester tester) async {
testWidgets('SliverAppBar.medium properties take priority over AppBarTheme properties', (WidgetTester tester) async {
const String title = 'Medium SliverAppBar Title';
const Color foregroundColor = Color(0xff00ff00);
const double titleSpacing = 10.0;
await tester.pumpWidget(MaterialApp(
theme: ThemeData(appBarTheme: const AppBarTheme(foregroundColor: Color(0xffff0000))),
theme: ThemeData(
appBarTheme: const AppBarTheme(
foregroundColor: Color(0xffff0000),
titleSpacing: 14.0,
centerTitle: true,
),
),
home: CustomScrollView(
primary: true,
slivers: <Widget>[
SliverAppBar.medium(
centerTitle: false,
titleSpacing: titleSpacing,
foregroundColor: foregroundColor,
title: const Text('Medium Title'),
leading: IconButton(
onPressed: () {},
icon: const Icon(Icons.menu),
),
title: const Text(title),
),
],
),
......@@ -710,16 +753,40 @@ void main() {
final RichText text = tester.firstWidget(find.byType(RichText));
expect(text.text.style!.color, foregroundColor);
// Scroll to collapse the SliverAppBar.
final ScrollController controller = primaryScrollController(tester);
controller.jumpTo(45);
await tester.pumpAndSettle();
final Offset titleOffset = tester.getTopLeft(find.text(title).first);
final Offset iconOffset = tester.getTopRight(find.byIcon(Icons.menu));
// Title spacing should be 10.0.
expect(titleOffset.dx, iconOffset.dx + titleSpacing);
});
testWidgets("SliverAppBar.large's title uses AppBarTheme.foregroundColor", (WidgetTester tester) async {
testWidgets('SliverAppBar.large uses AppBarTheme properties', (WidgetTester tester) async {
const String title = 'Large SliverAppBar Title';
const Color foregroundColor = Color(0xff00ff00);
const double titleSpacing = 10.0;
await tester.pumpWidget(MaterialApp(
theme: ThemeData(appBarTheme: const AppBarTheme(foregroundColor: foregroundColor)),
theme: ThemeData(
appBarTheme: const AppBarTheme(
foregroundColor: foregroundColor,
titleSpacing: titleSpacing,
centerTitle: false,
),
),
home: CustomScrollView(
primary: true,
slivers: <Widget>[
SliverAppBar.large(
title: const Text('Large Title'),
leading: IconButton(
onPressed: () {},
icon: const Icon(Icons.menu),
),
title: const Text(title),
),
],
),
......@@ -727,18 +794,43 @@ void main() {
final RichText text = tester.firstWidget(find.byType(RichText));
expect(text.text.style!.color, foregroundColor);
// Scroll to collapse the SliverAppBar.
final ScrollController controller = primaryScrollController(tester);
controller.jumpTo(45);
await tester.pumpAndSettle();
final Offset titleOffset = tester.getTopLeft(find.text(title).first);
final Offset iconOffset = tester.getTopRight(find.byIcon(Icons.menu));
// Title spacing should be 10.0.
expect(titleOffset.dx, iconOffset.dx + titleSpacing);
});
testWidgets(
"SliverAppBar.large's foregroundColor takes priority over AppBarTheme.foregroundColor", (WidgetTester tester) async {
testWidgets('SliverAppBar.large properties take priority over AppBarTheme properties', (WidgetTester tester) async {
const String title = 'Large SliverAppBar Title';
const Color foregroundColor = Color(0xff00ff00);
const double titleSpacing = 10.0;
await tester.pumpWidget(MaterialApp(
theme: ThemeData(appBarTheme: const AppBarTheme(foregroundColor: Color(0xffff0000))),
theme: ThemeData(
appBarTheme: const AppBarTheme(
foregroundColor: Color(0xffff0000),
titleSpacing: 14.0,
centerTitle: true,
),
),
home: CustomScrollView(
primary: true,
slivers: <Widget>[
SliverAppBar.large(
centerTitle: false,
titleSpacing: titleSpacing,
foregroundColor: foregroundColor,
title: const Text('Large Title'),
leading: IconButton(
onPressed: () {},
icon: const Icon(Icons.menu),
),
title: const Text(title),
),
],
),
......@@ -746,6 +838,16 @@ void main() {
final RichText text = tester.firstWidget(find.byType(RichText));
expect(text.text.style!.color, foregroundColor);
// Scroll to collapse the SliverAppBar.
final ScrollController controller = primaryScrollController(tester);
controller.jumpTo(45);
await tester.pumpAndSettle();
final Offset titleOffset = tester.getTopLeft(find.text(title).first);
final Offset iconOffset = tester.getTopRight(find.byIcon(Icons.menu));
// Title spacing should be 10.0.
expect(titleOffset.dx, iconOffset.dx + titleSpacing);
});
testWidgets('Default AppBarTheme debugFillProperties', (WidgetTester tester) async {
......
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

HIAST - GitLab