[CSS-in-JS] Simplify shadow parameters

[cchaos]

Based on a comment I made in #5891 :

So @constancecchen and I just ran into a similar issue with the font-sizing functions. We came up with what we think is a nice pattern we could re-use here.

• All required parameters are directly passed, while any optional ones are in the final parameter as a single object.
• If the function requires the use of any part of euiTheme it takes the whole UseEuiTheme, and always comes first

const euiMixin(
  euiTheme: UseEuiTheme;
  required: PropertyThatIsRequired;
  {
    optionalKey1?: something;
    optionalKey2?: something;
  }
) => {}

I've converted the shadow mixins to use this pattern. I think it simplifies them a lot.

I'm not considering this a breaking change since we just released these and under the Beta flag.

Checklist

• Checked in both light and dark modes
• [ ] Checked in mobile
• Checked in Chrome, Safari, Edge, and Firefox
• [ ] Props have proper autodocs and playground toggles
• [ ] Added documentation
• [ ] Checked Code Sandbox works for any docs examples
• [ ] Added or updated jest and cypress tests
• [ ] Checked for breaking changes and labeled appropriately
• [ ] Checked for accessibility including keyboard-only and screenreader modes
• [ ] Updated the Figma library counterpart
• A changelog entry exists and is marked appropriately

[kibanamachine]

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

[cee-chen]

This distinction feels somewhat confusing to me and I wish we were a bit more consistent on whether we pass the entire returned context or just euiTheme between various helpers (e.g., the typography fns all pass euiTheme.euiTheme and not the parent context, but this is also because basically every single .styles.ts file deconstructs { euiTheme } automatically in its params).

I also wish we had a clearer naming convention for the context vs the actual theme data so that we don't end up essentially deconstructing _euiTheme.euiTheme 🙈 Maybe themeContext.euiTheme instead?...

@thompsongl, any thoughts here?

[cchaos]

Agreed with all those points. I wonder if it's just easiest to say, any mixin/function should accept the entirety of UseEuiTheme whether they only need the .euiTheme portion or not? It keeps them scalable in case of future needs to grab the colorMode.

As for naming we could just change to also ensure we're naming it as lowercase useEuiTheme so that it is clear.

Suggested change

	export const euiBottomBarStyles = (_euiTheme: UseEuiTheme) => {
	const { euiTheme } = _euiTheme;
	export const euiBottomBarStyles = (useEuiTheme: UseEuiTheme) => {
	const { euiTheme } = useEuiTheme;

[thompsongl]

Yeah I had mixins passing the full result of useEuiTheme originally. The cases where mixins needed colorMode were scarce so I went the other way. But I'd rather see consistency like you've suggested if working with the current mixin shape is difficult.

We should update all mixins and the createStyleHookFromMixin util with the new pattern.

[cchaos]

++

Also I just realized we could simplify the code portion a bit too by deconstructing in the parameters.

Suggested change

	export const euiBottomBarStyles = (_euiTheme: UseEuiTheme) => {
	const { euiTheme } = _euiTheme;
	export const euiBottomBarStyles = ({ euiTheme }: UseEuiTheme) => {

[cchaos]

I pushed a commit based on those latest comments. But I didn't touch the util (that one still confuses me a bit)

[thompsongl]

I do not 😸

😄 But are you ok if I do it and add a bunch to this PR?

[thompsongl]

Or if you want to merge this I can do a follow up. Up to you!

[cchaos]

Yes, go for it. I'm out for the rest of the week.

[thompsongl]

Hmm not sure I can make a generic function that works with mixins that do have required args and mixins that do not have required args.

const euiMixin(
  euiTheme: UseEuiTheme;
  required: PropertyThatIsRequired;
  optional?: {
    optionalKey1?: something;
    optionalKey2?: something;
  }
) => {}

vs

const euiMixin(
  euiTheme: UseEuiTheme;
  optional?: {
    optionalKey1?: something;
    optionalKey2?: something;
  }
) => {}

So a couple options:

• Only have the former and you're required to pass an empty arg when no properties are required: euiScrollBarStyles(euiThemeContext, {}, {size: 20});
• Move the required args and optional args into a single object:

const euiMixin(
  euiTheme: UseEuiTheme;
  options: {
    requiredKey1: something;
    optionalKey1?: something;
    optionalKey2?: something;
  }
) => {}

• Create a second version of createStyleHookFromMixin so we have one with required args and one without
• Scrap createStyleHookFromMixin entirely and manually create hooks. (We lose the type safety/consistency that it adds to mixins; we have to just be consistent when creating new mixins).

[cee-chen]

Honestly I think my (opinionated) vote goes to

Scrap createStyleHookFromMixin entirely and manually create hooks. (We lose the type safety/consistency that it adds to mixins; we have to just be consistent when creating new mixins).

I haven't used this helper personally despite having created a few mixins and I don't super see the need for it. 🤔 It's not really that tedious to manually write a 5-line hook next to a mixin.

If we do strongly want the helper still, then my next vote goes to Move the required args and optional args into a single object

[kibanamachine]

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

[kibanamachine]

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

[kibanamachine]

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

[thompsongl]

Ok I've decided not to push anything to this branch and instead have a follow-up/parallel PR that removes the util and changes all the other mixins. We have a couple different patterns going that need to switch to the new style.

[cee-chen]

PR looks good to me as-is!

[cchaos]

Cool beans, I'll merge after CI

[kibanamachine]

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