added onFieldSubmitted callback

[abulka]

I have exposed an onSubmitted callback, including a test.
Exposing this was requested in #20

I needed this feature for one of my projects. The widget allowed the user to enter in text values but it was difficult to be notified of such changes and to validate them.

Notes

I introduced typedef void ValueCallBack(num newValue); for the type of the onSubmitted property, however that could be changed to a better name, and documented better. I could have re-used the DiffIncDecCallBack type but that didn't seem right.

The property is exposed as onSubmitted - perhaps it should be exposed as onFieldSubmitted.

[Abhilash-Chandran]

Hi, Great work and Thanks for the PR. So glad you wrote tests as well. 👍 💯 My thoughts on your notes 👇 .

• The Type ValueCallBack sounds apt for all the callbacks. I will see if it can be used for all the other callbacks.
• I will think about the property name onSubmitted. From my perspective it aligns well with TextFormField property. So I would leave it that way for now.

Todo

1. Could you format the functionality_test checking so the checks can proceed to codecoverage section.
2. Also it would be helpful if you could add this property in the readme.md under the configurable options section.

Once you finish this I will make a minor release so that you can use it right away. 😃

[codecov]

Codecov Report

Merging #22 into master will decrease coverage by 0.91%.
The diff coverage is 92.00%.

@@             Coverage Diff             @@
##            master      #22      +/-   ##
===========================================
- Coverage   100.00%   99.08%   -0.92%     
===========================================
  Files            1        1              
  Lines          210      219       +9     
===========================================
+ Hits           210      217       +7     
- Misses           0        2       +2     

Impacted Files	Coverage Δ
lib/src/number_increment_decrement.dart	99.08% <92.00%> (-0.92%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed1b639...f814625. Read the comment docs.

[abulka]

Ok formatting fixed and readme updated.

P.S. I initially created this pull request using my master branch rather than a separate branch, hope that's ok.

[Abhilash-Chandran]

OK I remove the print for time being. Merge and release it. If you need it we will discuss again how to proceed with that. Maybe with an error message or something.

[Abhilash-Chandran]

If you don't need this print statement, shall I remove it. I am not a big fan of print statements in code.

[Abhilash-Chandran]

Once again thanks a lot for the PR. I will make a minor release of the same.. 😃 🥳

[abulka]

No worries re the print. Thanks for accepting the PR. Cheers.

[Abhilash-Chandran]

I was trying cover integer case of onSubmitted to improve test coverage. Then, I was thinking, that since you are not setting the value back to the controller.text clicking the increment button is incrementing from the previously set value.

Additionally the inc/dec button logic is also not handling this.

😞 Not related to this change, another issue I found is user is able to edit the value directly which doesn't respect the inc decrement factor.

I will raise two issues and keep track of it so that it can be handled correctly. But I will work on it only during the weekend. Hope you can proceed with these issues not affecting your usecase.
.

[abulka]

I think your _clampAndUpdate fix is good.

I noticed a couple of other things:

• clamping does not happen unless you have created an onSubmitted callback. 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.
• you cannot enter negative numbers when text editing

As for the nuances re respecting the inc decrement factor - that's interesting.

• A relaxed interpretation of what should happen would be that the user can enter anything in range, and the inc/dec works off that.
• The other interpretation is that whatever the user enters is corrected to be a multiple of the inc/dec factor based off the min. This is bit trickier and also relies on correct settings of the min/max/inc widget values to be fully self consistent. For example I could set up min: -1 max: 4 inc: 2 and never be able to get to 4. I could enter 4 manually and it would auto-correct to a multiple of the inc. factor starting from min, which would be 3. The user might not be so happy, since the max is 4 !

[Abhilash-Chandran]

• clamping does not happen unless you have created an onSubmitted callback. 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.
• you cannot enter negative numbers when text editing

I noticed both these issues while testing. This is because the onChanged and other callbacks are not handled at all in the code. I have been postponing for someone to raise an issue to work on this. Now that its pointed I have the motivation to work it out. 😄

• A relaxed interpretation of what should happen would be that the user can enter anything in range, and the inc/dec works off that.
• The other interpretation is that whatever the user enters is corrected to be a multiple of the inc/dec factor based off the min. This is bit trickier and also relies on correct settings of the min/max/inc widget values to be fully self consistent. For example I could set up min: -1 max: 4 inc: 2 and never be able to get to 4. I could enter 4 manually and it would auto-correct to a multiple of the inc. factor starting from min, which would be 3. The user might not be so happy, since the max is 4 !

It is trickier like you said. As for the interpretation I prefer to go with the second one and create a flag for first mode interpretation, if someone raises an issue with a valid use case. I have to think about a correct way to enforce the second interpretation. If this min/max/inc fields are inconsistent it can be raised as an assertion error with a a warning of in consistent or possible missing values. I will give this a thought, meanwhile I will raise an issue for the first two points, so that we can track the discussion there. The interepretation of min/max/incFactor combo can be tracked via #23 I will update the description there.