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.

Sign up for GitHub

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

Jump to bottom

Inline sass-extract-js plugin #1590

Merged
merged 3 commits into from
Feb 26, 2019
Merged

Inline sass-extract-js plugin #1590

merged 3 commits into from
Feb 26, 2019

Conversation

sorenlouv
Copy link
Member

@sorenlouv sorenlouv commented Feb 25, 2019 •
edited
Loading

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

@sorenlouv
Inline sass-extract-js plugin
c5da256 
Remove camelcasing
Preserve hex values over rgb where possible
weltenwort
weltenwort suggested changes Feb 25, 2019
View reviewed changes
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.

scripts/sass-extract-js-plugin.js Show resolved Hide resolved
package.json Show resolved Hide resolved
weltenwort
weltenwort approved these changes Feb 25, 2019
View reviewed changes
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 •
edited
Loading

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
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.

snide
snide approved these changes Feb 26, 2019
View reviewed changes
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 added a commit that referenced this pull request Feb 26, 2019
@sorenlouv
Mark "Inline sass-extract-js plugin" as breaking change
4439980 
#1590
@sorenlouv sorenlouv mentioned this pull request Feb 26, 2019
Merged
@sorenlouv
Copy link
Member Author

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

@maryia-lapata maryia-lapata mentioned this pull request Mar 1, 2019
Closed
7 tasks
@thompsongl thompsongl mentioned this pull request Mar 12, 2019
Merged
@weltenwort weltenwort mentioned this pull request Mar 18, 2019
Merged
1 task
weltenwort added a commit to elastic/kibana that referenced this pull request Mar 18, 2019
@weltenwort
[Logs UI] Fix font size references after EUI upstream change (#33411)
04f816b 
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
@weltenwort
[Logs UI] Fix font size references after EUI upstream change (elastic…
4c29cec 
…#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
@weltenwort weltenwort mentioned this pull request Mar 20, 2019
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weltenwort weltenwort weltenwort approved these changes

@snide snide snide approved these changes

@chandlerprall chandlerprall Awaiting requested review from chandlerprall

Assignees
No one assigned
Labels
breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sorenlouv @snide @timroes @weltenwort