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

EUI 5.8.1 comes with a brand color refresh. Also removes lots of KUI #27009

Merged
merged 26 commits into from
Dec 18, 2018

Conversation

snide
Copy link
Contributor

@snide snide commented Dec 12, 2018

Summary

For team review pings:

  • I changed the APM JS variables that mirrored EUI. It'd be nice if you all imported this from the JSON we dist, but it was simple enough.
  • Beats team - I changed some hardcoded hex values you had in styled components. Also prefer if you pulled this in from EUI's vars.
  • Security - Small template / sass changes to match colors.

Color refresh

EUI 5.7.0 had a color refresh which switched around our palette to better match branding guidelines. Hex colors are still hard coded in large parts of Kibana so most of the changes not in kbn/ui-framework are simple shifts to match that styling.

Styling of KUI

KUI now imports the EUI global scope. I went through and touched a lot of the global variables in KUI to better match EUI. Outside of the coloring, I did some very minor work around KUI form fields and the verticalRhythm component.

Removal of many KUI components

KUI remains deprecated since 6.2.0 but still exists. I went ahead and removed any components and styling that were no longer being used in Kibana and then removed several KUI components that were being minimally used by replacing them with EUI styling, using EUI components or selectors instead. This resulted in KUI being a little under half its original size and is the reason this PR has such a large negative number.

Global nav cleanup

With the color change the global nav icons in the K6 nav was really off. I went ahead reworked them to use EUI icons and cleanup them up stylistically. The static images are still available if no EUI icon is used so this is backwards compatible and should be fine for custom plugins outside of core Kibana.

Tests

The color changes required me to rebuild the baselines for the TSVB screenshot. We have a lot of hard coded hex values that in turn are converted back and forth between rgb and hex. In one case I chose not to not change an advanced setting default to match our new colors because the tests went a little nuts. It's the green color and is mostly close to what we're using in both palettes. I'd like to do a later sweep there and make these tests less reliant on copy / paste codes, but given FF timing, I'm just avoiding that value for now.

Updates EUI to 5.8.1 from 5.6.2

5.8.1

Note: this release is a backport containing fixes made in 6.0.0

Bug fixes

  • Fixed some EUI services' TS definitions (#1380)

5.8.0

Note: this release broke some of the exported TypeScript definitions.

  • Reinstate (#1353) onBlur action on EuiComboBox (#1364)
  • Convert roughly half of the services to TypeScript (#1360)

Bug fixes

  • Fixed onCreateOption callback of EuiComboBox so it isn't called when the input is empty (#1364)
  • Added anchorClassName prop to EuiPopover (#1367)
  • Added support for fullWidth on EuiSuperSelect (#1367)
  • Applied new scrollbar customization for Firefox (#1367)
  • Fixed EuiSuperSelect from accessing ref when unmounted (1369)
  • Allow any color value to be passed to EuiIcon (#1370)

5.7.0

  • Adjust EUI coloring to better match brand guidelines from Creative Services (#1356)

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@snide snide added the Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. label Dec 12, 2018
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-design

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@snide
Copy link
Contributor Author

snide commented Dec 12, 2018

jenkins, test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

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.

Most of it looks good, some better than others, but these are just a few that I find needs a quick fix:

  1. Sidebar

screen shot 2018-12-12 at 14 40 47 pm

  1. Visualize top bar

  1. For some reason the disabled buttons in Kibana look different from EUI

screen shot 2018-12-12 at 15 13 09 pm

  1. I found a page where the background colors don't line up

screen shot 2018-12-12 at 15 15 39 pm

$kuiTextColor: #2d2d2d;
$kuiLinkColor: $kuiColorBlue;
$kuiTextColor: $euiTextColor;
$kuiLinkColor: $euiColorPrimary;
$kuiLinkColor-isHover: darken($kuiLinkColor, 10%);
$kuiInputTextColor: $kuiTextColor;
$kuiInputBackgroundColor: $kuiColorWhite;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is going to work very well if it ever exists in dark theme as $kuiColorWhite is ghost

Copy link
Contributor Author

Choose a reason for hiding this comment

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

KUI has it's own variables for 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.

That said, I need to check more into the dark theme coloring. I see some breaks this introduced.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@snide
Copy link
Contributor Author

snide commented Dec 13, 2018

This is closer. I need to figure out the test failures happening in dashboards and do some IE11 / dark mode testing.

I think it's questionable whether we should do this for 6.6. It's a big PR that makes some aggressive changes and rips out tons of unused code. I was able to style the K6 global nav to use our icons, which looks cool, but will likely surprise a lot of people.

@snide snide requested review from a team as code owners December 13, 2018 19:04
@snide snide changed the title [WIP] EUI 5.8.0 and clean out KUI EUI 5.8.1 and clean out KUI Dec 18, 2018
@snide snide changed the title EUI 5.8.1 and clean out KUI EUI 5.8.1 with brand color refresh and clean out KUI Dec 18, 2018
@snide snide changed the title EUI 5.8.1 with brand color refresh and clean out KUI EUI 5.8.1 with brand color refresh. Also removes lots of KUI Dec 18, 2018
@elasticmachine
Copy link
Contributor

💔 Build Failed

@snide
Copy link
Contributor Author

snide commented Dec 18, 2018

image

Ugh. I updated the baseline, but then changed the color back. I've now updated the baseline again now that it works. I think this should be the last one. Got the rest of the tests to pass now.

@snide snide changed the title EUI 5.8.1 with brand color refresh. Also removes lots of KUI EUI 5.8.1 comes with a brand color refresh. Also removes lots of KUI Dec 18, 2018
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@snide
Copy link
Contributor Author

snide commented Dec 18, 2018

@cchaos This now passes and is merged up. If you can during your morning, feel free to do a more thorough check. You're welcome to commit directly or just give a PR if you notice anything sizable.

@cchaos
Copy link
Contributor

cchaos commented Dec 18, 2018

I think we should be good to go. I thought about doing some more tweaks in the theming LESS styles, but I've already completely gotten rid of those in my next SASS conversion PR. So we can just wait for those final color conversions in that PR. But I'd really like to get this one in first.

@snide
Copy link
Contributor Author

snide commented Dec 18, 2018

Merged back up to master. Once tests pass I'll be merging this one.

@snide snide mentioned this pull request Dec 18, 2018
3 tasks
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@snide snide merged commit a94fd11 into elastic:master Dec 18, 2018
@snide snide deleted the kui/vars branch December 18, 2018 18:59
snide added a commit to snide/kibana that referenced this pull request Dec 18, 2018
…lastic#27009)

EUI 5.7.0 had a color refresh which switched around our palette to better match branding guidelines. Hex colors are still hard coded in large parts of Kibana so most of the changes not in kbn/ui-framework are simple shifts to match that styling.
snide added a commit that referenced this pull request Dec 18, 2018
…f KUI (#27009) (#27429)

* EUI 5.8.1 comes with a brand color refresh. Also removes lots of KUI (#27009)

EUI 5.7.0 had a color refresh which switched around our palette to better match branding guidelines. Hex colors are still hard coded in large parts of Kibana so most of the changes not in kbn/ui-framework are simple shifts to match that styling.

* snapshots
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:enhancement Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants