Skip to content

F
Front-End
Unverified Commit 6a7baf57 authored by Bruno Leroux's avatar Bruno Leroux Committed by GitHub
Browse files
Options

Fix M3 text field height + initial step for input decorator M3 test migration (#142981) 

## Description

This PR main purpose is to make progress on the M3 test migration for `InputDecorator` (see https://github.com/flutter/flutter/issues/139076).

Before this PR more than 80 of the 156 tests defined in `input_decorator_test.dart` fail on M3.
Migrating all those tests in one shot is not easy at all because many failures are related to wrong positionning due to M3 typography changes. Another reason is that several M3 specific changes are required in order to get a proper M3 compliant text field, for instance:
- https://github.com/flutter/flutter/issues/142972
- https://github.com/flutter/flutter/issues/141354

Most of the tests were relying on an helper function (`buildInputDecorator`) which had a `useMaterial3` parameter. Unfortunately when `useMaterial3: true `was passed to this function it forced `useMaterial3: false` at the top level but overrided it at a lower level, which was very misleading because people could assume that the tests are ok with M3 (but in fact they were run using M2 typography but have some M3 logic in use).
I considered various way to make this change and I finally decided to run all existing tests only on M2 for the moment. Next step will be to move most of those tests to M3. In this PR, I migrated two of these existing tests for illustration.

Because many of the existing tests are checking input decorator height, I think it would also make sense to fix https://github.com/flutter/flutter/issues/142972 first. That's why I choosed to include a fix to https://github.com/flutter/flutter/issues/142972 in this PR.

A M3 filled `TextField` on Android:

| Before this PR | After this PR |
|--------|--------|
| ![image](https://github.com/flutter/flutter/assets/840911/403225b7-4c91-4aee-b19c-0490447ae7e3) | ![image](https://github.com/flutter/flutter/assets/840911/e96cf786-a9f5-4e15-bcdd-078350ff1608) | 

## Related Issue

Fixes https://github.com/flutter/flutter/issues/142972
Related to https://github.com/flutter/flutter/issues/139076

## Tests

Updates many existing tests 
+ adds 2 tests related to the fix for https://github.com/flutter/flutter/issues/142972
+ adds 1 tests for the M3 migration
+ move 1 tests related to M3
parent c539ded6
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 402 additions and 441 deletions
examples/api/test/material/input_decorator/input_decoration.prefix_icon.0_test.dart
View file @ 6a7baf57
......@@ -11,6 +11,6 @@ void main() {
await tester.pumpWidget(
const example.PrefixIconExampleApp(),
);
expect(tester.getCenter(find.byIcon(Icons.person)).dy, 32.0);
expect(tester.getCenter(find.byIcon(Icons.person)).dy, 28.0);
});
}
examples/api/test/material/input_decorator/input_decoration.suffix_icon.0_test.dart
View file @ 6a7baf57
......@@ -11,6 +11,6 @@ void main() {
await tester.pumpWidget(
const example.SuffixIconExampleApp(),
);
expect(tester.getCenter(find.byIcon(Icons.remove_red_eye)).dy, 32.0);
expect(tester.getCenter(find.byIcon(Icons.remove_red_eye)).dy, 28.0);
});
}
packages/flutter/lib/src/material/input_decorator.dart
View file @ 6a7baf57
......@@ -2393,30 +2393,42 @@ class _InputDecoratorState extends State<InputDecorator> with TickerProviderStat
final EdgeInsets contentPadding;
final double floatingLabelHeight;
if (decoration.isCollapsed
?? themeData.inputDecorationTheme.isCollapsed) {
if (decoration.isCollapsed ?? themeData.inputDecorationTheme.isCollapsed) {
floatingLabelHeight = 0.0;
contentPadding = decorationContentPadding ?? EdgeInsets.zero;
} else if (!border.isOutline) {
// 4.0: the vertical gap between the inline elements and the floating label.
floatingLabelHeight = MediaQuery.textScalerOf(context).scale(4.0 + 0.75 * labelStyle.fontSize!);
if (decoration.filled ?? false) {
contentPadding = decorationContentPadding ?? (decorationIsDense
? const EdgeInsets.fromLTRB(12.0, 8.0, 12.0, 8.0)
: const EdgeInsets.fromLTRB(12.0, 12.0, 12.0, 12.0));
contentPadding = decorationContentPadding ?? (Theme.of(context).useMaterial3
? decorationIsDense
? const EdgeInsets.fromLTRB(12.0, 4.0, 12.0, 4.0)
: const EdgeInsets.fromLTRB(12.0, 8.0, 12.0, 8.0)
: decorationIsDense
? const EdgeInsets.fromLTRB(12.0, 8.0, 12.0, 8.0)
: const EdgeInsets.fromLTRB(12.0, 12.0, 12.0, 12.0));
} else {
// Not left or right padding for underline borders that aren't filled
// No left or right padding for underline borders that aren't filled
// is a small concession to backwards compatibility. This eliminates
// the most noticeable layout change introduced by #13734.
contentPadding = decorationContentPadding ?? (decorationIsDense
? const EdgeInsets.fromLTRB(0.0, 8.0, 0.0, 8.0)
: const EdgeInsets.fromLTRB(0.0, 12.0, 0.0, 12.0));
contentPadding = decorationContentPadding ?? (Theme.of(context).useMaterial3
? decorationIsDense
? const EdgeInsets.fromLTRB(0.0, 4.0, 0.0, 4.0)
: const EdgeInsets.fromLTRB(0.0, 8.0, 0.0, 8.0)
: decorationIsDense
? const EdgeInsets.fromLTRB(0.0, 8.0, 0.0, 8.0)
: const EdgeInsets.fromLTRB(0.0, 12.0, 0.0, 12.0));
}
} else {
floatingLabelHeight = 0.0;
contentPadding = decorationContentPadding ?? (decorationIsDense
? const EdgeInsets.fromLTRB(12.0, 20.0, 12.0, 12.0)
: const EdgeInsets.fromLTRB(12.0, 24.0, 12.0, 16.0));
contentPadding = decorationContentPadding ?? (Theme.of(context).useMaterial3
? decorationIsDense
? const EdgeInsets.fromLTRB(12.0, 16.0, 12.0, 8.0)
: const EdgeInsets.fromLTRB(12.0, 20.0, 12.0, 12.0)
: decorationIsDense
? const EdgeInsets.fromLTRB(12.0, 20.0, 12.0, 12.0)
: const EdgeInsets.fromLTRB(12.0, 24.0, 12.0, 16.0));
}
final _Decorator decorator = _Decorator(
......
packages/flutter/test/material/dropdown_form_field_test.dart
View file @ 6a7baf57
......@@ -234,7 +234,7 @@ void main() {
expect(value, null); // disabledHint shown.
final Offset hintEmptyLabel = tester.getTopLeft(find.text('labelText'));
expect(hintEmptyLabel, const Offset(0.0, 12.0));
expect(hintEmptyLabel, const Offset(0.0, 8.0));
});
testWidgets('label position test - show disabledHint: enable + null item', (WidgetTester tester) async {
......@@ -259,7 +259,7 @@ void main() {
expect(value, null); // disabledHint shown.
final Offset hintEmptyLabel = tester.getTopLeft(find.text('labelText'));
expect(hintEmptyLabel, const Offset(0.0, 12.0));
expect(hintEmptyLabel, const Offset(0.0, 8.0));
});
testWidgets('label position test - show disabledHint: enable + empty item', (WidgetTester tester) async {
......@@ -284,7 +284,7 @@ void main() {
expect(value, null); // disabledHint shown.
final Offset hintEmptyLabel = tester.getTopLeft(find.text('labelText'));
expect(hintEmptyLabel, const Offset(0.0, 12.0));
expect(hintEmptyLabel, const Offset(0.0, 8.0));
});
testWidgets('label position test - show hint: enable + empty item', (WidgetTester tester) async {
......@@ -309,7 +309,7 @@ void main() {
expect(value, null); // hint shown.
final Offset hintEmptyLabel = tester.getTopLeft(find.text('labelText'));
expect(hintEmptyLabel, const Offset(0.0, 12.0));
expect(hintEmptyLabel, const Offset(0.0, 8.0));
});
testWidgets('label position test - no hint shown: enable + no selected + disabledHint', (WidgetTester tester) async {
......@@ -347,7 +347,7 @@ void main() {
expect(value, null);
final Offset hintEmptyLabel = tester.getTopLeft(find.text('labelText'));
expect(hintEmptyLabel, const Offset(0.0, 24.0));
expect(hintEmptyLabel, const Offset(0.0, 20.0));
});
testWidgets('label position test - show selected item: disabled + hint + disabledHint', (WidgetTester tester) async {
......@@ -386,7 +386,7 @@ void main() {
expect(value, 1);
final Offset hintEmptyLabel = tester.getTopLeft(find.text('labelText'));
expect(hintEmptyLabel, const Offset(0.0, 12.0));
expect(hintEmptyLabel, const Offset(0.0, 8.0));
});
// Regression test for https://github.com/flutter/flutter/issues/82910
......@@ -603,7 +603,7 @@ void main() {
final RenderBox box =
tester.renderObject<RenderBox>(find.byType(dropdownButtonType));
expect(box.size.height, 72.0);
expect(box.size.height, 64.0);
});
testWidgets('DropdownButtonFormField.isDense is true by default', (WidgetTester tester) async {
......
packages/flutter/test/material/dropdown_test.dart
View file @ 6a7baf57
......@@ -2408,11 +2408,11 @@ void main() {
await tester.pumpWidget(buildFrame(isFormField: true, buttonKey: buttonKey, onChanged: onChanged, focusNode: focusNode, autofocus: true));
await tester.pumpAndSettle(); // Pump a frame for autofocus to take effect.
expect(focusNode.hasPrimaryFocus, isTrue);
expect(find.byType(Material), paints ..rect(rect: const Rect.fromLTRB(0.0, 264.0, 800.0, 336.0), color: const Color(0x1f000000)));
expect(find.byType(Material), paints ..rect(rect: const Rect.fromLTRB(0.0, 268.0, 800.0, 332.0), color: const Color(0x1f000000)));
await tester.pumpWidget(buildFrame(isFormField: true, buttonKey: buttonKey, onChanged: onChanged, focusNode: focusNode, focusColor: const Color(0xff00ff00)));
await tester.pumpAndSettle(); // Pump a frame for autofocus to take effect.
expect(find.byType(Material), paints ..rect(rect: const Rect.fromLTRB(0.0, 264.0, 800.0, 336.0), color: const Color(0x1f00ff00)));
expect(find.byType(Material), paints ..rect(rect: const Rect.fromLTRB(0.0, 268.0, 800.0, 332.0), color: const Color(0x1f00ff00)));
});
testWidgets("DropdownButton won't be focused if not enabled", (WidgetTester tester) async {
......
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

HIAST - GitLab