Rob--W / stackexchange-notifications

Realtime desktop notifications for Stack Exchange sites.
44 stars 8 forks source link

Added a high-res icon to Chrome version #9

Closed kav2k closed 10 years ago

kav2k commented 10 years ago

Additionally, I recommend updating the Web Store icon.

kav2k commented 10 years ago

Actually, wait. This is a tricky question from the standpoint of http://stackexchange.com/legal/trademark-guidance

kav2k commented 10 years ago

I removed (rolled back) all instances of icon use except for the notification itself. I do believe it plays nice with SE guidelines:

Do feel free to use names or logos for the purpose of labeling our sites within your product, as long as use of such logos could not be confused with the branding or endorsement of the product itself.

Rob--W commented 10 years ago

Could you revert the file name changes as well, squash the commits and add the source of the image in the commit message?

kav2k commented 10 years ago

Trying. Not going well. fumbles with git

kav2k commented 10 years ago

There we go. Done: only the icon changed. Sorry for the fumbling.

Rob--W commented 10 years ago

The image has a huge padding, see http://jsfiddle.net/cAHC4/show/. Can you remove this padding, and show a screenshot of the extension with the old and new image?

The extension prefers the "old" webNotifications API. When this is unavailable (because it's deprecated), the new "rich" notifications API is used. The behavior of "iconUrl" is not specified (http://crbug.com/376456) so I need to see what happens if you set a big image file. I think that a 128x128 image results in a notification with a 128x128 "icon". That would look really odd.

kav2k commented 10 years ago

webkitNotification is deprecated and removed completely already in Stable (Chrome 35) And using chrome.notification stretches the icon to fill the notification icon area.

kav2k commented 10 years ago

With the old icon: notify_old With the new icon: notify_new

Padding was done intentionally to conform to Chrome Web Store guidelines. While this no longer applies, I think padded icon looks better in the notification.

kav2k commented 10 years ago

In addition to WebkitNotification removal, starting with current Chrome Stable they switched Linux builds to Aura, which entails (finally) full functionality of chrome.notifications under Linux.

webkitNotifications were superseded by Notifications API, if you still want to support that.

kav2k commented 10 years ago

Hacked together a Notification implementation. Comparison:

Old: notify_old2 New: notify_new2

Rob--W commented 10 years ago

Looks alright, I will merge it and publish an update.