Enable editing of preset profile overrides

[dsnallfot]

• Enable edit of all settings and/or name for already stored profile overrides
• Swipe left to edit or delete
• Get list of all settings included in the override when saving a new one or editing an existing one

Suggested solution for issue #234

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

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	Settings summary before saving

[dsnallfot]

Short video demo

Simulator.Screen.Recording.-.iPhone.13.mini+.-.2024-05-25.at.00.49.35.mp4

[dnzxy]

The UI on this really is not intuitive. A swipe action implies that the selected element will be edited and that an action will prompt past the point of action, here the swipe. A user would expect to swipe and tap the edit button and then be forwarded to an editing mask or something similar where the data of the preset is prefilled and the you made in the „root“ view are then done. Here, the edit action actually saves data that was edited before.

I like that this gets some love, but also want to urge that we held of on any UI work past 1.0.0, and well, this is UI work.

[dsnallfot]

I'd say it's more UX than UI work to be able to edit stuff. But I totally agree. To make this intuitive the settings and slider should get moved into the popup.

What's even less intuitive is not being able to edit an entry at all. To have to delete it, and then recreate it with new settings and the same name (but other id).

But I guess you already have implemented all this functionality in your upcoming UI rework (that I have not seen myself) that will take place after 1.0?

Well. Then we can close both those PRs for just mine and Auggies personal use for now and wait until the big UI boom after 1.0 solves all outstanding UI/UX-issues. 😊

[dnzxy]

You are totally right, it’s UX, I just always see UI/UX as a whole. Adding UX usually also involved changing the UI (here introduce a new view and swipe actions).

Also fully agree not being able to delete it at all is unintuitive, yes. If this can be made so that you trigger a subsequent edit mask or the current course of action is more transparent, e.g. Save or select preset to edit? How about that?

We haven’t tackled the profiles section with the specific edit functionality you have proposed here, but just moved things and are mostly working on persistence layer.

Can we leave this open as a draft and move the idea of this over once we are at that point? I really want to get something like this in, but we need to make use of design elements and HCI as intended and here the swipe action ought to trigger an edit view view or a subsequent action 😊

[dsnallfot]

Closed for now. To be revisited in the future after 1.0 release.

[aug0211]

Thanks Daniel, I’m definitely still using in my personal fork. Not being able to edit overrides/temp targets has been a nightmare for me with, creating an huge amount of toil.

[dsnallfot]

Reopen and put in the "idea box" until after 1.0 release.
Will continue to push a few commits to this feature branch until then. (To move the settings adjustment inside the popup for instance)

[avouspierre]

Amazing @dsnallfot ! I proposed some changes to be more in editing mode like UX Apple. The idea is to start by choosing "editing" mode, to modify all the value and to save after. I also added a cancel button.

The example is here :

Video	Image

I currently do the update in the PR #219 but depends what the trio's community prefer !

[dsnallfot]

That's a beautiful solution Pierre! Much better than my quick draft.

