-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 icons to Add Data list and tutorial view #16592
Conversation
💔 Build Failed |
❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ @nreese!! @formgeist @snide what's the expected look and feel for tutorials without icons? Also, from a design perspective I imagine the card views can get pretty "colorful". Do we want a monochromatic feel here at all so that all cards are equal? For example, my eyes automatically jump to Redis because it's red. |
@@ -13,7 +13,6 @@ export function netflowSpecProvider() { | |||
' indexes the events into Elasticsearch, and installs a suite of Kibana dashboards.' + | |||
' This module support Netflow Version 5 and 9.' + | |||
' [Learn more]({config.docs.logstash}/netflow-module.html).', | |||
//iconPath: '', TODO |
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.
Is Netflow not going to get an icon? Ever (EDIT: otherwise we should leave the TODO
here)? I noticed that it wasn't in the screenshot either. @alexfrancoeur?
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.
@ycombinator I'm open to any icon here. I've pinged about screenshots a few times now too, would love to see one in there. Think you could add one? We could use a Cisco icon for Netflow, I can't find any others. We have a few things like system metrics / logs that do not have an icon either.
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.
Yeah, IIRC I used the Cisco icon in the modules UI prototype. If we can, I think that's the closest one we can use here. But I'm not sure about the legality of using it (I'd asked about this in an earlier issue: https://github.com/elastic/dev/issues/837#issuecomment-339625666).
So I'd say Cisco if legal is good with it or the generic/default icon if not.
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.
As for screenshots, I don't know how to create Netflow data w/o an actual Cisco device or something. I'm sure there is a way to simulate it though. @jsvd any ideas?
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.
@ycombinator I've ran icons by legal previously, I think we're good here. Let's add the Cisco icon. I'll link to it in the issue
@alexfrancoeur @snide I think we should have a default icon for those that currently don't have one. Some sort of Add data icon; On the colourful logos, I'm not sure - obviously the Redis is pretty stark in contrast to many of the others, but I also feel the pages will suffer if the icons are all grey or a single default blue. And then they're really hard to tell apart - I probably wouldn't instantly recognize the Redis logo if it was blue or grey. |
Yeah, I'd keep the color just because I generally think it's wrong to touch other people's branding. I think it's fine that they pop. I'd leave it as it, rather than make an icon for the generic options, because it would be confused that product's logo. For example, we wouldn't want netflow to have a generic logo there, we should proper add it. |
Sounds good. If all looks good from everyone else, I'm good with merging as-is and open a new issue for Netflow screenshots / icons. |
It would be nice to get the Netflow icon as part of this PR as it has other icons as well. That said, if we're trying to make some deadline here then I'm good with a follow up PR for the Netflow icon. ++ on separate PR for Netflow screenshots. |
I think it may take some time to get the Netflow icon if we plan to use the Cisco logo. Seems that there is a bit of a process on the Cisco side (https://www.cisco.com/c/en/us/about/brand-center/logo-usage-guidelines.html). I don't mind going through this process and checking with legal again, just don't want to hold up this PR waiting for Netflow. Does that work Shaunak? |
Sure. My main motivation for holding up this PR was to use it as a forcing function for us to not forget about the Netflow module and let its icon slip through the cracks. I've created #16684 so we can stay on top of it. Lets move any further conversation (legal and such) about the Netflow/Cisco icon there. |
b7ced86
to
0e50e5c
Compare
💚 Build Succeeded |
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 - technically, I'll defer to the others on whether we should merge this without the netflow icon.
@alexfrancoeur @ycombinator Are you ok with this PR getting merged as-is, without the netflow icon? |
@nreese yes, I'm good to go. I think we need to do a bit of cleanup with Netflow anyway (like including screenshot). @ycombinator created #16684 to track. It seems like it's a complicated process to use the Cisco icon. That may block this effort and I don't want that to block this PR. |
* add icons for home tutorials * add test for Synopsis component
Yep, I'm good too. |
fixes #16279
cc @alexfrancoeur