Skip to content

FormField should autovalidate only if its content was changed (fixed)#59766

Merged
fluttergithubbot merged 30 commits intoflutter:masterfrom
pedromassango:formfield
Jul 16, 2020
Merged

FormField should autovalidate only if its content was changed (fixed)#59766
fluttergithubbot merged 30 commits intoflutter:masterfrom
pedromassango:formfield

Conversation

@pedromassango
Copy link
Member

@pedromassango pedromassango commented Jun 18, 2020

Description

I propose to auto-validate FormField only if its content was changed since its prevents end users to see validation errors even without changing the field's content.

Related Issues

Fixes: #56363
Fixes: #18885
Fixes: #15404
Fixes: #36154
Fixes: #48876

Tests

I added the following test cases:

  • Do not auto-validate before value changes if widget.autovalidateMode (new) equals to AutovalidateMode.onUserInteraction;
  • auto-validate after value changes if widget.autovalidateMode equals to AutovalidateMode.onUserInteraction;
  • Form only validates form fields after one of the form fields changes if widget.autovalidateMode is AutovalidateMode.onUserInteraction;
  • auto-validate before value changes if autovalidate is true and widget.autovalidateMode is AutovalidateMode.always;
  • Form auto validate form fields even if none of them changes if widget.autovalidateMode is always;
    autovalidate parameter is still used if true;
  • DropdownButtonFormField passes autoValidateMode to superclass;
  • Form.reset() reset form fields and auto validation will only happen on the next user interaction if autoValidateMode is onUserInteraction;
  • autoValidateMode and autovalidate should not be used at the same time for the following widgets: Form, FormField, TextFormField and DropdownButtonFormField.

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.

  • I signed the [CLA].
  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read [Handling breaking changes].

@fluttergithubbot fluttergithubbot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. work in progress; do not review labels Jun 18, 2020
@pedromassango pedromassango marked this pull request as ready for review June 18, 2020 14:51
# Conflicts:
#	packages/flutter/lib/src/material/text_form_field.dart
#	packages/flutter/test/material/dropdown_form_field_test.dart
#	packages/flutter/test/material/text_form_field_test.dart
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

Assuming nothing change from previous PR, LGTM except the formatting nit

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.

Thanks for opening up a new PR to get this to work. A few question and some indentation corrections below.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@chunhtai
Copy link
Contributor

It looks like the branch may still out of date, are you sure you have the latest from the master?

@pedromassango
Copy link
Member Author

It looks like the branch may still out of date, are you sure you have the latest from the master?

There is new updates. Let me update this branch again.

@justinmc
Copy link
Contributor

🎉 Congrats!

Thanks again for all the persistence in getting this PR ready. An API like this is tricky to get right. Let's keep an eye on how Flutter developers are using it as it rolls out.

@pedromassango
Copy link
Member Author

🎉 Congrats!

Thanks again for all the persistence in getting this PR ready. An API like this is tricky to get right. Let's keep an eye on how Flutter developers are using it as it rolls out.

Thank you. I still interested in improve this feature. And I will do that.

@normancarcamo
Copy link

Hi there!, thank you for the effort on this project, I have a few questions about TextFormField class and the version of these changes:

1. Has the property:autovalidateMode in the TextFormField class not available anymore?

I did the mistake of upgrade the version I had because the notification on the IDE was really annoying (currently I can't remember the previous version, but after updating the autovalidateMode is not available)...

Screen Shot 2020-09-02 at 10 51 40

The current version of flutter I have is the following:

flutter --version

Flutter 1.20.2 • channel stable • https://github.com/flutter/flutter.git
Framework • revision bbfbf1770c (3 weeks ago) • 2020-08-13 08:33:09 -0700
Engine • revision 9d5b21729f
Tools • Dart 2.9.1

flutter doctor

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, 1.20.2, on Mac OS X 10.15.6 19G2021, locale en-HN)
 
