Move shared component mixins to global_styles

[johnbarrierwilson]

Summary

In order to create themes more easily, this PR will move component mixins that are used in more than one component—for example, euiPanel is used in the panel, card and droppable components—to the src/global_styling/mixins directory. Practically, this sets the foundation for doing more extensive work for EUI Amsterdam.

Mixins

Mixins that need moving to src/global_styling:

• components/button/_mixins.scss
• components/form/_mixins.scss
  • components/form/form_control_layout/_mixins.scss
  • components/form/switch/_mixins.scss
• components/loading/_mixins.scss
• components/panel/_mixins.scss
• components/popover/_mixins.scss

Checklist

• Checked in dark mode
• Checked in mobile
• Checked in IE11 and Firefox
  - [ ] Props have proper autodocs
  - [ ] Added documentation 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

[johnbarrierwilson]

A quick note: once I got into the form mixin stuff, it adversely required the variables and mixins of many more form-based components. This included: form, form_control_layout, range, and switch mixins and variables.

[johnbarrierwilson]

@cchaos @ryankeairns (and others) — As previously discussed, I've finished up pulling out any shared mixins from the components and put them into the /global_styles directory. I had a few thoughts at this point, so instead of doing it through comments, here's a quick screencast:

Screencast: https://www.loom.com/share/8095521ed8e940fea659ae35442aa5ed
Whimsical: https://whimsical.com/NgtG9QtjSxn7qYP2b3yKEw

Questions

1. Is it worth going down this route?
2. If so, should we do it in this PR or merge and start a new one?
3. If not, just merge and continue on as-is?

[ryankeairns]

@johnbarrierwilson I appreciate you digging into and grokking the world view of the /src directory, this map is helpful, thank you! 😄

My initial reaction, regardless of whether we tackle this re-organization or not, is that it feels wise to merge this PR with its current smaller scope - keeping it small and digestible while allowing you to keep momentum on the visual design effort.

Personally, I like the convenience of having the SCSS files right there with the component from a development perspective as it's very easy to find files/styles.

This proposed change also has me wondering about potential impacts this might have to our current build/release process (maybe it's minimal?) and what (if any) affect this could have on our future plans with regards to modularization/use of astroturf, etc. In other words, would moving these files have any impact down the road? Tagging in Chandler and Greg for more perspective on those fronts.

cc:/ @thompsongl @chandlerprall

[johnbarrierwilson]

@ryankeairns Glad the map is helpful! 😊

...it feels wise to merge this PR with its current smaller scope...

I totally agree and will do some more testing to make sure everything is stable, then I'll open it for review.

...has me wondering about potential impacts...

I think those are also valid concerns. The largest issue I was attempting to address here is the ease of theme-ability by trying to avoid this as illustrated in my previous PR:

From personal experience, I've used a /styles directory for many larger projects at this size, and I found it much easier to balance the scale using this "separate concerns" philosophy. That said, I'm still not as familiar with EUI as everyone else here, so I'll make sure to head any advice/concerns this may raise.

[cchaos]

@johnbarrierwilson I agree with @ryankeairns about having the component SASS files next to the component JS files. We did this very intentionally so that we knew exactly where to look for styles and we've matched our SASS file namings based on the JS files. We also carry this pattern to Kibana as that file structure is way less straight forward and plugins having their own compile file.

It does cause some weirdness with globals and shared styles/mixins. However, the tradeoff is not having to scour the files or searching in project for classes to find the right files. In all my previous projects I have worked the way you mentioned (styles in one folder, js in another) and I'm a complete convert to this pattern as a maintainer of such a large system and working within and gigantic file system like Kibana.

We are definitely happy to help you reorganize certain mixins to globals, but we will need to keep the general structure as it is.

[johnbarrierwilson]

👍🏻10-4! Thanks for shedding a bit more light on the thoughts. I'll move forward with this PR as-is!

[thompsongl]

Whoa, I really appreciate all your thought and explanation here.

The gist is, I agree that component-specific stylesheets should remain inside the same directory as the JS/TS component files, and we should reorganize some shared/global things.

Potential impacts to the current build system aren't concerning (even in the proposed reorg), but impacts to an eventual move to CSS Modules or CSS-in-JS would be fairly substantial. The main reason being that with either new approach, there will be actual interop between style files and component files (i.e., the component file will directly import the style file instead of a global manifest).

The argument for separating concerns requires more discussion. Separating technologies (HTML, CSS, JS, etc.,) is likely the wrong place for us to draw boundaries. In EUI, component state, logic, and appearance are fundamentally intertwined and require multiple technologies to accomplish the final rendered output. EUI concerns are more appropriately separated at the component level. The way we do Sass at the moment obscures this a bit, but it will become more clear as modularity is introduced.

That being said, globals are tricky in any case for EUI, and I think the current state of this PR helps address some of those and raises some really good discussion as we move toward future system choices.

[johnbarrierwilson]

That's a fantastic summary @thompsongl! Thanks for taking the time to pitch in!

[cchaos]

@johnbarrierwilson Is this ready for review? Just wasn't sure based on your comment above:

will do some more testing to make sure everything is stable, then I'll open it for review.

[johnbarrierwilson]

@cchaos Sorry, was out on Wednesday. Yes, this is ready for review now.

[cchaos]

I couldn't pull the changes to test locally because there's now a conflict, but doing a quick look through the files it looks like everything should still compile correctly. My comments are mainly suggestions for more consolidation.

EuiTooltip

We should move this mixin and variables files as well because they're prominently re-used in Kibana.

EuiPanel

Since we're already marking this as a breaking change, I'd really like to make a change to the panel mixin. It requires a "selector" but really it's asking for just the string portion of a class by automatically prepending with a ..

eui/src/global_styling/mixins/_panel.scss

Line 10 in fbcef8b

.#{$selector} {

Could you change this so that it just takes the full selector like @include euiPanel(.panel)?

Also it seems the $euiPanelBorderColor variable isn't used anywhere and should be removed.

[johnbarrierwilson]

Alrighty, those changes should be good-to-go.

• Moved the button mixins and variables
• Consolidated the form variables files
• Moved beta_badge mixins
• Moved tooltip variables and mixins
• Refactored panel mixin to require entire selector (. + string)
• Removed $euiPanelBorderColor

[thompsongl]

Make sure to move the changelog entry to the 'master' section. We've had a release since the last update.

[cchaos]

@johnbarrierwilson I just pushed you a couple of commits. Mainly moving a couple more mixins/variables around. It was easier/quicker for me to go ahead and do it because of my deep knowledge of the tokens.

I just had one more comment about the changelog, but other than that, it LGTM!

[johnbarrierwilson]

@cchaos Awesome! Thanks for walking me through my first contribution to EUI! 😊