resolve incorrect bolus amount due to uitextfield issue

[kskandis]

Resolves Incorrect bolus amount entered due to data entry problem #358

[dnzxy]

Thanks for providing this fix 💪 How does this relate or influence or affect #352 and #356 ?

[kskandis]

Thanks for providing this fix 💪 How does this relate or influence or affect #352 and #356 ?
Sorry, I didn't see these PRs.

#352 does include a removal of the groupingseparator for extension TextFieldWithToolBar.Coordinator: UITextFieldDelegate {which is also in this PR, so there could be a minor file change conflict, easy to resolve.
Remaining fixes in #352 are for extension TextFieldWithToolBarString.Coordinator: UITextFieldDelegate { which this PR does not address, so there should be no conflict.
#352 does not include the primary fix for extension TextFieldWithToolBar.Coordinator in this PR though.

#356 issues are not addressed in the PR. Files changed would have no conflict.

[dnzxy]

Thanks for the quick reply.
So I think #352 can be closed and #356 will not be affected - if this is tested and approved and fixes the issue in full?

[kskandis]

So I think #352 can be closed

No, #352 has fixes to the Note textfield which THIS PR does not address so that is a separate fix and assuming test and works, should be merged. #352 ALSO has a few lines in its code that this PR also has very similar code for the thousand separator symbol. What I can do is merge #352's thousand separator code into this PR so that it is exactly the same and hence there would be no conflicts. Then you can merge this PR and afterwards merge #352. Would that work?

OR alternatively, #352 author could remove the thousand separator code fix since it is unrelated to the Note field. Then both of these PRs can be merged w/out conflict.

[MikePlante1]

OR alternatively, #352 author could remove the thousand separator code fix since it is unrelated to the Note field. Then both of these PRs can be merged w/out conflict.

Okay. This has now been removed from my PR.

[MikePlante1]

LGTM from code review and testing in a simulator. I tested in both 🇺🇸 (1.05) and 🇩🇪 (1,05) regions.

[marionbarker]

Summary

There are places where you cannot type #.0# and get anything other than ##.
Perhaps none the places should accept those values, but if people type them, their entry does not match what was typed.

Configuration

• iPhone SE, iOS 17.5.1, blank instance (deleted app before starting)
• Trio dev, commit dff37ac
• Applied all open "entry" problem PR (got fresh patches from the PR as needed)
• all applied without errors

my CLI:

dff37ac2 (HEAD -> dev, origin/dev, origin/HEAD) Merge pull request #359 from kskandis/dev

 1016  git apply ~/dev/git_local/trio-patches/trio-pr-362-uitextfield-issue.patch
 1017  git apply ~/dev/git_local/trio-patches/trio-pr-356-mmol-fix.patch
 1018  git apply ~/dev/git_local/trio-patches/trio-pr-352_deleting-text-bug.patch

Testing:

Work my way through adding information to a fresh build - remark if there are issues.

Could not enter #.0# for these fields

Notifications;

• Low and High in mmol/L (note that 4.1 was ok, but type 4.05 and get 45)
• Carbs required: type 1.5 and get 2 (ok), but type 1.05 and get 15

Statistics

