Unverified Commit 73e1f234 authored by LongCatIsLooong's avatar LongCatIsLooong Committed by GitHub

Remove `textScaleFactor` dependent logic from `AppBar` (#128112)

I am trying to remove `textScaleFactor`-dependent logic from the framework since it's likely going to be deprecated, hopefully the original logic isn't from the material spec.

I stole the sample code from https://github.com/flutter/flutter/pull/125038 and here are the screenshots (`textScaleFactor = 3.0`). 

Internal Tests: **no relevant test failures**

|  Medium  |   Large |
| --------------- | --------------- |
| ![flutter_01](https://github.com/flutter/flutter/assets/31859944/515226d9-716f-470a-b794-5fd14b957d36) | ![flutter_03](https://github.com/flutter/flutter/assets/31859944/475c421d-550e-4d02-be08-709b63b63011) |
parent 257c4eed
...@@ -79,7 +79,7 @@ class _MediumScrollUnderFlexibleConfig with _ScrollUnderFlexibleConfig { ...@@ -79,7 +79,7 @@ class _MediumScrollUnderFlexibleConfig with _ScrollUnderFlexibleConfig {
${textStyle('md.comp.top-app-bar.medium.headline')}?.apply(color: ${color('md.comp.top-app-bar.medium.headline.color')}); ${textStyle('md.comp.top-app-bar.medium.headline')}?.apply(color: ${color('md.comp.top-app-bar.medium.headline.color')});
@override @override
EdgeInsetsGeometry? get expandedTitlePadding => const EdgeInsets.fromLTRB(16, 0, 16, 20); EdgeInsetsGeometry get expandedTitlePadding => const EdgeInsets.fromLTRB(16, 0, 16, 20);
} }
class _LargeScrollUnderFlexibleConfig with _ScrollUnderFlexibleConfig { class _LargeScrollUnderFlexibleConfig with _ScrollUnderFlexibleConfig {
...@@ -102,7 +102,7 @@ class _LargeScrollUnderFlexibleConfig with _ScrollUnderFlexibleConfig { ...@@ -102,7 +102,7 @@ class _LargeScrollUnderFlexibleConfig with _ScrollUnderFlexibleConfig {
${textStyle('md.comp.top-app-bar.large.headline')}?.apply(color: ${color('md.comp.top-app-bar.large.headline.color')}); ${textStyle('md.comp.top-app-bar.large.headline')}?.apply(color: ${color('md.comp.top-app-bar.large.headline.color')});
@override @override
EdgeInsetsGeometry? get expandedTitlePadding => const EdgeInsets.fromLTRB(16, 0, 16, 28); EdgeInsetsGeometry get expandedTitlePadding => const EdgeInsets.fromLTRB(16, 0, 16, 28);
} }
'''; ''';
} }
...@@ -2215,7 +2215,7 @@ class _ChipDefaultsM3 extends ChipThemeData { ...@@ -2215,7 +2215,7 @@ class _ChipDefaultsM3 extends ChipThemeData {
EdgeInsetsGeometry? get padding => const EdgeInsets.all(8.0); EdgeInsetsGeometry? get padding => const EdgeInsets.all(8.0);
/// The chip at text scale 1 starts with 8px on each side and as text scaling /// The chip at text scale 1 starts with 8px on each side and as text scaling
/// gets closer to 2 the label padding is linearly interpolated from 8px to 4px. /// gets closer to 2, the label padding is linearly interpolated from 8px to 4px.
/// Once the widget has a text scaling of 2 or higher than the label padding /// Once the widget has a text scaling of 2 or higher than the label padding
/// remains 4px. /// remains 4px.
@override @override
......
...@@ -62,6 +62,15 @@ TextStyle? iconStyle(WidgetTester tester, IconData icon) { ...@@ -62,6 +62,15 @@ TextStyle? iconStyle(WidgetTester tester, IconData icon) {
return iconRichText.text.style; return iconRichText.text.style;
} }
void _verifyTextNotClipped(Finder textFinder, WidgetTester tester) {
final Rect clipRect = tester.getRect(find.ancestor(of: textFinder, matching: find.byType(ClipRect)).first);
final Rect textRect = tester.getRect(textFinder);
expect(textRect.top, inInclusiveRange(clipRect.top, clipRect.bottom));
expect(textRect.bottom, inInclusiveRange(clipRect.top, clipRect.bottom));
expect(textRect.left, inInclusiveRange(clipRect.left, clipRect.right));
expect(textRect.right, inInclusiveRange(clipRect.left, clipRect.right));
}
double appBarHeight(WidgetTester tester) => tester.getSize(find.byType(AppBar, skipOffstage: false)).height; double appBarHeight(WidgetTester tester) => tester.getSize(find.byType(AppBar, skipOffstage: false)).height;
double appBarTop(WidgetTester tester) => tester.getTopLeft(find.byType(AppBar, skipOffstage: false)).dy; double appBarTop(WidgetTester tester) => tester.getTopLeft(find.byType(AppBar, skipOffstage: false)).dy;
double appBarBottom(WidgetTester tester) => tester.getBottomLeft(find.byType(AppBar, skipOffstage: false)).dy; double appBarBottom(WidgetTester tester) => tester.getBottomLeft(find.byType(AppBar, skipOffstage: false)).dy;
...@@ -1138,7 +1147,9 @@ void main() { ...@@ -1138,7 +1147,9 @@ void main() {
// Test the expanded title is positioned correctly. // Test the expanded title is positioned correctly.
final Offset titleOffset = tester.getBottomLeft(expandedTitle); final Offset titleOffset = tester.getBottomLeft(expandedTitle);
expect(titleOffset.dx, 16.0); expect(titleOffset.dx, 16.0);
expect(titleOffset.dy, closeTo(96.0, 0.1)); expect(titleOffset.dy, 96.0);
_verifyTextNotClipped(expandedTitle, tester);
// Test the expanded title default color. // Test the expanded title default color.
expect( expect(
...@@ -1223,8 +1234,14 @@ void main() { ...@@ -1223,8 +1234,14 @@ void main() {
// Test the expanded title is positioned correctly. // Test the expanded title is positioned correctly.
final Offset titleOffset = tester.getBottomLeft(expandedTitle); final Offset titleOffset = tester.getBottomLeft(expandedTitle);
expect(titleOffset.dx, 16.0); expect(titleOffset.dx, 16.0);
expect(titleOffset.dy, closeTo(128.0, 0.1)); final RenderSliver renderSliverAppBar = tester.renderObject(find.byType(SliverAppBar));
// The expanded title and the bottom padding fits in the flexible space.
expect(
titleOffset.dy,
renderSliverAppBar.geometry!.scrollExtent - 28.0,
reason: 'bottom padding of a large expanded title should be 28.',
);
_verifyTextNotClipped(expandedTitle, tester);
// Test the expanded title default color. // Test the expanded title default color.
expect( expect(
...@@ -4704,13 +4721,16 @@ void main() { ...@@ -4704,13 +4721,16 @@ void main() {
await tester.pumpWidget(buildAppBar()); await tester.pumpWidget(buildAppBar());
final Finder expandedTitle = find.text(title).first; final Finder expandedTitle = find.text(title).first;
expect(tester.getRect(expandedTitle).height, closeTo(31.9, 0.1)); expect(tester.getRect(expandedTitle).height, 32.0);
_verifyTextNotClipped(expandedTitle, tester);
await tester.pumpWidget(buildAppBar(textScaleFactor: 2.0)); await tester.pumpWidget(buildAppBar(textScaleFactor: 2.0));
expect(tester.getRect(expandedTitle).height, closeTo(43.0, 0.1)); expect(tester.getRect(expandedTitle).height, 43.0);
_verifyTextNotClipped(expandedTitle, tester);
await tester.pumpWidget(buildAppBar(textScaleFactor: 3.0)); await tester.pumpWidget(buildAppBar(textScaleFactor: 3.0));
expect(tester.getRect(expandedTitle).height, closeTo(43.0, 0.1)); expect(tester.getRect(expandedTitle).height, 43.0);
_verifyTextNotClipped(expandedTitle, tester);
}); });
testWidgets('SliverAppBar.large expanded title has upper limit on text scaling', (WidgetTester tester) async { testWidgets('SliverAppBar.large expanded title has upper limit on text scaling', (WidgetTester tester) async {
...@@ -4787,24 +4807,16 @@ void main() { ...@@ -4787,24 +4807,16 @@ void main() {
await tester.pumpWidget(buildAppBar()); await tester.pumpWidget(buildAppBar());
final Finder expandedTitle = find.text(title).first; final Finder expandedTitle = find.text(title).first;
Offset titleTop = tester.getTopLeft(expandedTitle); expect(tester.getBottomLeft(expandedTitle).dy, 96.0);
expect(titleTop.dy, 64.0); _verifyTextNotClipped(expandedTitle, tester);
Offset titleBottom = tester.getBottomLeft(expandedTitle);
expect(titleBottom.dy, closeTo(96.0, 0.1));
await tester.pumpWidget(buildAppBar(textScaleFactor: 2.0)); await tester.pumpWidget(buildAppBar(textScaleFactor: 2.0));
expect(tester.getBottomLeft(expandedTitle).dy, 107.0);
titleTop = tester.getTopLeft(expandedTitle); _verifyTextNotClipped(expandedTitle, tester);
expect(titleTop.dy, closeTo(57.0, 0.1));
titleBottom = tester.getBottomLeft(expandedTitle);
expect(titleBottom.dy, closeTo(100.0, 0.1));
await tester.pumpWidget(buildAppBar(textScaleFactor: 3.0)); await tester.pumpWidget(buildAppBar(textScaleFactor: 3.0));
expect(tester.getBottomLeft(expandedTitle).dy, 107.0);
titleTop = tester.getTopLeft(expandedTitle); _verifyTextNotClipped(expandedTitle, tester);
expect(titleTop.dy, closeTo(57.0, 0.1));
titleBottom = tester.getBottomLeft(expandedTitle);
expect(titleBottom.dy, closeTo(100.0, 0.1));
}); });
testWidgets('SliverAppBar.large expanded title position is adjusted with textScaleFactor', (WidgetTester tester) async { testWidgets('SliverAppBar.large expanded title position is adjusted with textScaleFactor', (WidgetTester tester) async {
...@@ -4834,29 +4846,28 @@ void main() { ...@@ -4834,29 +4846,28 @@ void main() {
} }
await tester.pumpWidget(buildAppBar()); await tester.pumpWidget(buildAppBar());
// TODO(tahatesser): https://github.com/flutter/flutter/issues/99933
// A bug in the HTML renderer and/or Chrome 96+ causes a
// discrepancy in the paragraph height.
const bool hasIssue99933 = kIsWeb && !bool.fromEnvironment('FLUTTER_WEB_USE_SKIA');
final Finder expandedTitle = find.text(title).first; final Finder expandedTitle = find.text(title).first;
Offset titleTop = tester.getTopLeft(expandedTitle); final RenderSliver renderSliverAppBar = tester.renderObject(find.byType(SliverAppBar));
expect(titleTop.dy, closeTo(hasIssue99933 ? 91.0 : 92.0, 0.1)); expect(
Offset titleBottom = tester.getBottomLeft(expandedTitle); tester.getBottomLeft(expandedTitle).dy,
expect(titleBottom.dy, closeTo(128.0, 0.1)); renderSliverAppBar.geometry!.scrollExtent - 28.0,
reason: 'bottom padding of a large expanded title should be 28.',
);
_verifyTextNotClipped(expandedTitle, tester);
await tester.pumpWidget(buildAppBar(textScaleFactor: 2.0)); await tester.pumpWidget(buildAppBar(textScaleFactor: 2.0));
expect(
tester.getBottomLeft(expandedTitle).dy,
renderSliverAppBar.geometry!.scrollExtent - 28.0,
reason: 'bottom padding of a large expanded title should be 28.',
);
_verifyTextNotClipped(expandedTitle, tester);
titleTop = tester.getTopLeft(expandedTitle); // The bottom padding of the expanded title needs to be reduced for it to be
expect(titleTop.dy, closeTo(86.1, 0.1)); // fully visible.
titleBottom = tester.getBottomLeft(expandedTitle);
expect(titleBottom.dy, closeTo(134.1, 0.1));
await tester.pumpWidget(buildAppBar(textScaleFactor: 3.0)); await tester.pumpWidget(buildAppBar(textScaleFactor: 3.0));
expect(tester.getBottomLeft(expandedTitle).dy, 124.0);
titleTop = tester.getTopLeft(expandedTitle); _verifyTextNotClipped(expandedTitle, tester);
expect(titleTop.dy, closeTo(86.1, 0.1));
titleBottom = tester.getBottomLeft(expandedTitle);
expect(titleBottom.dy, closeTo(134.1, 0.1));
}); });
group('AppBar.forceMaterialTransparency', () { group('AppBar.forceMaterialTransparency', () {
......
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