Add EQL glyph to EuiIcon #4110
Add EQL glyph to EuiIcon #4110
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4110/ |
|
Nice @marrasherrier ! Can you paste a screenshot of the design file with the pixel grid behind the icon? The strokes don't look like they're fully 1px thick at the 16x16 size compared to the other glyphs. |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4110/ |
There was a problem hiding this comment.
Thanks, @marrasherrier!
I'll leave the design review for @cchaos. But I noticed a few things that can be improved in your code.
|
I have one suggestion on the design of the icon. It wasn't until you pasted the screenshot of the artboard version that I noticed the detail of using your hexagon shape in the center. This is a detail that gets lost even at the 24x24 version but is essential to the understanding that this is Security specific. Have you tried an option where the hexagon is the outermost shape instead of a simple circle? We also have quite a few magnify glass icons not just in glyphs but app icons as well, so I think having the hexagon be the dominant shape here will really set it apart from all the other magnify options.
|
|
Hi Caroline, thanks for this feedback!! But, I think there is a way I can differentiate it from the other icons, make the hexagon stand out more, and still reference this existing icon. Will update soon. |
|
@cchaos @peluja1012 thoughts on the new design? |
|
@marrasherrier I like the prominent hexagon shape and the similarity with the existing icon. 👍 |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4110/ |
There was a problem hiding this comment.
The simplification definitely helps to make the "node" portion stand out better 👍
My only comment would be to check your alignment to the pixel grid. It's not bad, but could be crisper on non-retina screens. Here's a screenshot:
But I'll approve anyway and let you decide how you want to handle that bit.
@cchaos thanks! I updated it to be better aligned. :) |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4110/ |
Summary
This PR adds an EQL gylph to EuiIcon. The EQL icon appears in the 'Event Correlation' card, which is part of the 'Select a rule type' step of rule creation in the security app.
EQL icon in dark and light theme
Design
Based off of:
EQL icon near other icons
Checklist
[x] Check against all themes for compatibility in both light and dark modes
[ ] Checked in mobile[x] Checked in Safari and Firefox
[ ] Props have proper autodocs[x] Added documentation
[ ] Checked Code Sandbox works for the any docs examplesAdded or updated jest tests
[x] Checked for breaking changes and labeled appropriately
[x] Checked for accessibility including keyboard-only and screenreader modes
[x] A changelog entry exists and is marked appropriately