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

Rename Fat And Protein settings to Meal Settings, move Max Carbs to Meal Settings #173

Merged
merged 4 commits into from
May 13, 2024

Conversation

bjornoleh
Copy link
Contributor

No description provided.

…move Max Carbs from Pump Settings to Meal Settings

Reset all relevant localised strings and replaced:
- Fat And Protein Conversion with Meal Settings
- Conversion settings with Fat and Protein Conversion Settings
@bjornoleh
Copy link
Contributor Author

Please note that when exceeding the max limit, the carb entry is truncated to the limit without notification. This is how this was implemented in iAPS. There is scope for improvements here, but perhaps outside of this PR.

@MikePlante1 MikePlante1 self-requested a review May 12, 2024 21:48
Copy link
Contributor

@MikePlante1 MikePlante1 left a comment

Choose a reason for hiding this comment

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

LGTM. I'd like to add where it won't allow the entry at all instead of capping it, like was introduced in #100 and extended in #172 . Should I add that here, or wait until this is merged and open a new PR?

@bjornoleh
Copy link
Contributor Author

Thanks for the review. Maybe create a new PR for the improvements?

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.

LGTM
Also tested it on a phone

@MikePlante1 MikePlante1 merged commit f6afb8c into dev May 13, 2024
@MikePlante1 MikePlante1 deleted the meal_settings branch May 13, 2024 10:32
@MikePlante1 MikePlante1 mentioned this pull request May 14, 2024
4 tasks
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.

None yet

3 participants