Skip to content
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

Elastic logos support dark mode #1562

Merged
merged 6 commits into from
Feb 14, 2019
Merged

Elastic logos support dark mode #1562

merged 6 commits into from
Feb 14, 2019

Conversation

snide
Copy link
Contributor

@snide snide commented Feb 14, 2019

Summary

Now applies the coloring through a class, rather than the hard coded value. They look good in most cases, but sort of call out some minor inconsistencies in these. My guess is we might have some old versions. I'll check with @JessSmithSup before merging.

image

image

image

image

Checklist

  • This was checked in mobile
  • This was checked in IE11
  • This was checked in dark mode
  • Any props added have proper autodocs
  • Documentation examples were added
  • A changelog entry exists and is marked appropriately
  • This was checked for breaking changes and labeled appropriately
  • Jest tests were updated or added to match the most common scenarios
  • This was checked against keyboard-only and screenreader scenarios
  • This required updates to Framer X components

@JessSmithSup
Copy link

Looks like that is the old Logastash logo, and since X-Pack is going away we never touched it for a redesign.

@snide
Copy link
Contributor Author

snide commented Feb 14, 2019

@JessSmithSup Figured 👍 If you have a chance can you link me to the latest LS one and I'll replace.

@JessSmithSup
Copy link

@snide
New Logstash logo is here
lmk if it looks off.

Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM.

These don't get added extremely frequently, but often enough that perhaps making a note about adding/using the class="euiIcon__fillNegative" class would be worthwhile for future contributors.

A possible home for that note could be either on the wiki, code comment, or added to the EUI docs description for the Elastic logos section?

@@ -80,6 +80,10 @@
fill: $euiColorGhost;
}

.euiIcon__fillNegative {
fill: $euiColorDarkestShade;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So not fully white in dark mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Just like it's not fully black in light mode. I was suprised to see the light mode was actually just darkest shade.

@cchaos
Copy link
Contributor

cchaos commented Feb 14, 2019

Should we just remove the X-pack logo from the docs?

@snide
Copy link
Contributor Author

snide commented Feb 14, 2019

A possible home for that note could be either on the wiki, code comment, or added to the EUI docs description for the Elastic logos section?

@ryankeairns Added it in both the code and the docs.

@ryankeairns
Copy link
Contributor

Awesome, thanks!

@snide snide merged commit 98e5cbf into elastic:master Feb 14, 2019
@snide snide deleted the dark/logos branch February 14, 2019 18:53
@snide
Copy link
Contributor Author

snide commented Feb 14, 2019

Should we just remove the X-pack logo from the docs?

Probably, my guess is it's floating around there a bit though. I'll tackle that separately.

@cchaos
Copy link
Contributor

cchaos commented Feb 14, 2019

Yeah I just meant from the docs themselves

Shigawire pushed a commit to Shigawire/eui that referenced this pull request May 10, 2019
Logos now work better in dark mode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants