Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use asset when available in detail card view #579

Merged
merged 13 commits into from
May 7, 2021

Conversation

cbaker6
Copy link
Contributor

@cbaker6 cbaker6 commented May 5, 2021

If a card is displaying an OCKAnyTask it will automatically display the asset tied to the task when details are tapped. Close #524

@erik-apple and @gavirawson-apple I couldn't find testcases where I should add tests for this. Let me know where to look and I can add them if needed

@erik-apple
Copy link
Collaborator

I wonder if perhaps a better approach here would be to include an optional asset property on OCKAnyTask. Then no type casting would be required and we'd still get the desired behavior.

@cbaker6
Copy link
Contributor Author

cbaker6 commented May 5, 2021

I wonder if perhaps a better approach here would be to include an optional asset property on OCKAnyTask. Then no type casting would be required and we'd still get the desired behavior.

I was going to do this, but I didn't see asset in OCKAnyTask, let me look again...

@cbaker6
Copy link
Contributor Author

cbaker6 commented May 5, 2021

It looks like asset isn't part of OCKAnyTask from what I can tell (I think this may have happened when OCKObjectCompatible was removed). Do you want me to move it there to see if it will break anything? It seems like it shouldn't.

@cbaker6
Copy link
Contributor Author

cbaker6 commented May 5, 2021

Looking further back, it looks like the only OCKAny to have asset was OCKAnyContact and the other entities gained it through conforming to the OCKObjectCompatable protocol

@cbaker6
Copy link
Contributor Author

cbaker6 commented May 5, 2021

The last commit assumes UImage(named: "") will always will return nil which I don't know if that's true

@cbaker6
Copy link
Contributor Author

cbaker6 commented May 5, 2021

Let me know if you want me to remove the AppIcon asset from being used for this in the sample app. I wanted to provide an example without adding additional assets.

@erik-apple
Copy link
Collaborator

Great work Corey! LGTM!

@erik-apple
Copy link
Collaborator

I like that this will add an example of how to use SFSymbols to the sample app!

@cbaker6
Copy link
Contributor Author

cbaker6 commented May 6, 2021

I like that this will add an example of how to use SFSymbols to the sample app!

I was hoping to find some SF symbols to match some of the other tasks, but didn't see any other good matches.

@erik-apple erik-apple merged commit 6ac64df into carekit-apple:main May 7, 2021
@erik-apple
Copy link
Collaborator

Thanks for the PR Corey! 🎉

@cbaker6 cbaker6 deleted the linkAssetToTask branch May 7, 2021 15:33
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue adding an image to an OCKChecklistTaskViewController card
2 participants