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

Add Storyboard Support #109

Closed
wants to merge 24 commits into from

Conversation

tktsubota
Copy link
Contributor

@tktsubota tktsubota commented Aug 23, 2016

Updated 12/31/16: See Comment Below


This adds support for issue #108.

The Two Features

There are two major features in this: the default care plan store (OCKCarePlanStore.defaultStore()) and initWithCoder: implementations to support storyboard usage.

Implementation

I essentially followed what I outlined in #108.

Actual Source

OCKCarePlanStore.defaultStore() is the big one here. I added it as a class method, internally calling initWithPersistenceDirectoryURL: with a directory named "CarePlanStore" in the user's document directory.

initWithCoder: in the view controllers essentially duplicates the existing initializers, without the parameter passed in.

Most of the public properties of the view controllers were made IBInspectable so we can modify those properties in interface builder without needing to subclass.

initWithPersistenceDirectoryURL: in OCKCarePlanStore and initWithCarePlanStore: in the care card and symptom tracker were left to be, so these changes wouldn't be source-breaking and we can still use a custom care plan store if we want. init of the view controllers was made available, using the default store.

Documentation

With these two major features came with a huge change in the programming guides. I switched to using the default store for everything, while still providing an option to create a custom store if need be. The view controller creation in the storyboard was centralized in "Creating The Care Card".

I also had to revert #14's changes to the code-blocks, because when I ran the "CareKit-docs" build target to generate the appledoc, I found out appledoc didn't support the fenced code-blocks and it didn't look good at all. I also fixed broken links.

Test Apps

I wasn't sure if I should have used the singleton throughout the entire OCKTest (and even OCKSample), so I left it be and made a separate "Storyboard" test in OCKTest, where every view controller there is instantiated in the storyboard and uses the default store.

Unit Test

I also added a unit test that duplicates some of the existing care plan store test, just to make sure the default care plan store was working.

Bugs I found

  • When changing the tint color on the care card, the heart image color does not change. I know of a fix, but I didn't put it in this PR as it didn't seem to relate.

Future Improvements

  • We should start moving toward using the default store in the documentation, test apps, and so on. This should be the recommended way of using the care plan store, as there aren't a lot of reasons to use a custom URL.
  • We should consider switching from using appledoc to Jazzy. While they just recently got support for Objective-C, it looks more like the current documentation (although the look of documentation changed with iOS 10) and has a larger community. I noticed a lot of bugs and missing features in appledoc that I had to work around.

I expect a lot of changes from code review, as this is a major feature (and lots of changes in the programming guides).

@tktsubota
Copy link
Contributor Author

tktsubota commented Dec 31, 2016

Updates to finish off the year...

Xcode 8 Class Property

Xcode 8 came with the ability to create class properties in Objective-C to better interoperate with Swift. I converted the default store to a class property named defaultStore. It can be accessed through:

  • Objective-C dot notation: OCKCarePlanStore.defaultStore
  • Objective-C bracket notation: [OCKCarePlanStore defaultStore]
  • Swift: OCKCarePlanStore.default (NOT defaultStore)

Everything else related to the singleton remained the same.

Swift 3 OCKTest Update

The storyboard files were updated to Swift 3 and the default store property.

I also noticed that there are quite a few crash bugs in OCKTest that should be fixed.

Removed Documentation Changes

To narrow the scope of this PR, I removed the changes in documentation. They can be put back in (with the power of git) if need be.

@tktsubota
Copy link
Contributor Author

@umerkhan-apple Any updates on this PR?

@umerkhan-apple
Copy link
Contributor

@tktsubota Putting this PR on hold for now. Will test internally with the rest of the team and get back to you.

@tktsubota
Copy link
Contributor Author

Reopening later.

@tktsubota tktsubota closed this Jun 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants