[LOOP-5328] Fix Deeplinking and Widgets #786
Conversation
| struct AppSource: Hashable { | ||
| let name: String | ||
| let bundleId: String? | ||
| } |
There was a problem hiding this comment.
@ps2 this is something I was prototyping around with; a way to pre-fill the carbs value via a deepLink. Lmk your thoughts!
There was a problem hiding this comment.
Deeper value here could be the ability to observe carb entries from 3rd parties and then have Loop notify you to bolus for it. Or having a Siri Shortcut for common carb entries.
There was a problem hiding this comment.
BundleId here could allow to query the app icon from the App Store API if we wanted to display it 🤷🏻♂️
| if let source = viewModel.carbsSource { | ||
| HStack(spacing: 2) { | ||
| Text("Source") | ||
| .foregroundColor(.primary) | ||
| .frame(maxWidth: .infinity, alignment: .leading) | ||
|
|
||
| Spacer() | ||
|
|
||
| Text(source.name) | ||
| .foregroundColor(Color(.secondaryLabel)) | ||
| } | ||
| .accessibilityElement(children: .combine) | ||
|
|
||
| CardSectionDivider() | ||
| } | ||
|
|
There was a problem hiding this comment.
This is a way I thought we could display the source in the CarbEntryView if it's available:
There was a problem hiding this comment.
We could also make editing disabled if presented via deeplink
There was a problem hiding this comment.
I'm not sure we'd want to disable editing? Seems like that should always be a good thing, for cases where the originating app might not be correct? Or the user changes their mind?
Loop/Managers/LoopAppManager.swift
Outdated
| .environment(\.settingsManager, settingsManager) | ||
| .environment(\.temporaryPresetsManager, temporaryPresetsManager) | ||
| .edgesIgnoringSafeArea(.top) | ||
| let statusTableView = StatusTableView(viewModel: viewModel, displayGlucosePreference: deviceDataManager.displayGlucosePreference, settingsManager: settingsManager, temporaryPresetsManager: temporaryPresetsManager) |
There was a problem hiding this comment.
Moving environment modifiers into the view itself make type matching the type-erased viewController much easier.
There was a problem hiding this comment.
Honestly not a huge fan of this. I understand why you're doing it; so you can cast it later, but having to cast also seems like a smell.
It seems like the deep link manager should probably already have a reference to the the status table view controller, rather than having to dig through the view stack, and unwrap/cast things.
It looks like LoopAppManager.handle is the entry point, passing the url off to the deeplinkManager. LoopAppManager also creates StatusTableView, can we get a ref to the status table view controller through there?
There was a problem hiding this comment.
It seems like all DeeplinkManager is pass through, so I don't think that really justifies another manager layer. Same with RootNavigationController. LoopAppManager/AppDelegate is the main entry point; and the StatusTableViewController has the real logic, and LoopAppManager should be able to retain a ref to StatusTableViewController/StatusTableView
There was a problem hiding this comment.
@ps2 Alright -- I made some updates. Is this closer to what you'd expect?
|
|
||
| /// The root view controller in Loop | ||
| class RootNavigationController: UINavigationController { | ||
|
|
||
| /// Its root view controller is always StatusTableViewController after loading | ||
| var statusTableViewController: StatusTableViewController? { | ||
| return viewControllers.first as? StatusTableViewController | ||
| return (viewControllers.first as? UIHostingController<StatusTableView>)?.rootView.viewController |
There was a problem hiding this comment.
Here is where the type erasure makes things more simple.
| case .carbEntry: | ||
| if let value = components?.queryItems?.first(where: { $0.name == "value" })?.value, let doubleValue = Double(value) { | ||
| let sourceBundleId = components?.queryItems?.first(where: { $0.name == "sourceBundleId" })?.value | ||
| let sourceName = components?.queryItems?.first(where: { $0.name == "sourceName" })?.value |
There was a problem hiding this comment.
I'm wondering about possible misuse of this; any app could send a spoofed name to make the carb entry look it it came from a different app.
https://tidepool.atlassian.net/browse/LOOP-5295