Unverified Commit 5d4f2402 authored by LongCatIsLooong's avatar LongCatIsLooong Committed by GitHub

Add error message when lerping between TextStyles different `inherit` values (#112837)

parent 8b7a6e4f
......@@ -507,12 +507,27 @@ class TextStyle with Diagnosticable {
assert(backgroundColor == null || background == null, _kColorBackgroundWarning);
/// Whether null values are replaced with their value in an ancestor text
/// style (e.g., in a [TextSpan] tree).
/// Whether null values in this [TextStyle] can be replaced with their value
/// in another [TextStyle] using [merge].
///
/// If this is false, properties that don't have explicit values will revert
/// to the defaults: white in color, a font size of 14 pixels, in a sans-serif
/// font face.
/// The [merge] operation is not commutative: the [inherit] value of the
/// method argument decides whether the two [TextStyle]s can be combined
/// together. If it is false, the method argument [TextStyle] will be returned.
/// Otherwise, the combining is allowed, and the returned [TextStyle] inherits
/// the [inherit] value from the method receiver.
///
/// This property has no effect on [TextSpan]'s text style cascading: in a
/// [TextSpan] tree, a [TextSpan]'s text style can be combined with that of an
/// ancestor [TextSpan] if it has unspecified fields, regardless of its
/// [inherit] value.
///
/// Properties that don't have explicit values or other default values to fall
/// back to will revert to the defaults: white in color, a font size of 14
/// pixels, in a sans-serif font face.
///
/// See also:
/// * [TextStyle.merge], which can be used to combine properties from two
/// [TextStyle]s.
final bool inherit;
/// The color to use when painting the text.
......@@ -572,7 +587,7 @@ class TextStyle with Diagnosticable {
/// prefixed with 'packages/package_name/' (e.g. 'packages/cool_fonts/Roboto').
/// The package name should be provided by the `package` argument in the
/// constructor.
List<String>? get fontFamilyFallback => _package != null && _fontFamilyFallback != null ? _fontFamilyFallback!.map((String str) => 'packages/$_package/$str').toList() : _fontFamilyFallback;
List<String>? get fontFamilyFallback => _package == null ? _fontFamilyFallback : _fontFamilyFallback?.map((String str) => 'packages/$_package/$str').toList();
final List<String>? _fontFamilyFallback;
// This is stored in order to prefix the fontFamilies in _fontFamilyFallback
......@@ -801,10 +816,10 @@ class TextStyle with Diagnosticable {
// Return the original value of fontFamily, without the additional
// "packages/$_package/" prefix.
String? get _fontFamily {
if (_package != null && fontFamily != null) {
if (_package != null) {
final String fontFamilyPrefix = 'packages/$_package/';
assert(fontFamily!.startsWith(fontFamilyPrefix));
return fontFamily!.substring(fontFamilyPrefix.length);
assert(fontFamily?.startsWith(fontFamilyPrefix) ?? true);
return fontFamily?.substring(fontFamilyPrefix.length);
}
return fontFamily;
}
......@@ -1065,9 +1080,19 @@ class TextStyle with Diagnosticable {
);
}
/// Interpolate between two text styles.
/// Interpolate between two text styles for animated transitions.
///
/// Interpolation will not work well if the styles don't specify the same fields.
/// When this happens, to keep the interpolated transition smooth, the
/// implementation uses the non-null value throughout the transition for
/// lerpable fields such as colors (for example, if one [TextStyle] specified
/// `fontSize` but the other didn't, the returned [TextStyle] will use the
/// `fontSize` from the [TextStyle] that specified it, regarless of the `t`
/// value).
///
/// This will not work well if the styles don't set the same fields.
/// This method throws when the given [TextStyle]s don't have the same
/// [inherit] value and a lerpable field is missing from both [TextStyle]s,
/// as that could result in jumpy transitions.
///
/// {@macro dart.ui.shadow.lerp}
///
......@@ -1080,7 +1105,6 @@ class TextStyle with Diagnosticable {
/// based on the [backgroundColor] property).
static TextStyle? lerp(TextStyle? a, TextStyle? b, double t) {
assert(t != null);
assert(a == null || b == null || a.inherit == b.inherit);
if (a == null && b == null) {
return null;
}
......@@ -1153,8 +1177,70 @@ class TextStyle with Diagnosticable {
);
}
assert(() {
if (a.inherit == b.inherit) {
return true;
}
final List<String> nullFields = <String>[
if (a.foreground == null && b.foreground == null && a.color == null && b.color == null) 'color',
if (a.background == null && b.background == null && a.backgroundColor == null && b.backgroundColor == null) 'backgroundColor',
if (a.fontSize == null && b.fontSize == null) 'fontSize',
if (a.letterSpacing == null && b.letterSpacing == null) 'letterSpacing',
if (a.wordSpacing == null && b.wordSpacing == null) 'wordSpacing',
if (a.height == null && b.height == null) 'height',
if (a.decorationColor == null && b.decorationColor == null) 'decorationColor',
if (a.decorationThickness == null && b.decorationThickness == null) 'decorationThickness',
];
if (nullFields.isEmpty) {
return true;
}
throw FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary('Failed to interpolate TextStyles with different inherit values.'),
ErrorSpacer(),
ErrorDescription('The TextStyles being interpolated were:'),
a.toDiagnosticsNode(name: 'from', style: DiagnosticsTreeStyle.singleLine),
b.toDiagnosticsNode(name: 'to', style: DiagnosticsTreeStyle.singleLine),
ErrorDescription(
'The following fields are unspecified in both TextStyles:\n'
'${nullFields.map((String name) => '"$name"').join(', ')}.\n'
'When "inherit" changes during the transition, these fields may '
'observe abrupt value changes as a result, causing "jump"s in the '
'transition.'
),
ErrorSpacer(),
ErrorHint(
'In general, TextStyle.lerp only works well when both TextStyles have '
'the same "inherit" value, and specify the same fields.',
),
ErrorHint(
'If the TextStyles were directly created by you, consider bringing '
'them to parity to ensure a smooth transition.'
),
ErrorSpacer(),
ErrorHint(
'If one of the TextStyles being lerped is significantly more elaborate '
'than the other, and has "inherited" set to false, it is often because '
'it is merged with another TextStyle before being lerped. Comparing '
'the "debugLabel"s of the two TextStyles may help identify if that was '
'the case.'
),
ErrorHint(
'For example, you may see this error message when trying to lerp '
'between "ThemeData()" and "Theme.of(context)". This is because '
'TextStyles from "Theme.of(context)" are merged with TextStyles from '
'another theme and thus are more elaborate than the TextStyles from '
'"ThemeData()" (which is reflected in their "debugLabel"s -- '
'TextStyles from "Theme.of(context)" should have labels in the form of '
'"(<A TextStyle>).merge(<Another TextStyle>)"). It is recommended to '
'only lerp ThemeData with matching TextStyles.'
),
]);
}());
return TextStyle(
inherit: b.inherit,
inherit: t < 0.5 ? a.inherit : b.inherit,
color: a.foreground == null && b.foreground == null ? Color.lerp(a.color, b.color, t) : null,
backgroundColor: a.background == null && b.background == null ? Color.lerp(a.backgroundColor, b.backgroundColor, t) : null,
fontSize: ui.lerpDouble(a.fontSize ?? b.fontSize, b.fontSize ?? a.fontSize, t),
......@@ -1354,35 +1440,43 @@ class TextStyle with Diagnosticable {
}
@override
int get hashCode => Object.hash(
inherit,
color,
backgroundColor,
fontSize,
fontWeight,
fontStyle,
letterSpacing,
wordSpacing,
textBaseline,
height,
leadingDistribution,
locale,
foreground,
background,
shadows == null ? null : Object.hashAll(shadows!),
fontFeatures == null ? null : Object.hashAll(fontFeatures!),
fontVariations == null ? null : Object.hashAll(fontVariations!),
decoration,
decorationColor,
Object.hash(
int get hashCode {
final List<String>? fontFamilyFallback = this.fontFamilyFallback;
final int fontHash = Object.hash(
decorationStyle,
decorationThickness,
fontFamily,
fontFamilyFallback == null ? null : Object.hashAll(fontFamilyFallback!),
fontFamilyFallback == null ? null : Object.hashAll(fontFamilyFallback),
_package,
overflow,
),
);
);
final List<ui.Shadow>? shadows = this.shadows;
final List<ui.FontFeature>? fontFeatures = this.fontFeatures;
final List<ui.FontVariation>? fontVariations = this.fontVariations;
return Object.hash(
inherit,
color,
backgroundColor,
fontSize,
fontWeight,
fontStyle,
letterSpacing,
wordSpacing,
textBaseline,
height,
leadingDistribution,
locale,
foreground,
background,
shadows == null ? null : Object.hashAll(shadows),
fontFeatures == null ? null : Object.hashAll(fontFeatures),
fontVariations == null ? null : Object.hashAll(fontVariations),
decoration,
decorationColor,
fontHash,
);
}
@override
String toStringShort() => objectRuntimeType(this, 'TextStyle');
......
......@@ -540,4 +540,31 @@ void main() {
expect(const TextStyle().apply(fontFamily: 'fontFamily', package: 'foo').fontFamily, 'packages/foo/fontFamily');
expect(const TextStyle(fontFamily: 'fontFamily', package: 'foo').apply(fontFamily: 'fontFamily', package: 'bar').fontFamily, 'packages/bar/fontFamily');
});
test('Throws when lerping between inherit:true and inherit:false with unspecified fields', () {
const TextStyle fromStyle = TextStyle();
const TextStyle toStyle = TextStyle(inherit: false);
expect(
() => TextStyle.lerp(fromStyle, toStyle, 0.5),
throwsFlutterError,
);
expect(TextStyle.lerp(fromStyle, fromStyle, 0.5), fromStyle);
});
test('Does not throw when lerping between inherit:true and inherit:false but fully specified styles', () {
const TextStyle fromStyle = TextStyle();
const TextStyle toStyle = TextStyle(
inherit: false,
color: Color(0x87654321),
backgroundColor: Color(0x12345678),
fontSize: 20,
letterSpacing: 1,
wordSpacing: 1,
height: 20,
decorationColor: Color(0x11111111),
decorationThickness: 5,
);
expect(TextStyle.lerp(fromStyle, toStyle, 1), toStyle);
});
}
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