-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Icon] add LocationIssue and LocationValid icons #11940
Conversation
e28d75b
to
37b6a87
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, might want to wait for a polaris engineer to approve!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I've added a commit that cleans up the icon paths using the optimize
script.
Also a few consistency suggestions to closer match the conventions in other icon YAML files.
cc. @sam-b-rose Co-authored-by: Sam Rose <[email protected]>
@sam-b-rose thanks for the suggestions and optimizations! I've incorporated your feedback on the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JordanForeman what is the timeline/urgency for these icons? We are building a new icon library to replace this one.
I just realised that the location icons are updated since we first created the icons. So, I just updated the icons and respective issues with to reflect the latest icon designs. |
@alex-page I'd say "medium" urgency? We're aiming for mid-May to start using this. I'm not sure we'll have time to include migration to a new icon library, depending on the costs associated with that. |
@JordanForeman is there any blockers or things stopping you from using the icons without polaris-icons until the new library is implemented? |
@alex-page nope 😄 I can just hardcode these for now, and we can keep tabs on how icons evolves. |
WHY are these changes introduced?
resolves #11939
resolves #11938
WHAT is this pull request doing?
Adds a new icons to the
polaris-icons
package.cc. @ksvihar
How to 🎩
🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines
🎩 checklist
README.md
with documentation changes