ClampAndUpdate inconsitent while editing.

[Abhilash-Chandran]

As @abulka noted in the comments of another discusssion.

Clamping to min and max, does not happen unless an onSubmitted callback is created. Clamping should happen always. For example example2() lets you enter in anything you want using text editing - it disregards min/max. This bug presumably was there prior to the onSubmitted enhancement.

This needs to be addressed for all the manual edits.

Initial Thoughts: Exposing all form of callbacks in TextFormField and handling clamping could fix this issue.

[abulka]

Initial simple fix might be to move the calls to _tryParse and _clampAndUpdate on lines 707-709 to before the if statement on line 706.

In other words, the TextFormField that gets created by the widget always has an onFieldSubmitted handler thus can always clamp. Whether the user has supplied an onSubmitted handler is then a secondary concern.

[Abhilash-Chandran]

Sure, I will change it accordingly. I am thinking of a fix combining #27 #26 together by removing the regular expression check and only validate by parsing. I am checking in some changes in v0.6.8 branch. Its completely buggy. But you can see where I am going with it.

Maybe Validating onChanged is an overkill. I will test it further and see if its unnecessary.

I will work on this tomorrow and possibly on Monday. Got to get some sleep today. 😄 😴

[Abhilash-Chandran]

@abulka I have merged a fix into v0.6.8 branch. If you could give it a test in the context of onSubmitted it would be great. Now clamping doesn't happen by default. So you need to pass this boolean flag enableMinMaxClamping explicitly as true to have this effect. If you find any other issue please do let me know.

New Changes

• Removed removed regular expression based formatting for decimal field as it becomes tricky to capture and the replacemenText in the formatter is a difficult to get right.
• autovalidate defaults to true instead of false to enabled validation by default for non numeric entry.
• moved the tryparse and clampAndUpdate outside the if block in onSubmitted and extracted onSubmitted as a private method.
• default validation will be replaced if the user provides custom validators. This way users still have the possibility to perform customValidation or completely override internal validation.

[abulka]

Thanks for this work - a few observations:

• When the text entry field is blank, an error "is an invalid integer value" is displayed. Whilst technically correct, often users may backspace to clear the textfield before typing the value they want (rather than painstaking cursor fiddling and editing) and will see this rather scary message.
• Same as above for typing minus "-". I'm not convinced that errors should be displayed as a natural part of text entry.
• I can still enter a value that is out of range, it will be accepted and my onSubmit handler called with the invalid value. I prefer that the value be auto-clamped to min/max so that my handler does not get invalid values.

Also, nice to see the cleanup of attribute duplicates in the code! May I suggest that enum ButtonArrangement and class NumberInputWithIncrementDecrement be moved to the top of the file as they are where all the action is. Just a thought.

[Abhilash-Chandran]

Thanks a lot for you feedback. 😃

• When the text entry field is blank, an error "is an invalid integer value" is displayed. Whilst technically correct, often users may backspace to clear the textfield before typing the value they want (rather than painstaking cursor fiddling and editing) and will see this rather scary message.

Thank you and valid point. Also I see another issue with this. If the field is supposed to be optional and user tries to clear the field, it would disallow user to submit the form. Thanks for pointing it out. I will work out a way to remove this validation. Empty should be handle by other features like mandatory or optional. The user can handle it I suppose.

• Same as above for typing minus "-". I'm not convinced that errors should be displayed as a natural part of text entry.

For a minus sign a validation on onSubmitted should be carried out right? I agree it would be scary while typing to see it. so I will remove this validation from onChanged.

• I can still enter a value that is out of range, it will be accepted and my onSubmit handler called with the invalid value. I prefer that the value be auto-clamped to min/max so that my handler does not get invalid values.

Ideally provided onSubmitted should not be called with validation errors. I will check why this is not the case. This sounds like a bug.

I have exposed a flag for this purpose to enable or disable clamping. I felt it was out of users control to clamp it and return when it is out of bounds. My line of thoughts..

Showing an error for min and max bounds and then clamping while onSubmitted could be both be a good and bad.

• Imagine this value leads to some kind of windowed computation behind the scenes. Accidentally entering a large value say by a palm resting on a number key, could lead to some unwanted heavy computation.
• On the contrary to my own point, clamping is good that it didn't lead to large computation and held within the maximum allowed value. So shouldn't it be the developer who decides this behavior than the widget author..!

Do you have a better use case for the default clamping to be enforced?

Also, nice to see the cleanup of attribute duplicates in the code! May I suggest that enum ButtonArrangement and class NumberInputWithIncrementDecrement be moved to the top of the file as they are where all the action is. Just a thought.

Thanks to you for pointing it out. 😉 I am actually thinking of a refactoring where I split the files. I will definitely consider this. 👍 One thing I noticed with that clean up is that since all the fields are optional, I had to ensure that I properly pass down all the attributes in the super call. Most of the tests failed until I fixed them. I am not sure if there is a way to ensure that maybe some linter rules, will check it though. For now I will keep it like this. No more double declaration, yay. 🥳

PS: I really hope I didn't offend you by moving around the code you submitted as PR 😟. If yes please do let me know, We can discuss it out. I am new to open source collaboration.

[abulka]

Thanks for your thoughts. Here are some more of mine!

Re clamping, if you introduce a flag I'd want it to be on by default as clamping is a good thing. If you can't enter in values outside the min max range using the up/down arrows, then you shouldn't be able to do it via direct text entry either.

If you enter in 999 into a range 0-3 then the number should be clamped to 3 and the onSubmit handler called. Otherwise you are in "no man's land" where you have an invalid value sitting there, and the application is in an unknown state. Was onSubmit called? What do I do now? The behaviour should be like a form which doesn't accept bad values, they should never be allowed into the application.

Here is an example of how I am using this widget in my first flutter app:

In my case all the values depend on each other and I never want the user to type in anything crazy - or the app breaks.

Yes there are nuances where a developer may want more control in which case they can switch the auto clamping off and deal with everything manually with explicit code. Presumably the onSubmit would be called with the bad value - and the developer can deal with that. This would need to be documented.

[Abhilash-Chandran]

Thanks for your thoughts. Here are some more of mine!

Re clamping, if you introduce a flag I'd want it to be on by default as clamping is a good thing. If you can't enter in values outside the min max range using the up/down arrows, then you shouldn't be able to do it via direct text entry either.

If you enter in 999 into a range 0-3 then the number should be clamped to 3 and the onSubmit handler called. Otherwise you are in "no man's land" where you have an invalid value sitting there, and the application is in an unknown state. Was onSubmit called? What do I do now? The behaviour should be like a form which doesn't accept bad values, they should never be allowed into the application.

In my case all the values depend on each other and I never want the user to type in anything crazy - or the app breaks.

This makes sense that the form should not allow values exceeding the min/max bounds even accidentally. One of the reasons we have min/max bound by itself is to keep the values within the bounds. I am convinced and I stand corrected. I will change the default value to enable clamping always. Thanks a lot for the valuable feedback. I was a bit worried it would look like the widget is opinionated but in reality this is very good safety mechanism, especially in case of dependent fields. I will change this and perhaps make all the fixes part of 0.7.0 and prepone it to mid of September, instead of 0.6.8 as I intended initially.

Yes there are nuances where a developer may want more control in which case they can switch the auto clamping off and deal with everything manually with explicit code. Presumably the onSubmit would be called with the bad value - and the developer can deal with that. This would need to be documented.

Sure 👍