[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.1)
[✓] Xcode - develop for iOS and macOS (Xcode 11.5)
[✓] Android Studio (version 4.0)
[✓] IntelliJ IDEA Community Edition (version 2020.2.1)
[✓] VS Code (version 1.48.2)
[✓] Connected device (1 available)

• No issues found!

2. Which version of flutter were these changes merged?

In the changed files I can see that the property autovalidateMode is still present, but in the stable channel using the sdk release: 1.20.2 is not.

Could you give a hand to know what can I do because if not I will refactor the logic of some features already done.

Thanks!

@chunhtai
Copy link
Contributor

chunhtai commented Sep 2, 2020

It is not release to stable yet, you can switch to master channel and will be able to use this new API

@normancarcamo
Copy link

Oh!, understand it.
ok, thanks.

@EffyCoder
Copy link

EffyCoder commented Sep 23, 2020

I couldn't find this autovalidatemode field in the TextFormField.

Following is my flutter doctor output:

`
[√] Flutter (Channel stable, 1.20.4, on Microsoft Windows [Version 10.0.18363.1082], locale en-IN)
• Flutter version 1.20.4 at D:\flutter_windows_1.20.2-stable\flutter
• Framework revision fba99f6 (9 days ago), 2020-09-14 15:32:52 -0700
• Engine revision d1bc06f032
• Dart version 2.9.2

[√] Android toolchain - develop for Android devices (Android SDK version 30.0.2)
• Android SDK at D:\AndroidWork\sdk
• Platform android-30, build-tools 30.0.2
• ANDROID_HOME = D:\AndroidWork\sdk
• Java binary at: C:\Program Files\Android\Android Studio\jre\bin\java
• Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b01)
• All Android licenses accepted.

[√] Android Studio (version 4.0)
• Android Studio at C:\Program Files\Android\Android Studio
• Flutter plugin version 46.0.2
• Dart plugin version 193.7361
• Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b01)

[√] VS Code, 64-bit edition (version 1.48.2)
• VS Code at C:\Program Files\Microsoft VS Code
• Flutter extension version 3.14.1

[√] Connected device (1 available)
• Android SDK built for x86 (mobile) • emulator-5554 • android-x86 • Android 8.1.0 (API 27) (emulator)

• No issues found!
`

When it will be added to stable release?

@pedromassango
Copy link
Member Author

It is not yet on stable!
I don't know when it will land on stable release.

@normancarcamo
Copy link

normancarcamo commented Sep 23, 2020

@EffyCoder & @pedromassango I fixed my issue using autovalidate option, for example, only when the input has focus:

build(BuildContext context) {
  return Container(
    TextFormField(
      autovalidate: focusNode.hasFocus,
    )
  );
}

You can do more stuffs with this approach, and it's working in the same way before it was.

@pedromassango
Copy link
Member Author

@EffyCoder & @pedromassango I fixed my issue using autovalidate option, for example, only when the input has focus:

build(BuildContext context) {
  return Container(
    TextFormField(
      autovalidate: focusNode.hasFocus,
    )
  );
}

You can do more stuffs with this approach, and it's working in the same way before it was.

Last week I was thinking of adding this feature.

@chunhtai @justinmc My only concern it just that autovalidateMode.hasFocus will only be used on single form fields (FormField, TextFormField, etc..) and not on the Form widget.

We have make it clear that it will have no effect on the Form widget, but this may not be the best thing to do.

Do you have any thing in mind about this?

@chunhtai
Copy link
Contributor

chunhtai commented Sep 23, 2020

They can achieve the same result with

return Container(
    TextFormField(
      autovalidateMode: focusNode.hasFocus ?? AutovalidateMode.always :AutovalidateMode.disabled
    )
  );

We should not expend the mode unless there is a mode that cannot be achieve with current API and/or we have a lot of use cases.

@pumuckelo
Copy link

pumuckelo commented Oct 17, 2020

I just updated my flutter_form_builder package.

First off, thank you for implementing this feature!!

Why is the type of autovalidateMode dynamic though? Also, my IDE can't find "AutovalidateMode.always" etc

EDIT:
Sorry, just needed to restart my VS CODE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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