search

Automattic / notifications-panel

Core notifications panel for WordPress.com notifications
0 stars 1 forks source link

Add support for checkmark icon #197

Closed dmsnell closed 6 years ago

dmsnell commented 6 years ago

Despite having the checkmark Gridicon checked-in to the project the mapping from noticon index was missing in noticon2gridicon.

This patch adds the mapping and updates the patch version. No changes to the code or operating are affected by this: it's only a bug-fix.

Testing

Find a note with the checkmark icon in it and see if it displays as a checkmark or as an info i

Before

screen shot 2017-11-15 at 5 26 53 pm

After

screen shot 2017-11-15 at 5 26 30 pm

Creating a checkmark note

Open up any note building in your sandbox and change the icon type to checkmark

Also, Jetpack monitor and auto-update notices use the checkmark, so it could be possible to create one by taking down a Jetpack site temporarily.

dmsnell commented 6 years ago

This should coordinate with #196 if we add the changeling; also we can probably combine this and that one into one version bump if we want; they are both very small changes I think.

gwwar commented 6 years ago

Changes here look fine, but could you add instructions on how to generate a note with a checkmark? @dmsnell we're also on a 2 week release schedule. So 2nd and 4th Wednesdays of the month. Did you need this sooner?

dmsnell commented 6 years ago

So 2nd and 4th Wednesdays of the month. Did you need this sooner?

Is that today? that's more up to @singerb and those of us with activity log work. I guess if it's not problematic to do an off-interval update I don't mind releasing it. if problematic we can wait.

gwwar commented 6 years ago

For this month it'll be the 22nd. I'll leave it up to @kwight if he has extra cycles or not :)

dmsnell commented 6 years ago

rebased this again #196 so I could add the content to the changelog.

@kwight is there anything that you normally do in a release that I could do for you here?

singerb commented 6 years ago

I don't think it's super problematic to wait, especially since there are existing notifications already living with this. Since they continue to work, no code changes are needed on the notification creation end, and the info icon is a reasonable substitute, this can probably be released when appropriate.

kwight commented 6 years ago

Since we don't have anything on beta right now, I'll cut these two PRs into a beta today or tomorrow, and if there are no issues, we can then use next week's 22nd release to move it to prod.

kwight commented 6 years ago

Didn't get to it today, but will push out the beta tomorrow.