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

[EuiHeader] Fix SASS globals & docs #3592

Merged
merged 4 commits into from
Jun 11, 2020
Merged

Conversation

cchaos
Copy link
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

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

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

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