• Hours: can't type 4.05, if you type 4.5 it rounds up to 5, but you can type 66
• Low and High: if you type 5.01 you get 51 mmol/L
• but then leaving the screen and returning: Hours changed to 24 and low and high changed to 5 and 10 mmol/L
• tried again with 6 and 12 - left and returned and got 5 and 12 (Issue Unify how settings are stored #361 refers to this type of behavior)

Preferences

• All the entries on this screen allow the user to type in #.0# with no problem, even when it makes no sense
• Leave and return and the values typed in are maintained.

Pump Settings

• #.0# is accepted for all entries and maintained

Meal Settings

• some are ok with #.0#
• Fat and Protein conversion settings are not ok
  • for delay in minutes, type 60.5 and get 605.
  • maximum duration, type in 8.5 and get 85.
  • interval in minutes, type 30.5 and get 305
  • override with a factor, type 1.05 and get 15

These use picker wheels: basal profile, ISF, CR, Target Glucose

Log Insulin

• #.0# is accepted and saved

Log Glucose (mmol/L)

• 10.# is accepted
• 38.9 is accepted
• 10.05 shows up at 105

Carb Entry

• The carb, fat and protein rows convert #.0# to ##
• The notes field is fine

Enact Temp Target

• Target entry of #.0# becomes ##
• Duration entry of 30.05 becomes 305
• Temp Target of 55 appears to have been accepted
• Cancel the TT shortly after enacting it

Enact Bolus

• #.0# is accepted

[marionbarker]

This fixes the one issue, #362, so I'm adding an approval.
As one of the comments, I went through all the data entry fields for others to review.

[kskandis]

Meal Settings

• some are ok with #.0#

• Fat and Protein conversion settings are not ok

  • for delay in minutes, type 60.5 and get 605.

Thanks, Marion, as always, for your thorough testing. I actually only tested with Bolus Enact Entry, Meal Settings, and Carb Entry, all of which appeared to work for me.

For the Meal Settings - Delay in Minutes/Max Duration in Hours/Interval in Minutes, I believe should NOT accept a decimal symbol, since their View code specifically says it is an intFormater with allowsFloats=false - see my first comment in the corrsponding issue report. So that is how I coded it. Should they allow fractions?

Meal Settings - Override With A Factor Of has formatter with formatter.maximumFractionDigits = 1. To my understanding, this means data entry should allow only 1 digit entered to the right of the decimal separator. Hence, entering '1.05' would not be allowed. Once you enter 1.0, the textfield will drop the 0 and display 1, so if you then enter a 5, textfield will correctly display as 15. Am I misunderstanding the requirements?

I suspect the other fields that you tested have similar issues, esp anything whose View formatters have maximumFractionDigits = 1 (eg., Carb Entry, TT Entry) or allowsFloats=false. What are the expected requirements for each of these fields? Perhaps the Formatters need to be reviewed for each field?

[kskandis]

This fixes the one issue, #362, so I'm adding an approval. As one of the comments, I went through all the data entry fields for others to review.

Please see above comment! I think this PR would apply to all NumberFormat textfields but maybe we need to clarify data entry requirements for each field. Eg., should maxFractionDigits be 1, 2, 3, etc., should decimal entry be allowed. I added code specifically to support the View formatters but the View formatters (for each field) need to be defined appropriately for the field. I did not change any of the formatters which are defined on each field in various files.

[kskandis]

As one of the comments, I went through all the data entry fields for others to review.

I removed the code which addresses your other comments in your review.

You should now be able to enter decimal for the Integer fields in Meals Settings but upon returning to the screen, due to allowFloats=false, the number will revert to an Integer.

[marionbarker]

Summary

Success - thank you
Using "." as the decimal separator:

• If an entry must be an integer, typing in x.yz becomes x', where x' is rounded up or down depending on 0.yz.
• If an entry may only have one place to the right of the decimal, typing in x.yz becomes x.y', where y' is rounded up or down depending on the value of z.
• There is one nit (see end of comment) but not sufficient to delay approval
• LGTM! As well as PR 352 and 356.

Configuration

• Downloaded a fresh patch to get latest commit for PR 362.
• started with Trio dev branch, commit 207dd17
• applied all the entry commits current open:

 1028  git apply ~/dev/git_local/trio-patches/trio-pr-352_deleting-text-bug.patch
 1029  git apply ~/dev/git_local/trio-patches/trio-pr-356-mmol-fix.patch
 1030  git apply ~/dev/git_local/trio-patches/trio-pr-362-uitextfield-issue.patch

Tests

Repeated the testing from this comment.

• Notifications - works as expected, thank you
• Statistics - works as expected, thank you
• Preferences - no change (already ok)
• Pump Settings - no change (already ok)
• Meal Settings - works as expected, thank you
• Log Insulin - no change (already ok)
• Carb Entry - works as expected, thank you
• Enact Temp Target - works as expected, thank you
• Enact Bolus - no change (already ok)

One minor nit - when using "." as a decimal separator, when I have ##.# and hit back space, I am presented with ## with no decimal separator. Most people hitting backspace to modify a number will notice, but seems like the decimal separator should not be removed until hitting the keyboard symbol or hitting backspace. This will not prevent me from approving this.

[kskandis]

One minor nit - when using "." as a decimal separator, when I have ##.# and hit back space, I am presented with ## with no decimal separator. Most people hitting backspace to modify a number will notice, but seems like the decimal separator should not be removed until hitting the keyboard symbol or hitting backspace.

Thank you, again, for your thorough testing!! Yes, I agree with this comment. Hwr, it does behave the same as in Loop - Add Carb Entry.

• If an entry must be an integer, typing in x.yz becomes x', where x' is rounded up or down depending on 0.yz.

AFAIK, only the Meal Settings (top 3 under Conversion Settings displaying # of Hours or Minutes) contain what appear to be Integers (allowFloats=false).

[dnzxy]

Approving based on code review. LGTM.

[dnzxy]

Merging based on rigoros testing by Marion and Mike and 2 approvals (formerly 3, but changes since Mike tested).