Skip to content
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.

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

Prevent crash when holding backspace #352

Merged
merged 1 commit into from
Jul 27, 2024

Conversation

MikePlante1
Copy link
Contributor

@MikePlante1 MikePlante1 commented Jul 17, 2024

Currently, entering text into Note while entering a meal and then holding the backspace button would crash the app. This PR prevents that crash.

@MikePlante1 MikePlante1 changed the title Prevent crashing and mis-formatting from text entries Corrections for TextFieldWithToolBar Jul 18, 2024
@MikePlante1
Copy link
Contributor Author

MikePlante1 commented Jul 18, 2024

Updated to use decimalFormatter.groupingSeparator instead of just "," to allow it to work for all regions.

marionbarker
marionbarker previously approved these changes Jul 20, 2024
Copy link
Contributor

@marionbarker marionbarker left a comment

Choose a reason for hiding this comment

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

Summary

LGTM to fix the crash

Code Review

  • code changes looks appropriate
  • Can someone check localization?
    • I tried different languages and did not see the decimal/thousands place delimiters I expected
    • If confirmed, that should be a separate issue.

Test Details

Confirm problem

  • build with dev, 8303f0a
  • confirm 1 - crashed the app with backspace in Notes field
  • confirm 2 - entered in carbs field - see the bug

Confirm fix

  • apply patch from PR 352
  • confirm fix 1 - able to backspace in notes field, app does not crash
  • confirm fix 2 - entered 126555 in carb field, displays 126,555 (with > max warning symbol)
    • delete one character at a time
      • 12,655
      • 1,265
      • 126
      • 12 (no longer > max carbs, symbol gone)
  • confirm maxLength works
  • confirm clear works
  • confirm proper thousands' place behavior in temp target, bolus

@MikePlante1
Copy link
Contributor Author

  • Can someone check localization?

    • I tried different languages and did not see the decimal/thousands place delimiters I expected
    • If confirmed, that should be a separate issue.

You have to change the Region of the phone, not the Language. Each of the following produces a different grouping separator:
US 9,999,999,999.5
UK 9.999.999.999,5
Norway: 9 999 999 999,5
India: 9,99,99,99,999.5

@MikePlante1
Copy link
Contributor Author

I removed the grouping separator fix from this PR, as a similar solution has been added to #362

@MikePlante1 MikePlante1 changed the title Corrections for TextFieldWithToolBar Prevent crash when holding backspace Jul 25, 2024
Copy link
Contributor

@marionbarker marionbarker left a comment

Choose a reason for hiding this comment

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

Tested again with the latest commit.

@dnzxy dnzxy merged commit 6c88cee into nightscout:dev Jul 27, 2024
1 check passed
@MikePlante1 MikePlante1 mentioned this pull request Jul 27, 2024
mountrcg pushed a commit to mountrcg/Trio that referenced this pull request Jul 29, 2024
mountrcg pushed a commit to mountrcg/Trio that referenced this pull request Aug 1, 2024
* Update Bolus Cancel Button

* Refactor

---------

Co-authored-by: Jon Mårtensson <jon.m@live.se>
@MikePlante1 MikePlante1 deleted the deleting_meal_note_fix branch August 14, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants