fix survey, contact, and task view tint colors

[cbaker6]

When an app is using the SwiftUI lifecycle (App protocol) and doesn't use the scene delegate, the tintColor isn't properly inherited in surveyViewController in OCKSurveyTaskViewController from the calling view. This occurs with the latest commit on the main of CareKit and the latest commit on the main of ResearchKit (https://github.com/ResearchKit/ResearchKit/commit/a89059f5dc2a91f36785b184f914b5e5fdb05877)

The fix is to set the tintColor directly when setting up the ResearchKit task. This works for the most part with the exception of the ResearchKit Done button and some others (selecting a survey option) still don't inherit the color from the calling view. I believe the button issue is a ResearchKit issue and not a CareKit issue. When the ResearchKit Done button is disabled because the required questions hasn't been answered, the button shows the correct tintColor.

Since the app no longer depends on UIWindow; ORKWindowTintcolor doesn't work properly for a SwiftUI lifecycle based app, forcing everything to the default tintColor not set by the SwiftUI based app. This also causes issues when displaying a contact or tapping in other items in a list view. The problems using ResearchKit (https://github.com/ResearchKit/ResearchKit/commit/a89059f5dc2a91f36785b184f914b5e5fdb05877) look like below:

The button still has the wrong color

The disabled button still has the correct color

The enabled button still has the incorrect color

The selected answers still have the incorrect color

[cbaker6]

The latest commit should work for both the AppDelegate and App lifecycles

[cbaker6]

If https://github.com/ResearchKit/ResearchKit/pull/1519 is merged, all colors show correctly when using the latest commit of this PR.

The fixes result in the following:

The button now has the correct color

All have the correct color

The enabled button now has the correct color

The selected answers now have the correct color

[gavirawson-apple]

Very interesting bug! And nifty fix.

I've tested this locally and it does seem to fix the issue. I do have one concern - the initial tintColor is passed on to the presented view, but subsequent changes to the tintColor will be ignored. Consider this code, which simulates a scenario where the tintColor is changed.

@State
private var accentColor = Color.red

var body: some View {

    // Wrapper around OCKDetailedTaskViewController
    TaskView()
        .accentColor(accentColor)
        .onAppear {

            // Simulate changing the accent color. The change is not picked up by the presented detail
            // view controller if it is already presented.
            DispatchQueue.main.asyncAfter(deadline: .now() + 2) {
                accentColor = .purple
            }
        }
}

I'm not entirely sure how often our developers are changing the tintColor dynamically in their apps, but it does feel like something we should support. What do you think?

It seems to me like the core issue here might be architectural - A CareKit SwiftUI shouldn't require developers wrap our UIKit view controllers. There should instead be handles that are geared specifically for SwiftUI. That's something we're currently working on, and I think it will fix the issue you're seeing here. While that's in the works, this solution seems to be a great patch. But perhaps we shouldn't bring this into the framework? Curious what you think.

[cbaker6]

I do have one concern - the initial tintColor is passed on to the presented view, but subsequent changes to the tintColor will be ignored. Consider this code, which simulates a scenario where the tintColor is changed

I agree, but I think the same would happen when window tintColor changes. Both require the view to be dismissed and presented again to adapt the color update. There might be an exception for some ResearchKit presented views as many of those views implement tintColorDidChange, so those might update when the window tintColor changes, so this fix wouldn’t be equivalent.

I'm not entirely sure how often our developers are changing the tintColor dynamically in their apps, but it does feel like something we should support. What do you think?

I think this would be neat. I haven’t had a need for a dynamic change of tintColor as I’ve only needed to match the theme of the app, which I wouldn’t change while a survey is being completed, but I can imagine usecases for this.

It seems to me like the core issue here might be architectural - A CareKit SwiftUI shouldn't require developers wrap our UIKit view controllers. There should instead be handles that are geared specifically for SwiftUI. That's something we're currently working on, and I think it will fix the issue you're seeing here.

Definitely agree here, if all of the views were SwiftUI based views, this wouldn’t be needed. I will say I really like the UIKit views (e.g OCKTaskViewController that presents an ResearchKit UIKit view) and I imagine making the already designed views into SwiftUI views takes a lot of reinventing the wheel and increase code maintenance. In addition, turning the current views into SwiftUI views may cause some issues for UIKit developers.

While that's in the works, this solution seems to be a great patch. But perhaps we shouldn't bring this into the framework? Curious what you think.

For me, this works as I typically adapt to the latest code on the main branch when it’s available, but dev teams who don’t have the resources to adapt quickly will be stuck with aesthetic issues in their apps that they can’t address with out this fix. One option could be to add this fix, release an official minor/patch update, then release any SwiftUI based updates in a major update. This will allow dev teams to choose what works for them at the time and upgrade to the latest version when they are ready

[cbaker6]

@gavirawson-apple it could be helpful to create a helper method to accomplish the tintColor updates, similar to the ResearchKit fix https://github.com/ResearchKit/ResearchKit/pull/1519#issuecomment-1329491302.

I've added this helper method to UIViewController+Extensions.swift. The changes are in https://github.com/carekit-apple/CareKit/pull/676/commits/74f7e5829cc6c3e00159eeb1b294f78e5cb91bbf. The new helper method will use the window.tintColor if one is available in the respective hierarchy of the current ViewController, or else use the tintColor of the passed in view. This preserves the current functionality of the old app life-cycle and allow the tintColor to be set in SwiftUI life-cycle apps when a CareKit UIViewController is presented.

Assuming the CareKit team believes this is a good fit, I imagine a heuristic could be to call this method to determine the tintColor on any new UIView that is presented inside CareKit moving forward. Though from your comments in https://github.com/carekit-apple/CareKit/pull/676#issuecomment-1317821006 it seems most future CareKit views will be SwiftUI views, so they wouldn't need to use the heuristic unless they eventually present a UIViewController such as an ORKTaskViewController.

[gavirawson-apple]

Thanks for the updates Corey. I'm swayed - if folks are indeed wrapping our UIKit view controllers at the moment, we should support them in the short term. In the long term we'll explore the architectural fix I mentioned earlier, and then I believe this will no longer be necessary.

Can we leave some breadcrumbs to remind ourselves the reasoning behind this explicit tint color modification? Maybe consolidate setting the tint color in the view controller to a single block, and place a comment above it. That way we can come back to it after were explore an architectural change. Something like:

// Explicitly setting the tint color here to support current developers that have a SwiftUI lifecycle app and wrap this view controller in a `UIViewControllerRepresentable` implementation...Tint color is not propagated...etc.
detailViewController.view.tintColor = determineTintColor(...)

After that, let's merge away!

[cbaker6]

Can we leave some breadcrumbs to remind ourselves the reasoning behind this explicit tint color modification?

Sure, I can add this!

Maybe consolidate setting the tint color in the view controller to a single block, and place a comment above it.

I'm not sure how we can consolidate any further without writing extra code as there are a couple of different places in the code that present UIKit views that don't propagate the tintColor. I can add the comment to each change I made so it's marked for removal in a future update. Let me know if you have a better way to do this than the way I mentioned.

[gavirawson-apple]

That sounds perfect, thanks Corey.

[cbaker6]

@gavirawson-apple I marked it with a TODO, let me know if this works.

[gavirawson-apple]

Looks great, thanks for all of the work here Corey.