Skip to content

Conversation

@bharat
Copy link

@bharat bharat commented Dec 24, 2016

Use KVO to watch the UserDefaults object and update the view as
appropriate. This is a first pass at resolving #320.

Use KVO to watch the UserDefaults object and update the view as
appropriate.
self,
forKeyPath: defaults.statusExtensionContextObservableKey,
options: [.new, .initial],
context: nil)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do. The purpose of the context member was pretty opaque to me. I'm assuming it's some kind of semaphore that lets you know that the notification is relevant to this particular instance.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it legit to use the context variable (defined as var context: StatusExtensionContext?) for this purpose? It's convenient. If not, I'll create a new var.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't, foremost because it's convoluting the term "context" for future readers, but secondly because you're looking for a unique pointer value here, and it should feel safer to instead use one containing a simple integer value that never changes.

defaults.addObserver(
self,
forKeyPath: defaults.statusExtensionContextObservableKey,
options: [.new, .initial],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you aren't using the change dict in the observer than you should leave these blank.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you elaborate on that? Not sure what you mean by using the change dict in the observer.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

passing the .new key here is so it is included in the observeValue call below. .initial means the observer is called right away, which isn't what you want.

}

override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) {
update()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always check the context. If it doesn't match, then you must call super.

subtitleLabel.textColor = UIColor.secondaryLabelColor
}

override func viewWillAppear(_ animated: Bool) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be called multiple times, and disappear can as well. These two aren't always balanced in the case of an interactive transition.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, that's good to know. I'll move these to viewDidLoad() and deinit, following the pattern in PredictionTableViewController

1. call addObserver() in viewDidLoad()
2. call removeObserver() in deinit
3. don't specify options in addObserver()
4. provide a context to the observer code
@bharat
Copy link
Author

bharat commented Dec 24, 2016

Ok - I think I covered all the issues you raised. Thanks for the guidance!

I'm not sure how to properly test this in the simulator, because when I go to the lock screen, the Loop app isn't updating so I don't see any changes. Any thoughts on how to test it in the simulator? If not, I'll deploy it to my phone and just observe it for 5-10 minutes :-)

}

override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) {
if context != &self.context {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Early returns should really only exist in guard statements.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. fixed!

@ps2 ps2 merged commit d540cce into LoopKit:dev Jan 2, 2017
@bharat bharat deleted the observe branch January 2, 2017 21:44
ps2 pushed a commit that referenced this pull request Nov 25, 2020
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.

3 participants