nightscout / Trio

Trio - an automated insulin delivery system for iOS based on the OpenAPS algorithm with adaptations.
https://docs.diy-trio.org/en/latest/
MIT License
80 stars 414 forks source link

Enable editing of preset temp targets #236

Closed dsnallfot closed 4 months ago

dsnallfot commented 4 months ago

Connected to issue #234 and PR #235

Extra useful when preset temp targets are used with shortcuts (no need to re add changed presets in shortcuts since id remains unchanged after edit - even if you change name on the preset)

NOTE! This is not an attempt to redesign the entire UI. This is just to add edit functionality with minimal changes.

Screenshots Edit or Delete swipe Edit View
Simulator Screenshot - iPhone 13 mini+ - 2024-05-25 at 09 46 13 Simulator Screenshot - iPhone 13 mini+ - 2024-05-25 at 09 46 19

Video demo

https://github.com/nightscout/Trio/assets/72826201/72ac5941-af02-4232-8e6f-e49f366b54ca

dsnallfot commented 4 months ago

Bug: just figured out that md/dl values are saved as decimals instead of integers when editing presets, which causes unwanted decimals in temptargets_presets.json file when mmol is converted to mg/dl and not rounded. Will fix this shortly.

dnzxy commented 4 months ago

Please see my comment on #235, the same is applies for the UI of this proposed change.

dsnallfot commented 4 months ago

Closed since we should wait with all UI/UX changes until after 1.0 release.

aug0211 commented 4 months ago

I like this UX, it’s more intuitive than the override version (editing in the popup is more natural to other experiences on iOS)

I think swiping is clear enough, better than a long tap! I guess a dedicated “Edit” mode could be added in the top corner; maybe that’s more iOS standard? I like the swipe for options though, personally. I could see additional swipe functionality being added eventually, too, such as “duplicate” to save into a new entry with identical settings as the starting point to modify.

I’m very eager to get people who are ready and able to do UX and UI work unblocked - UI/UX is perhaps the weakest point of Trio currently (maybe behind Caregiver experience) 🙌

dsnallfot commented 4 months ago

Reopen and put in the "idea box" until after 1.0 release

dsnallfot commented 4 months ago

The mg/dl decimal rounding bug should be fixed now

dsnallfot commented 4 months ago

The same alert after swiping delete logic as in profile overrides PR now also implemented for TTs in this PR

https://github.com/nightscout/Trio/assets/72826201/74e497db-2789-40ea-8335-e178e684e8e1

avouspierre commented 4 months ago

The edit mode doesn't allow to use the experimental approach. Probably not useful as asked in #154

dsnallfot commented 4 months ago

The edit mode doesn't allow to use the experimental approach. Probably not useful as asked in #154

Thanks Pierre, Good spotted! You can edit temp targets set by the slider, but only target and duration (no experimental hbt slider in edit mode. That was a compromise I choose to make but didn't communicate clearly in the notes)

But I also noticed when testing right now this that I need to fix the mgdl-mmol conversion for targets set by slider to be able to edit them correctly. Right now only mgdl works as intended. Will fix tonite regardless of what happens with PR 154

dnzxy commented 4 months ago

There’s quite a lot of discussion in #154 and I don’t think the slider should be removed but rather fixed (as proposed by @mountrcg ) . In going with that, the slider would need to make a reappearance here for the TT edit view.

dsnallfot commented 4 months ago

Ok. Sounds reasonable for consistency. Should be doable

mountrcg commented 4 months ago

@dsnallfot can I make a proposal for the slider, which will not be based on changing HBT with slider as currently, but changing the ratio / insulin percentage as also depicted on the slider. refer to: https://github.com/mountrcg/Trio/commit/7974cc252ca0d66070f7403410dd3f2ba92dc11f

mountrcg commented 4 months ago

What would be great to save HBT in settings instead of pushing it as an extra variable to oref. I don't like this setup as it basically contradicts what is written in a preferences. In prefs currently the 160mg/dL resides and for an adv. TT oref gets a second HBT value that overrides the current setting. Not so good. What do you guys think?

dnzxy commented 4 months ago

Please do @mountrcg but in a separate PR. Daniel’s PR here can introduce the editability of presets and slightly change deletion behavior (alert, swipe action), but leaving logic as-is. You can fix / propose a change around the slider either in #154 or in a new PR 😊 let’s please not scopes here.

dsnallfot commented 4 months ago

Demo with latest (last?) changes. Experimental slider now available in edit mode

Video

https://github.com/nightscout/Trio/assets/72826201/039d0427-cb0f-4776-9f9f-ef0cb3bad1d7

EDIT: Will also fix sliders % populating in edit view based on set targetsFIxed. Video demo updated

dnzxy commented 4 months ago

Merging with 4 approvals.