Skip to content

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 18, 2025
Merged

Fix InputDecoration does not apply errorStyle to error #174787

merged 1 commit into from
Sep 18, 2025
+19 −1

Conversation

bleroux
Copy link
Contributor

@bleroux bleroux commented Sep 1, 2025

Description

This PR fixes InpuDecorator not applying InputDecoration.errorStyle to InputDecoration.error.

Before

Text from InputDecoration.error is not styled correctly:

image

After

Text from InputDecoration.error is styled correctly:

image

Related Issue

Fixes InputDecorator does not apply errorStyle to error

Tests

Adds 1 test.

@bleroux bleroux requested a review from justinmc September 1, 2025 09:33
@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Sep 1, 2025
gemini-code-assist[bot]
gemini-code-assist bot reviewed Sep 1, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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.

@bleroux bleroux mentioned this pull request Sep 2, 2025
Merged
@bleroux bleroux force-pushed the fix_input_decoration_does_not_apply_errorStyle_to_error branch 2 times, most recently from 2de498c to 5281771 Compare September 4, 2025 12:10
@bleroux
Copy link
Contributor Author

bleroux commented Sep 4, 2025

@justinmc If you get a chance to review this PR, it would be great if it can get merged before or not long after #174831 (which relies on this fix to get a proper visual rendering).

ValentinVignal
ValentinVignal approved these changes Sep 5, 2025
View reviewed changes
Copy link
Contributor

@ValentinVignal ValentinVignal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

packages/flutter/test/material/input_decorator_test.dart
buildInputDecorator(decoration: const InputDecoration(error: Text(errorText))),
);

expect(findError(), findsOneWidget);
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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:

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.

Copy link
Contributor

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

@bleroux bleroux force-pushed the fix_input_decoration_does_not_apply_errorStyle_to_error branch from 5281771 to 0dc7383 Compare September 17, 2025 06:10
justinmc
justinmc approved these changes Sep 17, 2025
View reviewed changes
Copy link
Contributor

@justinmc justinmc left a 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.

packages/flutter/test/material/input_decorator_test.dart
buildInputDecorator(decoration: const InputDecoration(error: Text(errorText))),
);

expect(findError(), findsOneWidget);
Copy link
Contributor

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:

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.

@bleroux
Copy link
Contributor Author

bleroux commented Sep 18, 2025

LGTM 👍. Sorry for the delay on this! Treat my comment below as a total nit.

Thanks for the review 🙏
I will let the findsOneWidget for the moment as it would be better to replace all occurrences in this test file at once.

@bleroux bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 18, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Sep 18, 2025
Merged via the queue into flutter:master with commit c0360ee Sep 18, 2025
79 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 18, 2025
@bleroux bleroux deleted the fix_input_decoration_does_not_apply_errorStyle_to_error branch September 18, 2025 07:46
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 18, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 18, 2025
@engine-flutter-autoroll
c0360ee0c Fix InputDecoration does not apply errorStyle to error (flu…
c53da4d 
…tter/flutter#174787)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 18, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 18, 2025
@engine-flutter-autoroll
c0360ee0c Fix InputDecoration does not apply errorStyle to error (flu…
f9c716b 
…tter/flutter#174787)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 18, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 18, 2025
@engine-flutter-autoroll
c0360ee0c Fix InputDecoration does not apply errorStyle to error (flu…
658a728 
…tter/flutter#174787)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 18, 2025
@engine-flutter-autoroll
c0360ee0c Fix InputDecoration does not apply errorStyle to error (flu…
fab45c0 
…tter/flutter#174787)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 18, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ValentinVignal ValentinVignal ValentinVignal approved these changes

@justinmc justinmc justinmc approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

InputDecorator does not apply errorStyle to error
3 participants
@bleroux @justinmc @ValentinVignal