-
Notifications
You must be signed in to change notification settings - Fork 29.3k
Step 1 of 3: Add opt-in for debugCheckHasMaterialLocalizations assertion on TextField #56090
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Step 1 of 3: Add opt-in for debugCheckHasMaterialLocalizations assertion on TextField #56090
Conversation
@HansMuller @justinmc @jonahwilliams This is another (simple?) ToDo resolution. It's slightly breaking, so I'll probably do the same as I did for #55336. Is there any other reason why we should not resolve this? I haven't found any reference in the git history to indicate otherwise. |
I think I added that, saw that it broke a bunch of tests and got scared that it was a breaking change (TM) |
I see no reason not to go forward with this 👍 |
This is ready for review. CI is failing because I have not tagged the deprecation notices with a build, but I'll do that before this lands. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Is there anywhere in our codebase that should be updated to set canAssertMaterialLocalizations
to true right now? Or maybe things like the gallery?
I don't think so. When I initially opened this I set it to true by default to see what would happen. Only that one test broke, I'll double check though before landing this first step in the migration. |
Description
This PR introduces a temporary opt-in flag as the first step of migrating TextField to be able to assert debugCheckHasMaterialLocalizations, which has been disabled for some time. Making the new change opt-in allows developers the opportunity to experience the changes ahead of time so that they can fix any issues with the app resulting from the change or fix any broken tests that do not expect details in the new implementation.
Example migration steps
TextField
instances in app to havecanAssertMaterialLocalizations
be true.Examine all screens with TextField to ensure that Material Localizations are present, make sure existing unit tests are not breaking. If something is broken, fix it. An example of a test that might fail is if a unit test was using a TextField without a MaterialApp or Localizations ancestor. If the issue cannot be fixed, it may not be an intended change, so file an issue to
flutter/flutter
on GitHub.A new PR will be introduced in the future to set
TextField.canAssertMaterialLocalizations
totrue
by default. This will default everybody's TextField to the latest design by default. If developers have properly migrated in step 2, this shouldn't cause any issues to your app. This would be the opportunity to remove thecanAssertMaterialLocalizations
parameter from your application since it is true by default now and it will be removed in a subsequent PR.canAssertMaterialLocalizations
parameter altogether. If you removed thecanAssertMaterialLocalizations
parameter from yourTextField
instances in step 3, this should not affect your application.Tests
Updated remaining test affected by the future default behavior.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze --flutter-repo
) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.