-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Fix InputDecoration does not apply errorStyle to error #174787
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
Fix InputDecoration does not apply errorStyle to error #174787
Conversation
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.
Code Review
This pull request correctly fixes an issue where InputDecoration.errorStyle was not applied to the InputDecoration.error widget. The implementation is sound and a regression test has been added to verify the fix. My feedback includes a suggestion to enhance the test coverage by adding a test case for custom errorStyle to ensure all scenarios are covered.
2de498c to
5281771
Compare
ValentinVignal
left a comment
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
| buildInputDecorator(decoration: const InputDecoration(error: Text(errorText))), | ||
| ); | ||
|
|
||
| expect(findError(), findsOneWidget); |
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.
Reading the documentation of findsOneWidget, it is written that
This is equivalent to the preferred [findsOne] method.
Should we be using findsOne instead of findsOneWidget?
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.
Should we be using findsOne instead of findsOneWidget?
Interesting question!
Searching through the codebase:
- findsOne: 8218
- findsOneWidget: 7610
In input_decorator_test.dart file, there are no occurences of findsOne, only findsOneWidget.
The reason is probably because findsOne was introduced somewhat recently, see #127137.
I would say using findsOneWidget when we are looking for a Widget is ok, but maybe @justinmc has some thinking about this.
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.
I did some major archaeology here and my conclusion is that we should actually be using findsOne and not findsOneWidget! See these two comment threads:
- Adds SemanticsNode Finders for searching the semantics tree #127137 (comment)
- Adds SemanticsNode Finders for searching the semantics tree #127137 (comment)
It seems like there was a long-term plan to deprecate findsOneWidget, but the people involved are no longer working on Flutter.
So I guess strictly this PR should be changed to use findsOne. And I should stop using findsOneWidget like I have been for all my time on Flutter.
But in reality, both are equivalent and it doesn't really matter. I'll just keep this fact in my head in case anyone ever comes along with the motivation to clean up these duplicate methods and do the deprecation.
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.
I see, thank you for those insights @justinmc. Is there something blocking us from deprecated findsOneWidget and other similar finders today?
If not, I could open PRs to deprecate them and migrate to findsOne and similar finders
5281771 to
0dc7383
Compare
justinmc
left a comment
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 👍. Sorry for the delay on this! Treat my comment below as a total nit.
| buildInputDecorator(decoration: const InputDecoration(error: Text(errorText))), | ||
| ); | ||
|
|
||
| expect(findError(), findsOneWidget); |
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.
I did some major archaeology here and my conclusion is that we should actually be using findsOne and not findsOneWidget! See these two comment threads:
- Adds SemanticsNode Finders for searching the semantics tree #127137 (comment)
- Adds SemanticsNode Finders for searching the semantics tree #127137 (comment)
It seems like there was a long-term plan to deprecate findsOneWidget, but the people involved are no longer working on Flutter.
So I guess strictly this PR should be changed to use findsOne. And I should stop using findsOneWidget like I have been for all my time on Flutter.
But in reality, both are equivalent and it doesn't really matter. I'll just keep this fact in my head in case anyone ever comes along with the motivation to clean up these duplicate methods and do the deprecation.
Thanks for the review 🙏 |
Roll Flutter from 8f94cb0d8f01 to 9ff2767f3cb6 (56 revisions) flutter/flutter@8f94cb0...9ff2767 2025-09-20 [email protected] Roll Fuchsia Linux SDK from 0_jKqLGnkILvQ5C8a... to CcCe3HpQtBYhTZscb... (flutter/flutter#175698) 2025-09-20 [email protected] Roll Dart SDK from e6e9248aee4f to 9e943fe076c8 (1 revision) (flutter/flutter#175697) 2025-09-20 [email protected] Add `menuController` to `DropdownMenu` (flutter/flutter#175039) 2025-09-20 [email protected] Roll Skia from 1dae085e2f31 to a38a531dec1d (3 revisions) (flutter/flutter#175694) 2025-09-20 [email protected] [a11y] TimePicker clock is unnecessarily announced (flutter/flutter#175570) 2025-09-20 [email protected] Roll Dart SDK from 78e68d1a7dbf to e6e9248aee4f (4 revisions) (flutter/flutter#175690) 2025-09-19 [email protected] Roll Skia from b56003bf2c20 to 1dae085e2f31 (1 revision) (flutter/flutter#175674) 2025-09-19 [email protected] Update `CODEOWNERS` (for dev-tooling) (flutter/flutter#175201) 2025-09-19 [email protected] [a11y-app] Add label to TextFormField in AutoCompleteUseCase. (flutter/flutter#175576) 2025-09-19 [email protected] Fix RadioGroup single selection check. (flutter/flutter#175654) 2025-09-19 [email protected] Roll Packages from f2a65fd to 3d5c419 (2 revisions) (flutter/flutter#175668) 2025-09-19 [email protected] Roll Skia from c74d2bdbd93c to b56003bf2c20 (2 revisions) (flutter/flutter#175665) 2025-09-19 [email protected] Add `CupertinoLinearActivityIndicator` (flutter/flutter#170108) 2025-09-19 [email protected] [ Widget Preview ] Don't update filtered preview set when selecting non-source files (flutter/flutter#175596) 2025-09-19 [email protected] Roll Dart SDK from 2c79803c97db to 78e68d1a7dbf (3 revisions) (flutter/flutter#175646) 2025-09-19 [email protected] Delete unused web_unicode library (flutter/flutter#174896) 2025-09-19 [email protected] Roll Skia from 684f3a831216 to c74d2bdbd93c (2 revisions) (flutter/flutter#175644) 2025-09-19 [email protected] Roll Skia from 462bdece17bf to 684f3a831216 (3 revisions) (flutter/flutter#175641) 2025-09-19 [email protected] Roll Skia from a2c38aa9df80 to 462bdece17bf (11 revisions) (flutter/flutter#175629) 2025-09-18 [email protected] fix(tool): Use merge-base for content hash in detached HEAD (flutter/flutter#175554) 2025-09-18 [email protected] [web] Unskip Cupertino datepicker golden tests in Skwasm (flutter/flutter#174666) 2025-09-18 [email protected] Update rules to include extension rules (flutter/flutter#175618) 2025-09-18 [email protected] [engine] Cleanup Fuchsia FDIO library dependencies (flutter/flutter#174847) 2025-09-18 [email protected] Make sure that a CloseButton doesn't crash in 0x0 environment (flutter/flutter#172902) 2025-09-18 [email protected] [ Tool ] Serve DevTools from DDS, remove ResidentDevToolsHandler (flutter/flutter#174580) 2025-09-18 [email protected] [engine][fuchsia] Update to Fuchsia API level 28 and roll latest GN SDK (flutter/flutter#175425) 2025-09-18 [email protected] Added a 36 device for Firebase Lab Testing (flutter/flutter#175613) 2025-09-18 [email protected] Roll Dart SDK from 09a101793af4 to 2c79803c97db (2 revisions) (flutter/flutter#175608) 2025-09-18 [email protected] Engine Support for Dynamic View Resizing (flutter/flutter#173610) 2025-09-18 [email protected] Roll Packages from fdee698 to f2a65fd (3 revisions) (flutter/flutter#175594) 2025-09-18 [email protected] Connect the FlutterEngine to the FlutterSceneDelegate (flutter/flutter#174910) 2025-09-18 [email protected] Roll Dart SDK from de5dd0f1530f to 09a101793af4 (2 revisions) (flutter/flutter#175583) 2025-09-18 [email protected] Roll Skia from 7b9fe91446ee to a2c38aa9df80 (1 revision) (flutter/flutter#175579) 2025-09-18 [email protected] Roll Skia from ab1b10547461 to 7b9fe91446ee (4 revisions) (flutter/flutter#175569) 2025-09-18 [email protected] [a11y-app] Fix form field label and error message (flutter/flutter#174831) 2025-09-18 [email protected] Fix InputDecoration does not apply errorStyle to error (flutter/flutter#174787) 2025-09-18 [email protected] Migrate to `WidgetPropertyResolver` (flutter/flutter#175397) 2025-09-18 [email protected] Migrate to WidgetState (flutter/flutter#175396) 2025-09-18 [email protected] Roll Skia from 79ec8dfcd9d4 to ab1b10547461 (17 revisions) (flutter/flutter#175561) 2025-09-18 [email protected] Removes NOTICES from licenses input (flutter/flutter#174967) 2025-09-18 [email protected] Roll Dart SDK from 116f7fe72839 to de5dd0f1530f (2 revisions) (flutter/flutter#175557) 2025-09-17 [email protected] [reland][web] Refactor renderers to use the same frontend code #174588 (flutter/flutter#175392) 2025-09-17 [email protected] feat: Enable WidgetStateColor to be used in ChipThemeData.deleteIconColor (flutter/flutter#171646) 2025-09-17 [email protected] Filter out unexpected process logs on iOS with better regex matching. (flutter/flutter#175452) 2025-09-17 [email protected] CupertinoContextMenu child respects available screen width (flutter/flutter#175300) 2025-09-17 [email protected] Correct documentation in PredictiveBackFullscreenPageTransitionsBuilder (flutter/flutter#174362) ...
## Description This PR fixes `InpuDecorator` not applying `InputDecoration.errorStyle` to `InputDecoration.error`. ## Before Text from `InputDecoration.error` is not styled correctly: <img width="239" height="75" alt="image" src="https://github.com/user-attachments/assets/9b27e029-f431-4448-9d2c-9a28f52087f8" /> ## After Text from `InputDecoration.error` is styled correctly: <img width="239" height="75" alt="image" src="https://github.com/user-attachments/assets/9b0ac7da-4933-4ee8-a3ae-d7b4db7ca218" /> ## Related Issue Fixes [InputDecorator does not apply errorStyle to error](flutter#174784) ## Tests Adds 1 test.
## Description This PR fixes `InpuDecorator` not applying `InputDecoration.errorStyle` to `InputDecoration.error`. ## Before Text from `InputDecoration.error` is not styled correctly: <img width="239" height="75" alt="image" src="https://github.com/user-attachments/assets/9b27e029-f431-4448-9d2c-9a28f52087f8" /> ## After Text from `InputDecoration.error` is styled correctly: <img width="239" height="75" alt="image" src="https://github.com/user-attachments/assets/9b0ac7da-4933-4ee8-a3ae-d7b4db7ca218" /> ## Related Issue Fixes [InputDecorator does not apply errorStyle to error](flutter#174784) ## Tests Adds 1 test.
## Description This PR fixes `InpuDecorator` not applying `InputDecoration.errorStyle` to `InputDecoration.error`. ## Before Text from `InputDecoration.error` is not styled correctly: <img width="239" height="75" alt="image" src="https://github.com/user-attachments/assets/9b27e029-f431-4448-9d2c-9a28f52087f8" /> ## After Text from `InputDecoration.error` is styled correctly: <img width="239" height="75" alt="image" src="https://github.com/user-attachments/assets/9b0ac7da-4933-4ee8-a3ae-d7b4db7ca218" /> ## Related Issue Fixes [InputDecorator does not apply errorStyle to error](flutter#174784) ## Tests Adds 1 test.
Description
This PR fixes
InpuDecoratornot applyingInputDecoration.errorStyletoInputDecoration.error.Before
Text from
InputDecoration.erroris not styled correctly:After
Text from
InputDecoration.erroris styled correctly:Related Issue
Fixes InputDecorator does not apply errorStyle to error
Tests
Adds 1 test.