Skip to content

[CSS-in-JS] Update mixin function API pattern#5904

Merged
thompsongl merged 12 commits intoelastic:mainfrom
thompsongl:mixin-refactor
May 26, 2022
Merged

[CSS-in-JS] Update mixin function API pattern#5904
thompsongl merged 12 commits intoelastic:mainfrom
thompsongl:mixin-refactor

Conversation

@thompsongl
Copy link
Copy Markdown
Contributor

@thompsongl thompsongl commented May 17, 2022 •
edited
Loading

Summary

Related to #5892

Refactoring mixin functions to use the following API pattern

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

Checklist

  • Checked in both light and dark modes
  • Props have proper autodocs and playground toggles
  • Checked Code Sandbox works for any docs examples
  • Checked for breaking changes and labeled appropriately
  • A changelog entry exists and is marked appropriately

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented May 17, 2022

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

@thompsongl thompsongl marked this pull request as ready for review May 23, 2022 16:33
@kibanamachine
Copy link
Copy Markdown

kibanamachine commented May 23, 2022

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

@cchaos
Copy link
Copy Markdown
Contributor

cchaos commented May 23, 2022

Can you add a section to the wiki around this pattern?

@thompsongl thompsongl requested a review from cchaos May 24, 2022 19:02
cee-chen
cee-chen reviewed May 24, 2022
View reviewed changes
@kibanamachine
Copy link
Copy Markdown

kibanamachine commented May 24, 2022

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

cchaos
cchaos reviewed May 25, 2022
View reviewed changes
@thompsongl thompsongl requested a review from cchaos May 25, 2022 19:33
@kibanamachine
Copy link
Copy Markdown

kibanamachine commented May 25, 2022

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

cchaos
cchaos approved these changes May 26, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Thank you!

cee-chen
cee-chen approved these changes May 26, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

My EuiText PR will probably cause some merge conflicts with this, would super appreciate that merging in first if possible (due to CI shenanigans and me being out after today) 🙇

@thompsongl
Copy link
Copy Markdown
Contributor Author

thompsongl commented May 26, 2022

would super appreciate that merging in first if possible

No problem!

@thompsongl thompsongl enabled auto-merge (squash) May 26, 2022 21:07
@kibanamachine
Copy link
Copy Markdown

kibanamachine commented May 26, 2022

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

@thompsongl thompsongl merged commit 805960c into elastic:main May 26, 2022
@thompsongl thompsongl mentioned this pull request Jun 8, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@cee-chen cee-chen cee-chen approved these changes

@cchaos cchaos cchaos approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@thompsongl @kibanamachine @cchaos @cee-chen