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

New product logos #1279

Merged
merged 2 commits into from
Nov 1, 2018
Merged

New product logos #1279

merged 2 commits into from
Nov 1, 2018

Conversation

snide
Copy link
Contributor

@snide snide commented Nov 1, 2018

Summary

Summarize your PR. If it includes design elements include a screenshot or gif.
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

@snide
Copy link
Contributor Author

snide commented Nov 1, 2018

cc @JessSmithSup @Michalwo I noticed some shape clippings because of the stack of these shapes on top of each other. It's only noticable on Dark, which likely would need touchups to work anyway. In 6.5 you don't need to worry about dark mode yet.

@JessSmithSup
Copy link

@snide I have a version that has no overlapping, I can export if we need it. Do you think that will solve it?

@snide
Copy link
Contributor Author

snide commented Nov 1, 2018

@JessSmithSup Honestly I don't think it's something we need to worry about immediately (it's invisible on white, which is the only place we'd see them in 6.5).

If you feel otherwise though feel free to update the Sketch EUI Libraries/logos file with your newer versions and I can move them in.

I think we do need a strategy for what do do with these in dark mode in the 7.0 world though. I could prolly adjust the colors manually per theme if we wanted.

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

LGTM. Yeah these don't work so well with swapping the dark gray #343741 with our text color (in dark theme), so it'll probably need to be a manual decision.

@@ -1,5 +1,7 @@
## [`master`](https://github.com/elastic/eui/tree/master)

- Added updated and new product logos to `EuiIcon` ([#1279](https://github.com/elastic/eui/pull/1279))
Copy link
Contributor

Choose a reason for hiding this comment

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

Added & updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will update is subsequent PR

@snide snide merged commit b510093 into elastic:master Nov 1, 2018
@snide snide deleted the logos/updates branch November 1, 2018 22:55
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.

3 participants