Wp-calypso: Store: Review notification

[justinshreve]

Right now you can get a normal comment notification for a review (since WooCommerce treats these as no different to a normal comment or a post).

As part of our notifications work, we should hook into the comment builder (wp-content/lib/notes/class.notification-builders-v1-1.php see build_comment) , and make our own version of the notification with some adjusted copy to make it clearer these comments are product reviews.

[justinshreve]

A patch for this is now up at D8337-code. It adjusts the copy and display of the comment notification that is already generated.

Here are a few screenshots of what I have so far:

Desktop

iOS

Could use some design feedback. I had something slightly different originally but needed to make some changes to make sure things worked mostly consistently across iOS & desktop. The rating display is currently using emoji, but we could generate some images for stars and load those instead.

cc @kellychoffman @timmyc @jameskoster

[kellychoffman]

The rating display is currently using emoji, but we could generate some images for stars and load those instead.

Are we allowed to use gridicons here? Asking because this would allow us to denote 4 stars out of 5 using the stars + start-outline.

I'm also thinking the 'Review for' and the 'Rating' need to be above the comment for better context.

[justinshreve]

I don't think the Gridicons will work. I remember having issues with it in the expanded view of the order notification (the one being displayed there is an image), and it just came up as a question mark box.

However, we can have 5 separate images, and just load the correct one based on the rating.

i.e.

rating-1.png

and so forth.

Would that work?

[kellychoffman]

Yep, that totally works.

[jameskoster]

Eeesh using an image makes me a sad panda. I think I'd actually prefer emoji to images but it's a minor point.

Assuming we go with images, we should be sure to make it fully transparent to account for any potential changes in background color.

[kellychoffman]

I don't feel strongly about the images, I just wanted to be consistent with other areas of Calypso that use the gridicon stars.

Example in /plugins:

@justinshreve: The color used is blue-medium, which is #00aadc.

[justinshreve]

So it turns out that images don't work well either. They get forced aligned center on mobile.

However, Normal unicode characters seem to be working OK on desktop and mobile, which has a filled and unfilled star. Unfortunately I don't have much power over the color, but with the stars linked, it's a lot closer to the rest of Calypso than the emoji.

[kellychoffman]

Ding ding ding. We have a winner!

[timmyc]

Okay here are some screen grabs from Android-land:

Not sure why the default gravatar is not showing up... and the underlining of "Review For" prior to the linked product name looks a little odd.

[justinshreve]

@timmyc Thanks for testing.

I fixed up the display of the review for area and tested with an Android emulator (which was annoying to setup, but made testing tweaks there easier :P).

I can't seem to solve the Gravatar issue. The Gravatar displays on Android if one is set, but the mystery man fallback will not.

This seems to be an issue with regular comment notifications as well:

I think that is worth a bug report to Android because it's definitely odd. But I don't think it's a blocker for this, since reviews already come in as comments and have the same problem.

If that sounds OK to you, do you mind giving the latest revision a 👍?

[timmyc]

I agree regarding the gravatar issue, that should just be reported upstream. So the latest iteration is looking great to me - here is the notif:

I did notice something a bit odd though - the lock-screen version of the notification is still showing as a Comment:

In the list-view of the App, things look fine:

I wonder if there is some other code that triggers the push notification that runs on a job server, and as such still send the initial push notif as a comment. Seems like the easiest way to test this assumption is to merge in the diff and give it a try :)

[timmyc]

Also just noticed that the default gravatar works on the push notif too

[justinshreve]

Yeah merging the diff should correct the push notification. We only see the new content in the app since public-api.wordpress.com is sandboxed, but the notification is being sent with production code.

[justinshreve]

This has been launched in r166039-wpcom.