Notification Center #2104
Notification Center #2104
Conversation
|
Here's my take on that : https://github.com/quicksilver/Quicksilver/tree/t/notification-rewrite It planned to 1) kill deprecated stuff 2) handle OS X versions that don't have Notification Center (but we might be way after that actually) 3) hide the modal/notification choice 4) The only thing I don't like (but that's an "Apple API problem") is how you can't use blocks to handle notification replies and have to go through delegation. So you litter code everywhere. Also, indentation ;-). |
Cool. Want me to merge that into this branch?
Agreed. I tried to have just one class be the delegate to keep it contained, but that’s messy, too.
Yeah, OK. Should I do anything about that for this pull? FWIW, I changed my Xcode settings (to tabs) a few days ago. |
| NSUserNotification *locationAlert = [[NSUserNotification alloc] init]; | ||
| [locationAlert setIdentifier:QSLocationChangedUserNotification]; | ||
| [locationAlert setTitle:NSLocalizedString(@"Running from a new location", nil)]; | ||
| [locationAlert setInformativeText:NSLocalizedString(@"The previous version of Quicksilver was located in America. Would you like to move this new version to that location?", nil)]; |
There was a problem hiding this comment.
Haha! It's just Rob testing our code-review quality I guess :P
There was a problem hiding this comment.
Ha! Yeah, uh… I was testing. You passed.
Seriously, no idea what that’s about. Find/Replace gone wrong maybe?
|
Note, I'm currently working on that one. I'm not fond of the hardcoded delegate stuff, so I'm moving the notification-managing to |
As it is not a physical object, it's unlikely that Quicksilver was ever located in America. :-)
As described in #1859, this replaces pop-up alerts with Notification Center notifications where appropriate.
I did not convert:
localizedDescription)All subjective, of course, so let me know if I didn’t go far enough (or went too far).
As part of this, I switched the default notification style for the entire application to “alert”. This is necessary to get the action buttons, but it’s probably not appropriate for the places we were previously using Notification Center (where you’d use Growl or the built-in notifier) because now they all have action buttons and require manual dismissal. I don’t see a way around it, so I’m inclined to change the default for those back to the built-in notifier.
Also, all the strings are localized now (as in available to translators but not actually translated), which is an improvement.
This also does the same thing as #2096. You wanted to know if my “interesting” project was small, Paddy? Let’s ask instead if it’s worth it. 😬
fixes #1859
fixes #1669