Closed SylvesterWilmott closed 6 years ago
Looks like a good idea to me. The icon seems clear too.
Code wise the SVG is clean, but it includes the blank rectangle <rect x="0" fill="none" width="24" height="24"/>
which should be removed.
Ok I checked again against the guideline and it's good.
Can you ping me once you push the build, right before merging? We had some issues in the past so I'd like to use this chance to cross-check.
Thanks!
Ok the build is good, just 8 files changed (only the ones affected from the change).
All good, thanks!
Great work, @SylvesterWilmott !!
Looks awesome, @Sylverster! :)
Not sure where this discussion should happen, but this solution is not thinking very holistically. We can't continue to solve for just one platform. Am I correct in thinking that WordPress.com web will now have a completely different icon than the native mobile apps for 'sticky post'?
Yep that needs to be coordinated, you're right. I'd probably considered updating Calypso with this icon, but I acknowledge we need to make these things explicit.
Feedback on the icon itself
I don't know if we state this anywhere, but Gridicons has a preference for vertical icons, not angled icons. If it make sense, we can make an exception. For example, the pencil icon is angled because it mimics the natural angle of the pencil while writing.
This is probably a side effect of the icon being angled (and maybe some pixel alignment), but the icon is not symmetrical and many of the points/curves don't line up.
Icon flipped horizontally:
Maybe something like this
Am I correct in thinking that WordPress.com web will now have a completely different icon than the native mobile apps for 'sticky post'?
No. As far as I am aware it won't be used on mobile unless it is adopted on web as well. But to be clear, the pin icon has another use in an upcoming project on mobile for stats. But anyway, yes there should be a discussion about changing the icon.
I don't know if we state this anywhere, but Gridicons has a preference for vertical icons, not angled icons.
I wasn't aware of this, no. If there is a consensus on this I can change it to a vertical icon.
the icon is not symmetrical and many of the points/curves don't line up.
Seems like an artefact of angling the icon and then expanding. I'll have to clean that up.
No. As far as I am aware it won't be used on mobile unless it is adopted on web as well. But to be clear, the pin icon has another use in an upcoming project on mobile for stats. But anyway, yes there should be a discussion about changing the icon.
@SylvesterWilmott To clarify, I mean WordPress.com web and mobile native will use 2 different icons for the same action, "sticky post". WordPress.com web continues to use the bookmark icon for 'sticky post', and mobile native will use a pin icon for 'sticky post'. Does that make sense?
@drw158 AFAIK The label will be used on mobile without an icon until we can align between web and mobile. This PR was just to add the icon to the gridicons library.
@nozomimimi Am I right in assuming that's the plan?
Introducing pin icon as candidate for sticky posts after conflict between 'Save for Later' and 'Sticky Posts' icons on mobile.