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

Rework DecimalTextField Struct and Toolbar #337

Merged
merged 3 commits into from
Jul 9, 2024

Conversation

polscm32
Copy link
Contributor

  • fix clear button not working as expected
  • make cursor right adjusted when tapping in the Textfield
  • fix constraint error appearing when tapping into Textfield

Tested by me and @AndreasStokholm

…constraint error - fix clear button not working as expected - make cursor right adjusted when tapping in the Textfield
@kskandis
Copy link
Contributor

Did you also want to add @bjornoleh's review request to right adjust the Note TextField, add Clear and Done buttons, and hide its Keyboard icon?

@polscm32
Copy link
Contributor Author

Sure, can do that. Forgot that because we got rid of this field in our UI rework😅

@Sjoerd-Bo3
Copy link
Contributor

Going to test this next

@marionbarker
Copy link
Contributor

Tested - works in these screens (very nice):

  • carb/fat/protein/notes entry
  • enact bolus
  • enact temp target
  • Nightscout (nit but the Port should not have a thousands place separator, shows us as 8,080 for US English)
  • Notifications
  • Statistics and Home View
  • Preferences
  • Pump Settings
  • Meal Settings
  • These use pickers so n/a: Basal Settings, Insulin Sensitivities, Carb Ratios, Target Glucose)

@marionbarker marionbarker self-requested a review July 2, 2024 14:15
marionbarker
marionbarker previously approved these changes Jul 2, 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.

Only item I saw was minor: formatting of Port in Nightscout Screen.

dnzxy
dnzxy previously approved these changes Jul 4, 2024
@Sjoerd-Bo3
Copy link
Contributor

Only item I saw was minor: formatting of Port in Nightscout Screen.

Also the only Item I observed, THe changes for the rest are great! Works as it should have been.

@polscm32 Could you adjust the Nightscout port field?

Sjoerd-Bo3
Sjoerd-Bo3 previously approved these changes Jul 6, 2024
Copy link
Contributor

@Sjoerd-Bo3 Sjoerd-Bo3 left a comment

Choose a reason for hiding this comment

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

Except for the nightscout port LGTM

@polscm32
Copy link
Contributor Author

polscm32 commented Jul 6, 2024

Will have a look today!

@polscm32 polscm32 dismissed stale reviews from Sjoerd-Bo3, dnzxy, and marionbarker via c24bd7b July 6, 2024 09:26
@polscm32
Copy link
Contributor Author

polscm32 commented Jul 6, 2024

Should be fixed. Pls test xD

Copy link
Contributor

@Sjoerd-Bo3 Sjoerd-Bo3 left a comment

Choose a reason for hiding this comment

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

Code looks good! And the change works! Tnx!

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, sane and surgical change. Thanks!

@Sjoerd-Bo3
Copy link
Contributor

Merging with 2 approvals and Previous from Marion

@Sjoerd-Bo3 Sjoerd-Bo3 merged commit bd56601 into nightscout:dev Jul 9, 2024
@MikePlante1 MikePlante1 mentioned this pull request Jul 16, 2024
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.

5 participants