I knew you were working with overrides, but not this edit things (I actually checked your PR to make sure there wasn't that much conflicts in override state and root.

Very nice!

[dnzxy]

@avouspierre ‘s way of editing is great and uses the swipe action just like I would expect it to in iOS. Great job!

Edit to add; could we split that from your current override PR and add it here? Would make the scope smaller and then it’s just the edit feature 😊

[dsnallfot]

I think Pierres implementation is more promising, advanced (and prettier), but I at least in this draft 2.0 I fixed the backwards workflow by moving the settings inside the edit popup view

Edit and delete	Edit pop up

Video

Simulator.Screen.Recording.-.iPhone.13.mini+.-.2024-05-25.at.23.35.28.mp4

[dnzxy]

I like the latest proposal by you, @dsnallfot , more than the version @avouspierre has proposed.

Pierre's version is equally as nice as yours, as both of you use the swipe action to toggle a subsequent action. However, your version opens a sub-view, pre-populates the values for the various inputs and options, asks the user to change something, then the user saves, view disappears, finished. It's a very nicely guided, "holding the user's hand" type approach.

Pierre's approach loads the preset values into the inputs / options in the root view, making it harder to grasp that "something" has happened or changed.

I must admit, I prefer your option and I'd even go as far as proposing that the view you use to edit a preset should actually also be the view to add a preset and/or start a non-preset profile override. So ideally the profile override root view would consist of a big Add or enact button, or a big [+] button, and the preset list. If there are no profiles stored/created yet, the list could show a little hint or simply display "No presets found. Proceed to create preset or enact profile override." This would also shrink the entire root view, getting rid of the scroll view to get to the start / add / cancel buttons. Imho, whenever a user needs to scroll, the view has potential to be refined so that they don't have to (unless it's settings, long text, lots of data displayed etc.)

Either way, both proposals are a huge improvement over what is currently there.

[bjornoleh]

Looks good. Would it be possible to add a confirmation before deleting? It’s pretty easy to accidentally delete when the intention was to edit?

[dsnallfot]

Looks good. Would it be possible to add a confirmation before deleting? It’s pretty easy to accidentally delete when the intention was to edit?

Of course possible, I use it in my personal build. BUT there is an annoying UI glitch with the row disappearing and quickly reappearing when the alert is triggered. I can live with that in a personal build, but not good enough for the official fork.

If we feel that an alert is important to add, I can dig further in this later on (remember that Dan or someone fixed the exact same glitch when deleting in datable (history)

Demo of my glitchy personal implementation as of right now (Sorry for Swedish text, didn't have my Norwegian version up n running right now)

Simulator.Screen.Recording.-.iPhone.13.mini+.-.2024-05-26.at.09.17.58.mp4

[dsnallfot]

Nevermind! Figured it out. Just needed to set the swipebutton to 'role .none' instead of '.destructive'. See new demo.

Do we want this alert?

New glitchfree demo

Simulator.Screen.Recording.-.iPhone.13.mini+.-.2024-05-26.at.09.29.02.mp4

[dnzxy]

Revisiting this PR today and looking again at my proposal from yesterday (quoted below)

I must admit, I prefer your option and I'd even go as far as proposing that the view you use to edit a preset should actually also be the view to add a preset and/or start a non-preset profile override. So ideally the profile override root view would consist of a big Add or enact button, or a big [+] button, and the preset list. If there are no profiles stored/created yet, the list could show a little hint or simply display "No presets found. Proceed to create preset or enact profile override." This would also shrink the entire root view, getting rid of the scroll view to get to the start / add / cancel buttons. Imho, whenever a user needs to scroll, the view has potential to be refined so that they don't have to (unless it's settings, long text, lots of data displayed etc.)

I think we should make that a second PR. This PR looks very feature-complete (full editability, deletion with alert for consistency with history view) and thus should be tested, approved and merged.

In a subsequent PR we can build on this and make the creation / enactment of a profile along the lines of my proposal - if that is something we want to move forward with. I‘d vote yes (naturally, haha), as it’s an iOS-esque change to creating something and also similar behavior to how basal profiles and other types of "listed data" is created in the app.

Further more, the changes from this PR would also need to be ported over to #236 so that we have consistency there (already done by Daniel ✅).

I do have a second follow-up proposal about profiles and TTs, but let’s discuss that when the time comes 😊🙏

[avouspierre]

I checkout the code and test the new functionalities. All functions "add,edit, delete" preset profile are OK.

Some improvements for maintenance POV @dsnallfot :

• move the function populateSettings in the OverrideProfilesStateModel
• use targetValue.asMmolL instead to convert manually (populateSettings func line 519)
• refactor the code between populateSettings and savedSettings functions to avoid duplicate code
• could be better to have a function in the StateModel to populate the edit view (profilesView - move line 399 to 413 to a statemodel function like loadProfile(idPreset: String).

Functionaly, a proposal : perhaps not apply the edited profile as a current profile ?
Example with the video :

• Current Test profile
• Editing the test#2 profile > Cancel
• The content of the test#2 replace the test

[dsnallfot]

Thanks Pierre! Will go through all your suggestions and try to adjust accordingly.

[dsnallfot]

Will soon commit all changes based on all your input regarding both refactoring and improved functionality @avouspierre , thanks again for great feedback!

And below is a demo of the improved functionality regarding what pre filled settings are used in the view.

1. I've made .onDisappear from the editPresetPopover back to root view behave the same way as .onAppear in root view when coming from home screen, that is:

• If ongoing override, populate settings with the ongoing override setting.
• If no ongoing override, populate settings with user defaults

2. And when entering edit by selecting edit on any override -> populate the settings in edit mode with that selected overrides settings as starting point for the edit

Do this look ok?

2024-05-27_08-34-19.mp4

[dsnallfot]

Now we are hopefully starting to see the finish line for this PR.
Regarding refactoring and avoiding duplicate code (note: there are som duplicate code in state model since before that I didn't touch) - and for new functions, I've tried the best I could to make the code ok and taken it as far as I can with making it pretty and maintainable.

If any more adjustments regarding maintenance and duplicates/code efficiency is needed, I really need help from anyone of you guys.

Well. Every change and improvement needed (so far) is committed in the PR now, and you could start testing when it suits you.

And the screenshot below is the "one more thing commit" that I almost forgot to add, the slider & percentage order switcharoo @dnzxy made in iAPS after 2.3.3 😊

[dnzxy]

Looking superb. Great job.

[bjornoleh]

Tested successfully on a phone.

[dnzxy]

Tested in a simulator phone. Looks good and works as intended.

Out of scope for this PR, but the data management really needs an overhaul but this is being addressed by @avouspierre in #238 to an extent.

🚢 it to add editability.

[Sjoerd-Bo3]

Tested this with a live device for several days, edited some profiles and found (apart from user error) no deviations.

[dnzxy]

Merging with 4 approvals.