Skip to content

Conversation

@mgadewoll
Copy link
Contributor

@mgadewoll mgadewoll commented Feb 12, 2025

Summary

Important

This PR merges into a feature branch.

closes https://github.com/elastic/eui-private/issues/128

This PR resolves any leftover direct imports from /themes/amsterdam in /eui.

Changes

  1. Moves currently shared styles to eui-theme-common package
  • moves shadow functions (e.g euiShadow)
    • leaves hook versions (e.g. useEuiShadow) in /eui due to the dependency on the theme hook useEuiTheme and EuiProvider component <- to move these we'll need to separate the package code further
  • moves breakpoint
  1. Moves types
  • internally moves a types files in eui-theme-common
  • duplicates UseEuiTheme type
  1. Updates imports in /eui
  • updates imports from themes/amsterdam for shadows, breakpoints, colorVis and buttons

Note

The changes have been tested against Kibana (testing draft PR) to ensure nothing breaks unexpectedly.
This currently still includes the weekly release upgrade, which obscures the meta stats result.
I'll rebase and rerun the Kibana testing PR once the release upgrade is merged.
✅ Currently rerunning Kibana CI after merge and rebase of the release upgrade PR

Screenshot 2025-02-14 at 11 05 25

QA

  • CI passes (docs & storybook build as expected)
  • verify package builds run as expected
    • run yarn build in /packages/eui
    • run yarn build in /packages/eui-theme-common
  • Smoke test EUI/EUI+ docs pages for shadow functionality (Amsterdam & Borealis)
  • EuiCard (EUI, EUI+)
  • EuiKeyPadMenu (EUI, EUI+)
  • EuiFlyout (EUI, EUI+)
  • EuiModal (EUI, EUI+)
  • EuiPanel (EUI, EUI+)
  • EuiToast (EUI, EUI+)
  • EuiToolTip (EUI, EUI+)

@mgadewoll mgadewoll added skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) visual refresh labels Feb 12, 2025
@mgadewoll mgadewoll self-assigned this Feb 12, 2025
@mgadewoll mgadewoll force-pushed the eui-theme/128-resolve-theme-imports branch from 61e755a to 9e64bf6 Compare February 13, 2025 06:11
@mgadewoll mgadewoll marked this pull request as ready for review February 13, 2025 09:42
@mgadewoll mgadewoll requested a review from a team as a code owner February 13, 2025 09:42
@weronikaolejniczak
Copy link
Contributor

Testing notes

CI pipeline and deployments

  • Storybook ✅
  • Current docs ✅
  • New docs ✅

Package builds

  • yarn workspace @elastic/eui build
  • yarn workspace @elastic/eui-theme-common build

Smoke tests

  • EuiCard ✅
  • EuiKeyPadMenu ✅
  • EuiFlyout ✅
  • EuiModal ✅
  • EuiPanel ✅
  • EuiToast ✅
  • EuiToolTip ✅

⚠️ Just noticed a small "issue" (?) with Flyout overlay outside of the changes of this PR, thought I'll mention it anyway:

Screenshot 2025-02-13 at 11 08 19

There's a small 1px line at the top in the current docs. Not sure if this is on purpose to separate the header from the content? In light mode it looks a bit "off".

@mgadewoll
Copy link
Contributor Author

mgadewoll commented Feb 13, 2025

⚠️ Just noticed a small "issue" (?) with Flyout overlay outside of the changes of this PR, thought I'll mention it anyway:

Screenshot 2025-02-13 at 11 08 19

There's a small 1px line at the top in the current docs. Not sure if this is on purpose to separate the header from the content? In light mode it looks a bit "off".

Yes, that's expected. It was introduced in this PR (issue) that headers in dark mode have a border applied.
(we're using the dark variant header for our docs)

- choosing duplication over moving the type to keep the current separation for base theme fundamentals and helpers and the EUI theme usage code
@mgadewoll
Copy link
Contributor Author

mgadewoll commented Feb 13, 2025

ℹ️ After talking with @tkajtoch we agreed to keep the UseEuiTheme in /eui and rather duplicate it into eui-theme-common to keep concerns separate for now (theme fundamentals vs EUI theme context/consumption). (commit)

@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mgadewoll

@mgadewoll mgadewoll merged commit b698b25 into elastic:eui-theme/borealis Feb 14, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) visual refresh

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants