Skip to content

[EuiHeader] Fix SASS globals & docs#3592

Merged
cchaos merged 4 commits intoelastic:masterfrom
cchaos:fix/header_sass
Jun 11, 2020
Merged

[EuiHeader] Fix SASS globals & docs#3592
cchaos merged 4 commits intoelastic:masterfrom
cchaos:fix/header_sass

Conversation

@cchaos
Copy link
Copy Markdown
Contributor

@cchaos cchaos commented Jun 10, 2020

Fixes #3589

It was pointed out that the mixin provided for fixed headers wasn't properly importing the necessary variables so consumers got hit with a missing variable error when trying to use it. This was because the header variable that was being used in the mixin wasn't in the global_styles but in the component styles.

I've moved all the header variables to global_variables since these seem to be affecting more than itself anyway.

I have tested that this fixes the missing variable issue by linking locally in the K8 POC. Errors are gone 👍 !

This also fixes the docs that accidentally said to use @mixin ... instead of @include ....

Checklist

  • Check against all themes for compatibility in both light and dark modes
  • [ ] Checked in mobile
  • [ ] Checked in IE11 and Firefox
  • [ ] Props have proper autodocs
  • Updated documentation
  • [ ] Checked Code Sandbox works for the any docs examples
  • [ ] Added or updated jest tests
  • [ ] Checked for breaking changes and labeled appropriately
  • [ ] Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

@cchaos cchaos requested a review from thompsongl June 10, 2020 20:04
@kibanamachine
Copy link
Copy Markdown

Preview documentation changes for this PR: https://eui.elastic.co/pr_3592/

Copy link
Copy Markdown
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 works. I can't wait till we have emotion and can potentially apply a global style when an individual component is used. Would come in really handy here.

I think this quirk of the header is important enough I'd include a note in https://github.com/elastic/eui/blob/master/wiki/consuming.md

Copy link
Copy Markdown
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

LGTM!

@kibanamachine
Copy link
Copy Markdown

Preview documentation changes for this PR: https://eui.elastic.co/pr_3592/

@cchaos cchaos merged commit 8bb3671 into elastic:master Jun 11, 2020
@cchaos cchaos deleted the fix/header_sass branch June 11, 2020 15:18
phyolim pushed a commit to phyolim/eui that referenced this pull request Jun 11, 2020
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.

[EuiHeader] Some SASS issues from latest PR

4 participants