search

HermesApp / Hermes

Compact macOS Pandora client that doesn’t use Flash
http://hermesapp.org/
MIT License
381 stars 99 forks source link

Adding 'Skip' button to notification (like in iTunes) #273

Closed paullj1 closed 8 years ago

paullj1 commented 8 years ago

Creates 'Skip' button on notifications.

nriley commented 8 years ago

Nice idea! As this uses SPI, please guard it so it doesn't crash if the SPI is removed, and link to any (reverse-engineered) documentation that may help to explain what you're doing.

Thanks.

paullj1 commented 8 years ago

I can definitely add some comments to better explain what's going on, but I'm not sure what you mean by SPI. All of the stuff used is in the Foundation libraries, documented here.

Next commit will contain comments, but for reference: NSUserNotification Center Reversed Engineered Docs

paullj1 commented 8 years ago

Okay, I see now what you were talking about. It should be good to go now.

paullj1 commented 8 years ago

Although, now I'm thinking that it shouldn't matter... since that delegate won't be called if the NSUserNotificationCenter doesn't exist. Thoughts?

paullj1 commented 8 years ago

Okay, so I tested it with OS X notifications on/off, as well as with Growler notifications on/off. The app delegate only gets called when the OS X notifications are on (because the actions are only created in that case).

I also added additional actions if the 'Alert' style notifications are enabled which allow you to thumb up/down (or remove thumb up) from the notification.

Finally, I moved the album art over to the left side of the notification (just like is done in iTunes). Let me know if you want me to create a new pull request, or merge all of these changes into a single commit.

nriley commented 8 years ago

It's fine if this stuff is all in one pull request. Also if this is too much to ask I'm happy to fix it up myself. :-)

paullj1 commented 8 years ago

No worries, it looks like you've done enough for this project! I'm happy to help if I can. Just pushed a try/catch block. Is that what you were looking for?

paullj1 commented 8 years ago

Also, I'm just now seeing your other comments. I'll make those changes as well.

paullj1 commented 8 years ago

Okay, that should do it. Made all of the requested changes. Let me know if you need anything else changed!

nriley commented 8 years ago

Looks good. Catching exception for the key-value coding failures works fine — actually probably a better idea than using -respondsToSelector:.

Thanks again!