nightscout / Trio

MIT License
46 stars 130 forks source link

Add spacing for exceeding max bolus warning #210

Closed JeremyStorring closed 4 weeks ago

JeremyStorring commented 1 month ago

Describe the bug When the user exceeds their max bolus, the formatting of the error is a bit ugly.

To Reproduce Steps to reproduce the behavior:

  1. Go to bolus
  2. Enter amount > max bolus
  3. See warning "Max bolus exceeded! (>#U)"

Expected behavior I believe that the warning should 1) Be more visible, instead of blending in with everything else 2) Have different wording, such as "Max bolus of # U exceeded."

Screenshots image

Smartphone (please complete the following information): iPhone 14 (but not applicable)

Setup Information (please complete the following information):

bjornoleh commented 1 month ago

I agree with the suggestions above. The current grey colour is not very noticeable. The rewording could make it a little clearer too.

MikePlante1 commented 1 month ago

I agree a color change here would be helpful. How about just changing the text color to red? After line 92 in BolusRootView:

.foregroundStyle(state.amount <= 0 ? .gray : state.amount > state.maxBolus ? .red : .blue)
Screenshot 2024-05-19 at 10 34 24 AM

I used this wording in #172 because I didn't want to break the translations that already exist for this label, and inserting a variable in the middle of the label splits it into two separate translations.

old: Max Bolus exceeded! + U your suggestion: Max Bolus of + U exceeded or to retain the current units translations: Max Bolus of + U + exceeded

JeremyStorring commented 1 month ago

@MikePlante1 this all looks good to me. I agree with making the text red, and we can use Max Bolus of + U + exceeded to retain translations :)

MikePlante1 commented 1 month ago

That’s the thing, though… changing it means translations aren’t retained (except units). It is a better wording, and there are already plenty of changes that need to be retranslated, so I’ll put this together for a PR. Maybe I’ll combine it with adding a similar feature for Max Carbs.

MikePlante1 commented 1 month ago

Potential solution added in #226