Skip to content

fix: Form AutovalidateMode.onUserInteraction behavior #120730

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
Closed
pedromassango wants to merge 1 commit into from
Closed

fix: Form AutovalidateMode.onUserInteraction behavior #120730

pedromassango wants to merge 1 commit into from

Conversation

@pedromassango
Copy link
Member

@pedromassango pedromassango commented Feb 15, 2023

List which issues are fixed by this PR. You must list at least one issue.
Fixes #107350

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Feb 15, 2023
@pedromassango pedromassango added a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. labels Feb 15, 2023
chunhtai
chunhtai reviewed Feb 16, 2023
View reviewed changes
packages/flutter/lib/src/widgets/form.dart
for (final FormFieldState<dynamic> field in _fields) {
hasError = !field.validate() || hasError;
if (widget.autovalidateMode == AutovalidateMode.onUserInteraction) {
if (_hasInteractedByUser && field._hasInteractedByUser.value) {
Copy link
Contributor

@chunhtai chunhtai Feb 16, 2023

Choose a reason for hiding this comment

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

Is there a documentation on what Form.autovalidateMode will interact with FormField.autovalidateMode if they are both set? If not, we should document it.

packages/flutter/lib/src/widgets/form.dart
for (final FormFieldState<dynamic> field in _fields) {
hasError = !field.validate() || hasError;
if (widget.autovalidateMode == AutovalidateMode.onUserInteraction) {
if (_hasInteractedByUser && field._hasInteractedByUser.value) {
Copy link
Contributor

@chunhtai chunhtai Feb 16, 2023

Choose a reason for hiding this comment

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

also, should we remove _hasInteractedByUser here? It seems like _hasInteractedByUser will always be true if field._hasInteractedByUser.value is true..

@deandreamatias deandreamatias mentioned this pull request Apr 30, 2023
Open
2 tasks
@Hixie
Copy link
Contributor

Hixie commented May 2, 2023

@pedromassango Is this PR still on your radar?

@pedromassango
Copy link
Member Author

pedromassango commented May 4, 2023

@pedromassango Is this PR still on your radar?

Yes

justinmc
justinmc reviewed May 8, 2023
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.

Related to: #125766

CC @Renzo-Olivares who's looking into that bug.

The PR looks good to me if we're on board with this behavior.

packages/flutter/lib/src/widgets/form.dart
Comment on lines +244 to +250
if (widget.autovalidateMode == AutovalidateMode.onUserInteraction) {
if (_hasInteractedByUser && field._hasInteractedByUser.value) {
hasError = validateFormField(field);
}
} else {
hasError = validateFormField(field);
}
Copy link
Contributor

@justinmc justinmc May 8, 2023

Choose a reason for hiding this comment

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

Nit: If I have the logic correct, maybe this is cleaner?

Suggested change
if (widget.autovalidateMode == AutovalidateMode.onUserInteraction) {
if (_hasInteractedByUser && field._hasInteractedByUser.value) {
hasError = validateFormField(field);
}
} else {
hasError = validateFormField(field);
}
if (widget.autovalidateMode != AutovalidateMode.onUserInteraction
|| (_hasInteractedByUser && field._hasInteractedByUser.value)) {
hasError = validateFormField(field);
}

@Hixie
Copy link
Contributor

Hixie commented Jul 11, 2023

@pedromassango Hey sorry to badger you, just wondering if we should close this one for now to get it off the review queue or if it's something you expect to get back to in the near future. Obviously no rush either way.

@Hixie
Copy link
Contributor

Hixie commented Aug 29, 2023

@pedromassango I'm going to close this for now, please don't hesitate to reopen it if this is something you'd like to continue working on. Thanks!

@Hixie Hixie mentioned this pull request Aug 29, 2023
Open
@Hixie Hixie closed this Nov 14, 2023
@justinmc justinmc mentioned this pull request Jan 10, 2024
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@justinmc justinmc justinmc left review comments

@chunhtai chunhtai chunhtai left review comments

Assignees

@pedromassango pedromassango

Labels

a: text input Entering text in a text field or keyboard related problems 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.

[Form] AutovalidateMode.onUserInteraction validates Textfields without interaction

4 participants

@pedromassango @Hixie @justinmc @chunhtai