Normalize input decoration theme#168981
Conversation
github-actions
bot
added
a: text input
|
@QuncCccccc This PR sets Because, |
bleroux
force-pushed
the
input_decoration_theme_normalization
branch
from
7e4398b to
520a3d9
Compare
|
I think we can leave it for now until we decided to clean up the properties in |
bleroux
force-pushed
the
input_decoration_theme_normalization
branch
2 times, most recently
from
e87d275 to
02b9d21
Compare
QuncCccccc
left a comment
QuncCccccc
left a comment
There was a problem hiding this comment.
Overall looks good to me! Thanks so much for working on this big theme! Just left some comments, please let me know if you have any thoughts on them:)
| /// Overrides the [InputDatePickerFormField]'s input decoration theme. | ||
| /// If this is null, [ThemeData.inputDecorationTheme] is used instead. | ||
| final InputDecorationTheme? inputDecorationTheme; | ||
| final InputDecorationThemeData? inputDecorationTheme; |
There was a problem hiding this comment.
I'm actually not sure if we should change the parameter type here because it may introduce more breaking changes. Do you think if it would be better to change the type when we are really removing the properties in InputDecorationTheme?
Similar for other widgets, just feel these public apis may cause too many breaking changes.
There was a problem hiding this comment.
Makes sense. I reverted this changes and just kept the ThemeData.inputDecorationTheme change.
There was a problem hiding this comment.
I was adding g3fix for this PR and noticed for this property and DropdownMenu.inputDecorationTheme, several usages are: ThemeData.inputDecorationTheme.copyWith() so with the changes in PR, breaking changes happened. Do you think if we should change both type to Object so the commonly-used cases can be accepted? Also once the normalization is done, we actually should deprecate these properties because at that time, developers can wrap the widget with InputDecorationTheme.
There was a problem hiding this comment.
I changed both types to Object? in the last commit, seems reasonable.
Let me know if it helps with g3 fix.
bleroux
force-pushed
the
input_decoration_theme_normalization
branch
7 times, most recently
from
673b58e to
06d1336
Compare
bleroux
force-pushed
the
input_decoration_theme_normalization
branch
4 times, most recently
from
a08dca9 to
6d5b2f4
Compare
bleroux
force-pushed
the
input_decoration_theme_normalization
branch
2 times, most recently
from
80609ec to
32f1bd0
Compare
QuncCccccc
left a comment
QuncCccccc
left a comment
There was a problem hiding this comment.
Overall looks great! Thank you!
| @@ -41,7 +41,7 @@ const String threeLines = 'line1\nline2\nline3'; | |||
| Widget buildInputDecorator({ | |||
There was a problem hiding this comment.
Can we add a test to check if a local InputDecorationTheme can override the themedata.inputDecortationTheme?
There was a problem hiding this comment.
Oh, I missed that. It will required several changes as the current implementation as many references to Theme.of(context).inputDecorationTheme.
For the test, I will create a group because there are many properties and combinations to test.
There was a problem hiding this comment.
I added a group to test that most of the properties can be overridden with a local theme.
Some properties are missing because there are no M3 tests for them. To make it easier to track this work, I will filed other PRs to add the missing M3 tests and, when needed, fix the implementation if it does not use the local theme properly.
There was a problem hiding this comment.
Thank you!
Some properties are missing because there are no M3 tests for them. To make it easier to track this work, I will filed other PRs to add the missing M3 tests
We don't need to add the M3 tests if M3 didn't specify the property in defaults. It would be fine as long as we can make sure the properties works when we use them in a local InputDecorationThemeData.
when needed, fix the implementation if it does not use the local theme properly.
Does this mean that some properties don't work in a InputDecorationThemeData? If so, maybe we should not add them in InputDecorationThemeData.
There was a problem hiding this comment.
Migrating InputDecoration tests to M3 is still a work in a progress. This is why some properties are missing existing M3 tests.
To correctly check that the local theme properly work for those properties I would first have to add tests that check if the general theme is working for those properties (hopefully most of them are probably ok, otherwise we would have get some reports).
My problem is that I prefer to do that in separate PRs as it becomes difficult to track changes in this PR, and It would be more revert proof.
There was a problem hiding this comment.
Ah I see. Yeah, separating PRs makes sense to me!
bleroux
force-pushed
the
input_decoration_theme_normalization
branch
from
32f1bd0 to
4e17bec
Compare
## Description This PR adds tests for `InputDecorationThemeData.floatingLabelBehavior`, `InputDecorationThemeData.floatingLabelAlignment`, and `InputDecorationThemeData.contentPadding`. ## Related Issue Related to #168981 ## Tests Adds 3 tests, moves one.
) ## Description This PR is similar to what was done for `DatePickerThemeData` in #168981. It changes `TimePickerThemeData.inputDecorationTheme` type to `InputDecorationThemeData` (instead of `InputDecorationTheme`) and uses Object? for the corresponding constructor parameter. ## Tests Adds 1 test
…ter#171584) ## Description This PR is similar to what was done for `DatePickerThemeData` in flutter#168981. It changes `TimePickerThemeData.inputDecorationTheme` type to `InputDecorationThemeData` (instead of `InputDecorationTheme`) and uses Object? for the corresponding constructor parameter. ## Tests Adds 1 test
This PR is to make `InputDecorationTheme` conform to Flutter Material's conventions for component themes: - Added a `InputDecorationThemeData` class which defines overrides for the defaults for `InputDecorator` properties. - Added `InputDecorationTheme` constructor parameters: `InputDecorationThemeData? data` and `Widget? child`. This is now the preferred way to configure a `InputDecorationTheme`: ```dart InputDecorationTheme( data: InputDecorationThemeData( filled: true, fillColor: Colors.amber, ... ), child: const TextField() ) ``` These two properties are made nullable to not break existing apps which has customized `ThemeData.inputDecorationTheme`. - Update `InputDecorationTheme` to be an `InheritedTheme` subclass. - Changed the type of component theme defaults from `InputDecorationTheme` to `InputDecorationThemeData`. - Changed the `InputDecorationTheme bottomAppBarTheme` property to `Object? bottomAppBarTheme` in `ThemeData` and `ThemeData.copyWith()` (Object? is used for the moment to minimize Google tests failure. A follow-up PR will replace `Object?` with `InputDecorationThemeData`. - Addresses the "theme normalization" sub-project within flutter#91772. A migration guide will be created on website repo.
…70905) ## Description This PR fixes `InputDecoration.floatingLabelBehavior` logic to query ambient InputDecorationTheme.floatingLabelBehavior, previously it was ignored. ## Related Issue Fixes [InputDecorationTheme and IconTheme isn't fully inherited](flutter#71813) Will help to complete flutter#168981 ## Tests Adds 1 test
…lutter#170905)" (flutter#170994) <!-- start_original_pr_link --> Reverts: flutter#170905 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: goderbauer <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: Analyzer failure: `The argument type 'InputDecorationTheme' can't be assigned to the parameter type 'InputDecorationThemeData?'. • packages/flutter/test/material/input_decorator_test.dart:2959:33 • argument_type_not_assignable` <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: bleroux <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {justinmc} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: ## Description This PR fixes `InputDecoration.floatingLabelBehavior` logic to query ambient InputDecorationTheme.floatingLabelBehavior, previously it was ignored. ## Related Issue Fixes [InputDecorationTheme and IconTheme isn't fully inherited](flutter#71813) Will help to complete flutter#168981 ## Tests Adds 1 test <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
…lutter#170995) ## Description Reland flutter#170905 which was reverted in flutter#170994 The change from flutter#170905 had to be adjusted with a change which landed just before (flutter#168981).
## Description This PR adds missing M3 tests for `InputDecoration.isDense`. ## Related Issue Related to flutter#168981 ## Tests Adds 4 tests.
…ter#170903) ## Description This PR adds missing M3 tests for InputDecoration.floatingLabelAlignment. ## Related Issue Will help to complete flutter#168981 ## Tests Adds 8 tests (based on existing M2 tests).
) ## Description This PR adds tests for `InputDecorationThemeData.floatingLabelBehavior`, `InputDecorationThemeData.floatingLabelAlignment`, and `InputDecorationThemeData.contentPadding`. ## Related Issue Related to flutter#168981 ## Tests Adds 3 tests, moves one.
…ter#171584) ## Description This PR is similar to what was done for `DatePickerThemeData` in flutter#168981. It changes `TimePickerThemeData.inputDecorationTheme` type to `InputDecorationThemeData` (instead of `InputDecorationTheme`) and uses Object? for the corresponding constructor parameter. ## Tests Adds 1 test
…ter#171584) ## Description This PR is similar to what was done for `DatePickerThemeData` in flutter#168981. It changes `TimePickerThemeData.inputDecorationTheme` type to `InputDecorationThemeData` (instead of `InputDecorationTheme`) and uses Object? for the corresponding constructor parameter. ## Tests Adds 1 test
flutter/flutter@c7362b4...0ab008a 2025-06-22 [email protected] Roll Skia from 203469ef4672 to fcd1c55da9cc (1 revision) (flutter/flutter#170986) 2025-06-22 [email protected] Roll Dart SDK from e5db5e0f81ba to 98db1db5ff65 (2 revisions) (flutter/flutter#170979) 2025-06-21 [email protected] Normalize input decoration theme (flutter/flutter#168981) 2025-06-21 [email protected] Roll Skia from dceb857b1c47 to 203469ef4672 (1 revision) (flutter/flutter#170959) 2025-06-21 [email protected] Roll Dart SDK from 6325aeacaef4 to e5db5e0f81ba (1 revision) (flutter/flutter#170958) 2025-06-21 [email protected] Roll Dart SDK from a6a3d65e33d7 to 6325aeacaef4 (2 revisions) (flutter/flutter#170955) 2025-06-21 [email protected] Roll Skia from 773188560221 to dceb857b1c47 (1 revision) (flutter/flutter#170954) 2025-06-21 [email protected] Roll Dart SDK from 750eaa028445 to a6a3d65e33d7 (1 revision) (flutter/flutter#170953) 2025-06-21 [email protected] Fix the Japanese IME problem on macOS reported in the following issue. (flutter/flutter#166291) 2025-06-21 [email protected] Roll Fuchsia Test Scripts from IfPORaYjQ2zP4bcq-... to 2wEtX_lDNhHhDNsP6... (flutter/flutter#170946) 2025-06-20 [email protected] Roll Skia from 104460420c48 to 773188560221 (2 revisions) (flutter/flutter#170942) 2025-06-20 [email protected] Normalize AppBarTheme (flutter/flutter#169130) 2025-06-20 [email protected] Close CupertinoContextMenu overlay if the widget is disposed or a new route is pushed (flutter/flutter#170186) 2025-06-20 [email protected] Roll Dart SDK from a554bdd0a2cc to 750eaa028445 (1 revision) (flutter/flutter#170933) 2025-06-20 [email protected] Roll Skia from b638003de37e to 104460420c48 (1 revision) (flutter/flutter#170932) 2025-06-20 [email protected] Roll Packages from 0ec4053 to 7f41e75 (1 revision) (flutter/flutter#170925) 2025-06-20 [email protected] Roll Skia from f1e68950ea7b to b638003de37e (5 revisions) (flutter/flutter#170923) 2025-06-20 [email protected] [licenses_cpp] jun17 (flutter/flutter#170845) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…ter#171584) ## Description This PR is similar to what was done for `DatePickerThemeData` in flutter#168981. It changes `TimePickerThemeData.inputDecorationTheme` type to `InputDecorationThemeData` (instead of `InputDecorationTheme`) and uses Object? for the corresponding constructor parameter. ## Tests Adds 1 test
## Description This PR replaces global `ThemeData.inputDecorationTheme` usage in `TextFormField` with `InputDecorationTheme.of ` which returns the ambient `InputDecorationTheme`. It is a follow up to #168981 which introduces `InputDecorationTheme.of `. ## Related Issue Fixes [TextFormField does not inherit local InputDecorationTheme](#176391) ## Tests - Adds 1 test
2 participants
This PR is to make
InputDecorationThemeconform to Flutter Material's conventions for component themes:InputDecorationThemeDataclass which defines overrides for the defaults forInputDecoratorproperties.InputDecorationThemeconstructor parameters:InputDecorationThemeData? dataandWidget? child. This is now the preferred way to configure aInputDecorationTheme:These two properties are made nullable to not break existing apps which has customized
ThemeData.inputDecorationTheme.InputDecorationThemeto be anInheritedThemesubclass.InputDecorationThemetoInputDecorationThemeData.InputDecorationTheme bottomAppBarThemeproperty toObject? bottomAppBarThemeinThemeDataandThemeData.copyWith()(Object? is used for the moment to minimize Google tests failure. A follow-up PR will replaceObject?withInputDecorationThemeData.A migration guide will be created on website repo.