Add a sentinel value for TextStyle.height (#149049)

Fixes: https://github.com/flutter/flutter/issues/58765

The rationale for the choice of the sentinel value: https://github.com/flutter/engine/pull/52940
The exact value of `kTextHeightNone` should be kept as an implementation detail. It's unfortunate that the current value `0` is dangerously close to `TextStyle.height`'s valid domain. If we ever allow `TextStyle.height == 0` (which totally makes sense) then it shouldn't be difficult to change the const.
This commit is contained in:
LongCatIsLooong
2024-05-29 11:24:16 -07:00
committed by GitHub
parent 90937b02eb
commit 557fca4582
4 changed files with 27 additions and 10 deletions

View File

@@ -17,7 +17,7 @@
/// painting boxes.
library painting;
export 'dart:ui' show PlaceholderAlignment, Shadow, TextHeightBehavior, TextLeadingDistribution;
export 'dart:ui' show PlaceholderAlignment, Shadow, TextHeightBehavior, TextLeadingDistribution, kTextHeightNone;
export 'src/painting/alignment.dart';
export 'src/painting/basic_types.dart';

View File

@@ -8,6 +8,7 @@ import 'dart:ui' as ui show
Shadow,
StrutStyle,
TextStyle,
kTextHeightNone,
lerpDouble;
import 'package:flutter/foundation.dart';
@@ -640,14 +641,13 @@ class TextStyle with Diagnosticable {
/// The height of this text span, as a multiple of the font size.
///
/// When [height] is null or omitted, the line height will be determined
/// by the font's metrics directly, which may differ from the fontSize.
/// When [height] is non-null, the line height of the span of text will be a
/// multiple of [fontSize] and be exactly `fontSize * height` logical pixels
/// tall.
/// When [height] is [kTextHeightNone], the line height will be determined by
/// the font's metrics directly, which may differ from the fontSize. Otherwise
/// the line height of the span of text will be a multiple of [fontSize],
/// and be exactly `fontSize * height` logical pixels tall.
///
/// For most fonts, setting [height] to 1.0 is not the same as omitting or
/// setting height to null because the [fontSize] sets the height of the EM-square,
/// For most fonts, setting [height] to 1.0 is not the same as setting height
/// to [kTextHeightNone] because the [fontSize] sets the height of the EM-square,
/// which is different than the font provided metrics for line height. The
/// following diagram illustrates the difference between the font-metrics
/// defined line height and the line height produced with `height: 1.0`
@@ -954,7 +954,8 @@ class TextStyle with Diagnosticable {
/// [TextStyle] with a [FontWeight.w300].
///
/// If the underlying values are null, then the corresponding factors and/or
/// deltas must not be specified.
/// deltas must not be specified. Additionally, if [height] is [kTextHeightNone]
/// it will not be modified by this method.
///
/// If [foreground] is specified on this object, then applying [color] here
/// will have no effect and if [background] is specified on this object, then
@@ -1014,7 +1015,7 @@ class TextStyle with Diagnosticable {
letterSpacing: letterSpacing == null ? null : letterSpacing! * letterSpacingFactor + letterSpacingDelta,
wordSpacing: wordSpacing == null ? null : wordSpacing! * wordSpacingFactor + wordSpacingDelta,
textBaseline: textBaseline ?? this.textBaseline,
height: height == null ? null : height! * heightFactor + heightDelta,
height: (height == null || height == ui.kTextHeightNone) ? height : height! * heightFactor + heightDelta,
leadingDistribution: leadingDistribution ?? this.leadingDistribution,
locale: locale ?? this.locale,
foreground: foreground,

View File

@@ -1696,6 +1696,17 @@ void main() {
);
});
test('kTextHeightNone unsets the text height multiplier', () {
final TextPainter painter = TextPainter(
textDirection: TextDirection.ltr,
text: const TextSpan(
style: TextStyle(fontSize: 10, height: 1000),
children: <TextSpan>[TextSpan(text: 'A', style: TextStyle(height: kTextHeightNone))],
),
)..layout();
expect(painter.height, 10);
});
test('TextPainter dispatches memory events', () async {
await expectLater(
await memoryEvents(() => TextPainter().dispose(), TextPainter),

View File

@@ -570,6 +570,11 @@ void main() {
style.apply(leadingDistribution: TextLeadingDistribution.proportional).leadingDistribution,
TextLeadingDistribution.proportional,
);
expect(
const TextStyle(height: kTextHeightNone).apply(heightFactor: 1000, heightDelta: 1000).height,
kTextHeightNone,
);
});
test('TextStyle fontFamily and package', () {