Skip to content

Conversation

@sorenlouv
Copy link
Member

@sorenlouv sorenlouv commented Feb 25, 2019

Closes #1588

  • Inline sass-extract-js plugin
  • Remove camelCasing logic
  • Preserve hex values where possible

Diff of the light theme with the new variables: https://www.diffchecker.com/ocT4Z4lA

Remove camelcasing
Preserve hex values over rgb where possible
Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

Nice, looks like it's almost there. I left just two small comments below.

Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

LGTM

@sorenlouv
Copy link
Member Author

@chandlerprall WDYT?

@sorenlouv
Copy link
Member Author

sorenlouv commented Feb 25, 2019

This contains breaking changes due to the removal of camelCasing. In particular the following properties will have to be replaced:

  • euiSizeXs -> euiSizeXS
  • euiSizeXl -> euiSizeXL
  • euiSizeXxl -> euiSizeXXL
  • euiFontSizeXs -> euiFontSizeXS
  • euiFontSizeXl -> euiFontSizeXL
  • euiFontSizeXxl -> euiFontSizeXXL

@snide snide added the breaking change PRs with breaking changes. (Don't delete - used for automation) label Feb 26, 2019
@snide
Copy link
Contributor

snide commented Feb 26, 2019

cc @zumwalt, @daveyholler and @peteharverson. We'll clean this up once it goes into Kibana, but we're going to do a breaking change to replace thedist/eui_theme_*.json files so the variables match against the Sass layer (and documentation). Check @sqren's note above. This will only effect you if you're importing the EUI variables as JSON.

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

This will need a CL. Please mark it as a breaking change. Thanks for doing this. One less regret from the past destroyed from my conscious :)

Here is a diff (stored for one month) in case anyone needs a reference of what this changes in the dist. I did a build and looked through the diff to make sure it matched what we'd expect.

https://www.diffchecker.com/ArT67fzc

@snide
Copy link
Contributor

snide commented Feb 26, 2019

Also cc @timroes @markov00 since they might be importing these into visualizations and will need to be aware of the change.

@timroes
Copy link
Contributor

timroes commented Feb 26, 2019

It seems this is used in two files x-pack/plugins/beats_management/public/components/autocomplete_field/suggestion_item.tsx and x-pack/plugins/infra/public/components/autocomplete_field/suggestion_item.tsxso whoever updates Kibana to that EUI version should remember to update those two files to the new format.

@sorenlouv sorenlouv merged commit 6dc7ea1 into elastic:master Feb 26, 2019
@sorenlouv sorenlouv deleted the inline-sass-extract-js-plugin branch February 26, 2019 08:47
@sorenlouv
Copy link
Member Author

Oh, was a bit too quick and missed the changelog item. Adding it via this PR. Sorry. #1594

weltenwort added a commit to elastic/kibana that referenced this pull request Mar 18, 2019
This fixes the `small` font scale of the log viewer to correctly reference the EUI font size again. Before this fix, the `small` font scale was rendered larger than `medium`. Similarly, the tick labels of the log minimap were being rendered too large.

Both resulted from a breaking change of the exported variable names in elastic/eui#1590.

fixes #32759
weltenwort added a commit to weltenwort/kibana that referenced this pull request Mar 18, 2019
…#33411)

This fixes the `small` font scale of the log viewer to correctly reference the EUI font size again. Before this fix, the `small` font scale was rendered larger than `medium`. Similarly, the tick labels of the log minimap were being rendered too large.

Both resulted from a breaking change of the exported variable names in elastic/eui#1590.

fixes elastic#32759
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change PRs with breaking changes. (Don't delete - used for automation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants