-
Notifications
You must be signed in to change notification settings - Fork 841
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
Add new icons #2102
Add new icons #2102
Conversation
@claracruz we have existing/similar icons for a couple of these (document and edit) and I'm not certain the others ought to be added to the EUI icon set since they don't strike me as generally re-usable (is this for Kibana?). My recommendation is that these icons be stored elsewhere and referenced via a path or URL as described in the 'Custom SVGs' section at the bottom of this docs page: https://elastic.github.io/eui/#/display/icons |
@ryankeairns It's for the new cloud portal we're building. I assume it may become relevant eventually. There are slight difference in the |
There are downsides with providing too many icon options. Mostly you'll see people use different icons to mean the same thing and you can lose consistency and recognition for the user. However, I think the documents isn't duplicative (since it's representing multiple) and we can probably just rename this edit one to documentEdit so it's more specific. The main thing that needs to happen here, though, is that we'll need a designer to pull these svg's into our Sketch library to: A. Make them consistent with the rest of the glyph set. Example: B. Ensure there are no strokes or direct hex values inside the export. This file has both which will cause our theming to not work with these icons. |
Currently looking into this... |
@claracruz The design team is going to discuss our general approach to managing the growing EUI icon library, tomorrow, at our regular design meeting. We'll provide you clearer feedback afterwards, thanks for your patience as we sort out this process. |
ok, no worries |
Summary from the meeting Follow @cchaos' review above
I'll add a last one
|
@cristina-eleonora Are we able to do this pls? |
update changelog Per PR review - Update new icons to icon design specs
Updated PR with new icon designs provided by @cristina-eleonora |
I'm in the process of reviewing the SVGs and adding them to our Sketch library. Will report back soon! |
@cristina-eleonora There is some potential duplication within the new icons and we're wondering if the existing document and copy icons can be used (A). If not, then we could add both your new document icons (B), but it may lead to a mixed use of the similar document icons. A third alternative would be to add lines to our existing document icon and add your new documents icon (C). |
I second that emotion 👍 |
I've also touched up the other icons in this PR as well to fit more closely to the EUI style and with non-retina simplifications. Once we settle on the document icon set, I'll either update this PR or open a new one (for simplicity's sake) as I'd also like to change some of the names, etc. |
Thank you for your work on this, @ryankeairns
I third that idea 🙂 I think it can work great with what we need. |
👍 |
@claracruz @cristina-eleonora I've opened a PR against this one. Presuming everything looks good, you can simply merge the PR: claracruz#1 |
Retina tweaks, re-names, eui-ify
LGTM 🙂 |
Re-shape cloudStormy
Summary
This PR adds new icons provided @ here
Checklist