Unverified Commit efcc8b76 authored by Taha Tesser's avatar Taha Tesser Committed by GitHub

Fix license page title color issues (#121872)

parent 965ddfd8
......@@ -463,11 +463,11 @@ class _LicensePageState extends State<LicensePage> {
Widget _packagesView(final BuildContext _, final bool isLateral) {
final Widget about = _AboutProgram(
name: widget.applicationName ?? _defaultApplicationName(context),
icon: widget.applicationIcon ?? _defaultApplicationIcon(context),
version: widget.applicationVersion ?? _defaultApplicationVersion(context),
legalese: widget.applicationLegalese,
);
name: widget.applicationName ?? _defaultApplicationName(context),
icon: widget.applicationIcon ?? _defaultApplicationIcon(context),
version: widget.applicationVersion ?? _defaultApplicationVersion(context),
legalese: widget.applicationLegalese,
);
return _PackagesView(
about: about,
isLateral: isLateral,
......@@ -870,10 +870,11 @@ class _PackageLicensePageState extends State<_PackageLicensePage> {
page = Scaffold(
appBar: AppBar(
title: _PackageLicensePageTitle(
title,
subtitle,
theme.primaryTextTheme,
theme.appBarTheme.titleTextStyle,
title: title,
subtitle: subtitle,
theme: theme.useMaterial3 ? theme.textTheme : theme.primaryTextTheme,
titleTextStyle: theme.appBarTheme.titleTextStyle,
foregroundColor: theme.appBarTheme.foregroundColor,
),
),
body: Center(
......@@ -909,7 +910,12 @@ class _PackageLicensePageState extends State<_PackageLicensePage> {
automaticallyImplyLeading: false,
pinned: true,
backgroundColor: theme.cardColor,
title: _PackageLicensePageTitle(title, subtitle, theme.textTheme, theme.textTheme.titleLarge),
title: _PackageLicensePageTitle(
title: title,
subtitle: subtitle,
theme: theme.textTheme,
titleTextStyle: theme.textTheme.titleLarge,
),
),
SliverPadding(
padding: padding,
......@@ -935,29 +941,29 @@ class _PackageLicensePageState extends State<_PackageLicensePage> {
}
class _PackageLicensePageTitle extends StatelessWidget {
const _PackageLicensePageTitle(
this.title,
this.subtitle,
this.theme,
const _PackageLicensePageTitle({
required this.title,
required this.subtitle,
required this.theme,
this.titleTextStyle,
);
this.foregroundColor,
});
final String title;
final String subtitle;
final TextTheme theme;
final TextStyle? titleTextStyle;
final Color? foregroundColor;
@override
Widget build(BuildContext context) {
final Color? color = Theme.of(context).appBarTheme.foregroundColor;
final TextStyle? effectiveTitleTextStyle = titleTextStyle ?? theme.titleLarge;
return Column(
mainAxisAlignment: MainAxisAlignment.center,
crossAxisAlignment: CrossAxisAlignment.start,
children: <Widget>[
Text(title, style: effectiveTitleTextStyle?.copyWith(color: color)),
Text(subtitle, style: theme.titleSmall?.copyWith(color: color)),
Text(title, style: effectiveTitleTextStyle?.copyWith(color: foregroundColor)),
Text(subtitle, style: theme.titleSmall?.copyWith(color: foregroundColor)),
],
);
}
......@@ -1227,16 +1233,18 @@ class _MasterDetailFlowState extends State<_MasterDetailFlow> implements _PageOp
MaterialPageRoute<void> _masterPageRoute(BuildContext context) {
return MaterialPageRoute<dynamic>(
builder: (BuildContext c) => BlockSemantics(
child: _MasterPage(
leading: widget.automaticallyImplyLeading && Navigator.of(context).canPop()
? BackButton(onPressed: () => Navigator.of(context).pop())
: null,
title: widget.title,
automaticallyImplyLeading: widget.automaticallyImplyLeading,
masterViewBuilder: widget.masterViewBuilder,
),
),
builder: (BuildContext c) {
return BlockSemantics(
child: _MasterPage(
leading: widget.automaticallyImplyLeading && Navigator.of(context).canPop()
? BackButton(onPressed: () { Navigator.of(context).pop(); })
: null,
title: widget.title,
automaticallyImplyLeading: widget.automaticallyImplyLeading,
masterViewBuilder: widget.masterViewBuilder,
),
);
},
);
}
......@@ -1285,14 +1293,14 @@ class _MasterPage extends StatelessWidget {
@override
Widget build(BuildContext context) {
return Scaffold(
appBar: AppBar(
title: title,
leading: leading,
actions: const <Widget>[],
automaticallyImplyLeading: automaticallyImplyLeading,
),
body: masterViewBuilder!(context, false),
);
appBar: AppBar(
title: title,
leading: leading,
actions: const <Widget>[],
automaticallyImplyLeading: automaticallyImplyLeading,
),
body: masterViewBuilder!(context, false),
);
}
}
......
......@@ -8,6 +8,7 @@ import 'dart:ui';
import 'package:flutter/cupertino.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart';
void main() {
......@@ -1094,8 +1095,7 @@ void main() {
),
);
// To stop loading
await tester.pumpAndSettle();
await tester.pumpAndSettle(); // Finish rendering the page.
// If the layout width is less than 840.0 pixels, nested layout is
// used which positions license page title at the top center.
......@@ -1118,8 +1118,7 @@ void main() {
),
);
// To finish the FakeTimer
await tester.pumpAndSettle();
await tester.pumpAndSettle(); // Finish rendering the page.
// If the layout width is greater than 840.0 pixels, lateral UI layout
// is used which positions license page title and packageList
......@@ -1159,8 +1158,7 @@ void main() {
),
);
// To stop loading
await tester.pumpAndSettle();
await tester.pumpAndSettle(); // Finish rendering the page.
// If the layout width is less than 840.0 pixels, nested layout is
// used which positions license page title at the top center.
......@@ -1183,8 +1181,7 @@ void main() {
),
);
// To finish the FakeTimer
await tester.pumpAndSettle();
await tester.pumpAndSettle(); // Finish rendering the page.
// If the layout width is greater than 840.0 pixels, lateral UI layout
// is used which positions license page title and packageList
......@@ -1197,6 +1194,123 @@ void main() {
// Configure to show the default layout.
await tester.binding.setSurfaceSize(defaultSize);
});
testWidgets('License page title in lateral UI does not use AppBarTheme.foregroundColor', (WidgetTester tester) async {
// This is a regression test for https://github.com/flutter/flutter/issues/108991
final ThemeData theme = ThemeData(
appBarTheme: const AppBarTheme(foregroundColor: Color(0xFFFFFFFF)),
useMaterial3: true,
);
const String title = 'License ABC';
LicenseRegistry.addLicense(() {
return Stream<LicenseEntry>.fromIterable(<LicenseEntry>[
const LicenseEntryWithLineBreaks(<String>['ABC'], 'DEF'),
]);
});
// Configure a wide window to show the lateral UI.
await tester.binding.setSurfaceSize(const Size(1200.0, 600.0));
await tester.pumpWidget(
MaterialApp(
title: title,
theme: theme,
home: const Scaffold(
body: LicensePage(),
),
),
);
await tester.pumpAndSettle(); // Finish rendering the page.
final RenderParagraph renderParagraph = tester.renderObject(find.text('ABC').last) as RenderParagraph;
// License page title should not use AppBarTheme's foregroundColor.
expect(renderParagraph.text.style!.color, isNot(theme.appBarTheme.foregroundColor));
// License page title in the lateral UI uses default text style color.
expect(renderParagraph.text.style!.color, theme.textTheme.titleLarge!.color);
// Configure to show the default layout.
await tester.binding.setSurfaceSize(const Size(800.0, 600.0));
});
testWidgets('License page default title text color in the nested UI', (WidgetTester tester) async {
// This is a regression test for https://github.com/flutter/flutter/issues/108991
final ThemeData theme = ThemeData(useMaterial3: true);
const String title = 'License ABC';
LicenseRegistry.addLicense(() {
return Stream<LicenseEntry>.fromIterable(<LicenseEntry>[
const LicenseEntryWithLineBreaks(<String>['ABC'], 'DEF'),
]);
});
await tester.pumpWidget(
MaterialApp(
title: title,
theme: theme,
home: const Scaffold(
body: LicensePage(),
),
),
);
await tester.pumpAndSettle(); // Finish rendering the page.
// Currently in the master view.
expect(find.text('License ABC'), findsOneWidget);
// Navigate to the license page.
await tester.tap(find.text('ABC'));
await tester.pumpAndSettle();
// Master view is no longer visible.
expect(find.text('License ABC'), findsNothing);
final RenderParagraph renderParagraph = tester.renderObject(find.text('ABC').first) as RenderParagraph;
expect(renderParagraph.text.style!.color, theme.textTheme.titleLarge!.color);
});
group('Material 2', () {
// Tests that are only relevant for Material 2. Once ThemeData.useMaterial3
// is turned on by default, these tests can be removed.
testWidgets('License page default title text color in the nested UI', (WidgetTester tester) async {
// This is a regression test for https://github.com/flutter/flutter/issues/108991
final ThemeData theme = ThemeData(useMaterial3: false);
const String title = 'License ABC';
LicenseRegistry.addLicense(() {
return Stream<LicenseEntry>.fromIterable(<LicenseEntry>[
const LicenseEntryWithLineBreaks(<String>['ABC'], 'DEF'),
]);
});
await tester.pumpWidget(
MaterialApp(
title: title,
theme: theme,
home: const Scaffold(
body: LicensePage(),
),
),
);
await tester.pumpAndSettle(); // Finish rendering the page.
// Currently in the master view.
expect(find.text('License ABC'), findsOneWidget);
// Navigate to the license page.
await tester.tap(find.text('ABC'));
await tester.pumpAndSettle();
// Master view is no longer visible.
expect(find.text('License ABC'), findsNothing);
final RenderParagraph renderParagraph = tester.renderObject(find.text('ABC').first) as RenderParagraph;
expect(renderParagraph.text.style!.color, theme.primaryTextTheme.titleLarge!.color);
});
});
}
class FakeLicenseEntry extends LicenseEntry {
......
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