Use StateObject in Settings view to avoid parent view updates from re…#558
Use StateObject in Settings view to avoid parent view updates from re…#558gestrich wants to merge 1 commit into
Conversation
…setting the model
Test
Apply modification to LoopWorkspace dev branchSuccess: Issue 2267 is fixed Apply modification to LoopWorkspace tidepool-merge branchSuccess: tidepool-merge works as expected |
|
This worked for me as well! |
StatusI think this test indicates the modification in PR 558 is acceptable. Happy to do additional tests as requested. Test of OnboardingBecause of @gestrich statement "The view will no longer automatically pick up parent‑level updates", I decided to test this with onboarding. Test PlanEach of the numbered tests was performed side-by-side on 2 SE phones running iOS 18.4.
For all tests, after onboarding is completed, add pump (Pump Simulator) and CGM (NS as CGM). Then test for Issue 2267.
Test Results
In other words, the |
ps2
left a comment
There was a problem hiding this comment.
Thanks for the testing, Marion. I'm stil not sure this is the right fix, and I'm hesitant to put this in as there may be other side effects from not publishing settings updates. Interdependent settings, like how corrections ranges affect what you're allowed to enter for suspend threshold, or vice versa, might be affected. It's also a display only bug; the data does get saved, and the workaround is simple; just ignore the non-updated setting.
|
There are changes to how this works coming in tidepool-merge (and Tidepool code that isn't in tidepool-merge branches yet). |
For LoopKit/Loop#2267
This PR tackles the state‑loss problem that arises when TherapySettingsViewModel is instantiated inline inside SettingsView.
What’s happening
View‑model recreation
Because the view model is created inside the body of SettingsView, every SwiftUI refresh tears it down and builds a new one. Any changes to the View Model (e.g. basal‑rate schedule changes) disappears until the upstream data source happens to publish again.
Silent nested mutations (not fixing this - but FYI)
TherapySettingsViewModel.therapySettings is a single @published value. Mutating its nested properties doesn’t emit a change unless the containing struct itself is reassigned, so some updates never reach the UI. It's not clear if this is affecting the refreshes but this may be something to visit in the future.
Scope of this PR
This PR fixes problem 1 by converting the view model from @ObservedObject to @StateObject, anchoring its lifetime to the view hierarchy instead of the render cycle.
Trade‑offs / testing notes
I believe this therapy settings view is used during app setup, so that will need tested too.