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

Fix manual glucose entry for mmol/L #356

Merged
merged 1 commit into from
Jul 27, 2024
Merged

Conversation

MikePlante1
Copy link
Contributor

  • Prevents mmol/L entry from being forced into having 1 decimal place
  • Fixes typo that swapped high/low limits from mg/dL and mmol/L
  • Round down for entry instead of to nearest

Addresses Issue #339

Before this PR:

prePR_.mov

After this PR:

postPR_.mov

* Prevents mmol/L entry from being forced into having 1 decimal place
* Fixes typo that swapped high/low limits from mg/dL and mmol/L
* Round down for entry instead of to nearest
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.

Status

LGTM to fix the mmol/L Glucose entry problem
Then we can resolve whether there is a better method for fixing this with more consistent units handling

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

Confirm problem

  • build with dev, 8303f0a
  • confirm cannot enter mmol/L values > 9.9 on glucose screen

Confirm fix

  • apply patch from PR 356
  • confirm can now enter mmol/L values up to 40 mmol/L on glucose screen
  • no error message is give when > 40 mmol/L is attempted, but the save button is inactive so that's ok for now
  • It is a little disconcerting to enter 40.0 or 10.0 and have it modify the display to 40 or 10 respectively
    • The display on the log includes the single decimal point for mmol/L
  • Switch to mg/dL - confirm min/max range is now 14 mg/dL to 720 mg/dL
  • Switch to mmol/L to test the low range (log changes to 0.8 mmol)
    • confirm min/max range is 0.8 to 40.0 mmol/L

@MikePlante1
Copy link
Contributor Author

MikePlante1 commented Jul 21, 2024

  • 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.

  • It is a little disconcerting to enter 40.0 or 10.0 and have it modify the display to 40 or 10 respectively

    • The display on the log includes the single decimal point for mmol/L

I agree that it's not great, but unfortunately having it auto-format to a single decimal place is what messed up being able to enter double digits. I played around with it a bunch to make it so minimumFractionDigits defaults to 0, is set to 1 when the user types a decimal point, and is reset to 0 if the decimal point is deleted... but couldn't get it to work right.

Copy link
Contributor

@dnzxy dnzxy left a comment

Choose a reason for hiding this comment

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

LGTM

@dnzxy dnzxy merged commit 03ef229 into nightscout:dev Jul 27, 2024
@MikePlante1 MikePlante1 mentioned this pull request Jul 27, 2024
mountrcg pushed a commit to mountrcg/Trio that referenced this pull request Jul 29, 2024
@MikePlante1 MikePlante1 deleted the mmol_entry 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