development/flutter
development/flutter
1
0
Fork 0
mirror of https://github.com/flutter/flutter synced 2024-09-21 09:11:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix handling of null body2 text style for chip and slider (#17311)

Browse source 
Before this change, if you specified a non-null textTheme, but the theme you specified didn't have a body2 defined, then creating a ChipTheme would assert (which means creating a ThemeData would fail).

This adds handling for this corner case to default to reasonable values in that case. The slider had the same problem, but for accentTextTheme, so I fixed that too.

While I had the patient open, Hans and I noticed that TextTheme.merge wasn't doing the right thing in the case where some members were null either, so I fixed that, and added some examples, since merge/copyWith are common operations that are not always well understood.

Fixes #17251
This commit is contained in:
Greg Spencer 2018-05-10 14:37:14 -07:00 committed by GitHub
parent bb4afb0472
commit a365c41cb5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 136 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
packages/flutter/lib/src/material/theme_data.dart
View file

@ -144,9 +144,12 @@ class ThemeData extends Diagnosticable {
accentIconTheme ??= accentIsDark ? const IconThemeData(color: Colors.white) : const IconThemeData(color: Colors.black);
platform ??= defaultTargetPlatform;
final Typography typography = new Typography(platform: platform);
textTheme ??= isDark ? typography.white : typography.black;
primaryTextTheme ??= primaryIsDark ? typography.white : typography.black;
accentTextTheme ??= accentIsDark ? typography.white : typography.black;
final TextTheme defaultTextTheme = isDark ? typography.white : typography.black;
textTheme = defaultTextTheme.merge(textTheme);
final TextTheme defaultPrimaryTextTheme = primaryIsDark ? typography.white : typography.black;
primaryTextTheme = defaultPrimaryTextTheme.merge(primaryTextTheme);
final TextTheme defaultAccentTextTheme = accentIsDark ? typography.white : typography.black;
accentTextTheme = defaultAccentTextTheme.merge(accentTextTheme);
if (fontFamily != null) {
textTheme = textTheme.apply(fontFamily: fontFamily);
primaryTextTheme = primaryTextTheme.apply(fontFamily: fontFamily);

100
packages/flutter/lib/src/material/typography.dart
View file

@ -7,6 +7,8 @@ import 'package:flutter/painting.dart';
import 'colors.dart';
/// Material design text theme.
///
/// Definitions for the various typographical styles found in material design
@ -103,6 +105,39 @@ class TextTheme extends Diagnosticable {
/// Consider using [Typography.black] or [Typography.white], which implement
/// the typography styles in the material design specification, as a starting
/// point.
///
/// ## Sample code
///
/// ```dart
/// /// A Widget that sets the ambient theme's title text color for its
/// /// descendants, while leaving other ambient theme attributes alone.
/// class TitleColorThemeCopy extends StatelessWidget {
/// TitleColorThemeCopy({Key key, this.child, this.titleColor}) : super(key: key);
///
/// final Color titleColor;
/// final Widget child;
///
/// @override
/// Widget build(BuildContext context) {
/// final ThemeData theme = Theme.of(context);
/// return new Theme(
/// data: theme.copyWith(
/// textTheme: theme.textTheme.copyWith(
/// title: theme.textTheme.title.copyWith(
/// color: titleColor,
/// ),
/// ),
/// ),
/// child: child,
/// );
/// }
/// }
/// ```
///
/// See also:
///
/// * [merge] is used instead of [copyWith] when you want to merge all
/// of the fields of a TextTheme instead of individual fields.
TextTheme copyWith({
TextStyle display4,
TextStyle display3,
@ -139,24 +174,63 @@ class TextTheme extends Diagnosticable {
/// the value of [TextStyle.inherit] flag. For more details, see the
/// documentation on [TextStyle.merge] and [TextStyle.inherit].
///
/// If this theme, or the `other` theme has members that are null, then the
/// non-null one (if any) is used. If the `other` theme is itself null, then
/// this [TextTheme] is returned unchanged. If values in both are set, then
/// the values are merged using [TextStyle.merge].
///
/// This is particularly useful if one [TextTheme] defines one set of
/// properties and another defines a different set, e.g. having colors defined
/// in one text theme and font sizes in another.
/// properties and another defines a different set, e.g. having colors
/// defined in one text theme and font sizes in another, or when one
/// [TextTheme] has only some fields defined, and you want to define the rest
/// by merging it with a default theme.
///
/// ## Sample code
///
/// ```dart
/// /// A Widget that sets the ambient theme's title text color for its
/// /// descendants, while leaving other ambient theme attributes alone.
/// class TitleColorTheme extends StatelessWidget {
/// TitleColorTheme({Key key, this.child, this.titleColor}) : super(key: key);
///
/// final Color titleColor;
/// final Widget child;
///
/// @override
/// Widget build(BuildContext context) {
/// ThemeData theme = Theme.of(context);
/// // This partialTheme is incomplete: it only has the title style
/// // defined. Just replacing theme.textTheme with partialTheme would
/// // set the title, but everything else would be null. This isn't very
/// // useful, so merge it with the existing theme to keep all of the
/// // preexisting definitions for the other styles.
/// TextTheme partialTheme = new TextTheme(title: new TextStyle(color: titleColor));
/// theme = theme.copyWith(textTheme: theme.textTheme.merge(partialTheme));
/// return new Theme(data: theme, child: child);
/// }
/// }
/// ```
///
/// See also:
///
/// * [copyWith] is used instead of [merge] when you wish to override
/// individual fields in the [TextTheme] instead of merging all of the
/// fields of two [TextTheme]s.
TextTheme merge(TextTheme other) {
if (other == null)
return this;
return copyWith(
display4: display4.merge(other.display4),
display3: display3.merge(other.display3),
display2: display2.merge(other.display2),
display1: display1.merge(other.display1),
headline: headline.merge(other.headline),
title: title.merge(other.title),
subhead: subhead.merge(other.subhead),
body2: body2.merge(other.body2),
body1: body1.merge(other.body1),
caption: caption.merge(other.caption),
button: button.merge(other.button),
display4: display4?.merge(other.display4) ?? other.display4,
display3: display3?.merge(other.display3) ?? other.display3,
display2: display2?.merge(other.display2) ?? other.display2,
display1: display1?.merge(other.display1) ?? other.display1,
headline: headline?.merge(other.headline) ?? other.headline,
title: title?.merge(other.title) ?? other.title,
subhead: subhead?.merge(other.subhead) ?? other.subhead,
body2: body2?.merge(other.body2) ?? other.body2,
body1: body1?.merge(other.body1) ?? other.body1,
caption: caption?.merge(other.caption) ?? other.caption,
button: button?.merge(other.button) ?? other.button,
);
}

20
packages/flutter/test/material/theme_data_test.dart
View file

@ -56,6 +56,26 @@ void main() {
expect(darkTheme.accentTextTheme.title.color, typography.white.title.color);
});
test('Default slider indicator style gets a default body2 if accentTextTheme.body2 is null', () {
const TextTheme noBody2TextTheme = const TextTheme(body2: null);
final ThemeData lightTheme = new ThemeData(brightness: Brightness.light, accentTextTheme: noBody2TextTheme);
final ThemeData darkTheme = new ThemeData(brightness: Brightness.dark, accentTextTheme: noBody2TextTheme);
final Typography typography = new Typography(platform: lightTheme.platform);
expect(lightTheme.sliderTheme.valueIndicatorTextStyle, equals(typography.white.body2));
expect(darkTheme.sliderTheme.valueIndicatorTextStyle, equals(typography.black.body2));
});
test('Default chip label style gets a default body2 if textTheme.body2 is null', () {
const TextTheme noBody2TextTheme = const TextTheme(body2: null);
final ThemeData lightTheme = new ThemeData(brightness: Brightness.light, textTheme: noBody2TextTheme);
final ThemeData darkTheme = new ThemeData(brightness: Brightness.dark, textTheme: noBody2TextTheme);
final Typography typography = new Typography(platform: lightTheme.platform);
expect(lightTheme.chipTheme.labelStyle.color, equals(typography.black.body2.color.withAlpha(0xde)));
expect(darkTheme.chipTheme.labelStyle.color, equals(typography.white.body2.color.withAlpha(0xde)));
});
test('Default icon theme contrasts with brightness', () {
final ThemeData lightTheme = new ThemeData(brightness: Brightness.light);
final ThemeData darkTheme = new ThemeData(brightness: Brightness.dark);

23
packages/flutter/test/material/typography_test.dart
View file

@ -23,6 +23,29 @@ void main() {
}
});
test('TextTheme merges properly in the presence of null fields.', () {
const TextTheme partialTheme = const TextTheme(title: const TextStyle(color: const Color(0xcafefeed)));
final TextTheme fullTheme = ThemeData.fallback().textTheme.merge(partialTheme);
expect(fullTheme.title.color, equals(partialTheme.title.color));
const TextTheme onlyHeadlineAndTitle = const TextTheme(
headline: const TextStyle(color: const Color(0xcafefeed)),
title: const TextStyle(color: const Color(0xbeefcafe)),
);
const TextTheme onlyBody1AndTitle = const TextTheme(
body1: const TextStyle(color: const Color(0xfeedfeed)),
title: const TextStyle(color: const Color(0xdeadcafe)),
);
TextTheme merged = onlyHeadlineAndTitle.merge(onlyBody1AndTitle);
expect(merged.body2, isNull);
expect(merged.body1.color, equals(onlyBody1AndTitle.body1.color));
expect(merged.headline.color, equals(onlyHeadlineAndTitle.headline.color));
expect(merged.title.color, equals(onlyBody1AndTitle.title.color));
merged = onlyHeadlineAndTitle.merge(null);
expect(merged, equals(onlyHeadlineAndTitle));
});
test('Typography on Android, Fuchsia defaults to Roboto', () {
expect(new Typography(platform: TargetPlatform.android).black.title.fontFamily, 'Roboto');
expect(new Typography(platform: TargetPlatform.fuchsia).black.title.fontFamily, 'Roboto');