Third party icons #1040
Third party icons #1040
Conversation
There was a problem hiding this comment.
Missing "munin" and "postgressql"? I see them in the folder of logos but not in this list.
They look great. Even most of them still work well in dark mode. The only two I see that could use some help are the Slack and osquery logos. The dark gray parts look so close to black that I'll bet we can just remove the fill on those.
| 'logoGithub', | ||
| 'logoGmail', | ||
| 'logoKubernetes', | ||
| 'logoLogstash', |
There was a problem hiding this comment.
Forgot to remove Logstash icon from 3rd party grouping
| 'logoCouncbase', | ||
| 'logoDropwizard', | ||
| 'logoEtcd', | ||
| 'logoHaproxy', |
|
Also, you might want to add them to the TS definitions file. |
|
Thanks, feedback addressed. |
|
I still don't see the "munin" and "postgressql" logos. Also should we be updating for dark theme or leave that for later? |
|
Sorry, somehow missed your actual review comment in the emails. The missing files were because i got bitmaps / messy files. As to dark theme, I don't think there's a good way to support that other than converting / collecting them all to flat icon marks so they can be inverted. Even ours don't work too hot on dark right now. Not saying we shouldn't do it, but likely needs some more thought. Alex has a new batch coming, ill add those in over the weekend and see if i can hunt some better files down. |
|
Yeah I don't think we can/need to tweak each one for dark theme. I just noticed that they pretty much all looked alright in dark mode except we could just leave out the fill from the slack and osquery black parts and it would look ok in dark mode. |
|
Good idea @cchaos. Adjusted them where I could. The Osquery one was a little tricky cuz it won't accept the empty fill, so I just tweaked it slightly so it would work. TS updated, new icon for index management added as well cc @bmcconaghy
|
| <svg width="32px" height="32px" viewBox="0 0 32 32" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> | ||
| <!-- Generator: Sketch 51.1 (57501) - http://www.bohemiancoding.com/sketch --> | ||
| <title>Artboard</title> | ||
| <desc>Created with Sketch.</desc> |
There was a problem hiding this comment.
Still got a bunch of cruft in this one.
|
We didn't get to integrate the icons before 6.4 FF. I was kind of worried to open a non bugfix PR after 6.4 FF. Two questions:
|
|
If the Kibana team is comfortable with this change (just adding icon lines in the tutorials), we might also make it in 6.4.0 with a bit of luck. |
|
No it doesn't. 6.4 is on 3.0.3, icons are on 3.2.1 (kb master is on this) I don't think we should risk an EUI update this late in the game during a release, even if it is just icons, and even if we were to push a 3.0.4 with just this change. This is an additive visual that doesn't impact functionality and I want to stick to normal Kibana policy to keep EUI updates during FF to bug releases only. We're trying really hard to lock down EUI in advance of Kibana releases so there's a level of stability for the developers (which is something we get feedback on) and they aren't cleaning up after us while they have their own last minute changes going in. I think the lesson learned here is that we should aim to get these types of requests in a little bit earlier before FF if we can manage it. Granted, I likely could have hopped a little faster on this PR, but that doesn't account for review, amendments and the actual packaging of EUI which comes with its own day or two lag (in this case, everything was slow when Kibana was in flakey test hell). |
|
@snide makes total sense, agree that we should have entered the request a bit earlier. Definitely not worth pushing to 6.4 if it's a new version of EUI. Appreciate the amount of detail here, next time we'll be sure to put in requests a earlier for any icons 👍 |
Fixes #1033
I went ahead and split the logos section into Elastic and Third party sections