You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

You can test the changes on this Pull Request by downloading the APK here.

👋 @ashiagr !

We don’t usually add _gridicons to icon names, it might be nice to remove it from these icons for consistency.

Thank you for this comment, I was actually wanted to discuss on that! 🙏

I left the gridicons word on purpose thinking that this is what we want, just to differentiate our gridicons from the rest of them. I got to that conclusion since I saw other icons like that, which was containing the gridicons wording within them. From your comment, it seems that we are not interested in that.

Suggestion (💡): After update the icons in this PR to remove the gridicons word, should I go forward and do the same with all the other icons that contain the gridicons word in them, just to avoid any future suchlike discussion?

Cc: @malinajirka to share his thoughts as well (since he might be more aware of the history).

I left the gridicons word on purpose thinking that this is what we want, just to differentiate our gridicons from the rest of them. I got to that conclusion since I saw other icons like that, which was containing the gridicons wording within them. From your comment, it seems that we are not interested in that.

I see that out of several icons, there's one icon ic_gridicons_checkmark_circle in the main project and 3-4 icons in the separate library modules with _gridicons in the name. I think we're using gridicons for most of our icons and usually do not append _gridicons to the name (e.g. ic_cloud_white_24dp, ic_cog_white_24dp). Either we append _gridicons to all existing such icons or we remove from where they're used, we can go either ways! Or may be not worry about it 😄.

Cc: @malinajirka to share his thoughts as well (since he might be more aware of the history).

Agree, let's wait for inputs from @malinajirka :).

As @ashiagr mentioned, we usually do not include "gridicon" in the name of the icon. I think removing it from all our resources would be good (not necessary). Having said that, watch out that icons in the LoginLib might be used in Woo (I think there is just one).

I see that out of several icons, there's one icon ic_gridicons_checkmark_circle in the main project and 3-4 icons in the separate library modules with _gridicons in the name. I think we're using gridicons for most of our icons and usually do not append _gridicons to the name (e.g. ic_cloud_white_24dp, ic_cog_white_24dp). Either we append _gridicons to all existing such icons or we remove from where they're used, we can go either ways!

Yeap, not sure about it either, since I was seeing the gridicons word being used, I initially thought that we want that there in order to easily much the file name with that on the gridicons repo. Thus, I didn't want to delete this information. However, I am really okay with anything we decide, and wanted to raise the question here so I know what to do in the future without further thinking about it.

Or may be not worry about it 😄.

😄 That is true! Either way, it is a small change and we will end up merging to develop today, so I am not stressed about it too much. Also, since we are waiting for @osullivanchris to do the design review on them, we have some additional time to decide on that. 😄

it is a small change and we will end up merging to develop today,

Right, let's merge it 🚀 🚀. We can discuss it later 👍.

👋 @osullivanchris !

This PR was merged to develop without getting your approval first, this is my fault as I should have added specific merge instructions to not merge it before it gets a design approval.

Would you mind do a design review anyway and let us know if you find anything that need correction and I'll make sure to create another PR with those right away, then link that PR to this one.

Many thanks and apologies for any confusion! 🙏

This PR was merged to develop without getting your approval first, this is my fault as I should have added specific merge instructions to not merge it before it gets a design approval.

😄 no problem. The new icons look good.

One thing that I should have thought of sooner too, I think it should be "Jetpack Settings" and "Activity Log" with title case. As when I look down the rest of the menu that's what we are using (Blog Posts, Site Pages).

😄 no problem. The new icons look good.

🙏

One thing that I should have thought of sooner too, I think it should be "Jetpack Settings" and "Activity Log" with title case. As when I look down the rest of the menu that's what we are using (Blog Posts, Site Pages).

Thanks for the review @osullivanchris !

Yes, this makes sense. I'll prepare another PR by the end of the day with that exact change and let you know. 💪