Skip to content

Conversation

@acstll
Copy link
Contributor

@acstll acstll commented Jul 17, 2025

Summary

Implements https://github.com/elastic/eui-private/issues/195

Important

This PR targets a feature branch!

Docs draft: https://eui.elastic.co/pr_8880/docs/getting-started/theming/tokens/shadows/

Changes

  • Added new shadow tokens f48fbfc
  • Defined shadow tokens for Amsterdam, based on values from original mixins bbea477
    • in Amsterdam nothing should have changed
    • added a flat token to keep euiShadowFlat unchanged
    • in Borealis, flat falls back to xs
  • Updated mixins to use the new tokens
  • Added a docs page to Theming / Tokens / Shadows
  • Added tokens as JSON, Borealis-only
  • Added tokens as SCSS and update SCSS mixins 25ed168
  • Deprecated because undocumented and/or no longer needed
    • euiSlightShadowHover
    • euiShadowFlat
    • EuiShadowOptions.color
    • EuiShadowCustomColor
  • Added unit test to ensure overrides in theme.modify work as expected c3b2896
  • Updated VRT 1eb9aa6
  • Remove floating border for dark mode

After refactoring to (a) not expose values in theme object and (b) address double border issue

Important

If you pay attention to the commit history you'll notice we tried fixing the double border in dark mode issue by using inset in the first layer of box-shadow, this didn't work as expected (f0e2a91)

  • Update JSON tokens (SCSS too?)
  • Update docs to document the useEuiShadow hook instead of individual tokens
  • Update VRT (again)
  • Double-check overrides in theme.modify work as expected
  • Add deprecated items to deprecation calendar

Why are we making this change?

This is part of the Visual Refresh project.

Screenshots

N/A

Impact to users

No impact for users. The visual aspect of shadows is slightly different.

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in both MacOS and Windows high contrast modes
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.
    • If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist
    • If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)

@acstll acstll self-assigned this Jul 17, 2025
@acstll acstll added skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) visual refresh labels Jul 17, 2025
@mgadewoll mgadewoll self-requested a review July 21, 2025 08:26
@acstll
Copy link
Contributor Author

acstll commented Jul 21, 2025

There is an issue with dark mode in EuiPanel. The new shadows include a slight light border in dark mode. Panels also have a very similar border (in order to make the panel more visible). So this border is double the size because they add up.

I'm working on removing this extra border for dark mode but it's not straightforward because of the many moving parts (Borealis/Amsterdam, accessibility, color mode, shadow/border props).

Toggle screenshot Screenshot 2025-07-19 at 09 26 15

Edit: solving the double-border-in-dark-mode issue is not straightforward. I'm putting that on hold until we understand the problem better.

@acstll acstll marked this pull request as ready for review July 21, 2025 13:23
@acstll acstll requested a review from a team as a code owner July 21, 2025 13:23
also add new hover token, and add inset for dark mode-only floating border layer
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @acstll

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @acstll

@acstll
Copy link
Contributor Author

acstll commented Jul 30, 2025

@mgadewoll regarding adding colors to the shadows key in the theme object, I would love to know your thoughts on the following:

  • I would add only 2 keys: base (or layer) which has mode switch, and darkModeFloatingBorder (or edge) which is a single color
  • would you remove the color from the "primitive" then or just ignore it? I was thinking we should just pass the proper color to the formatting function (and ignore the color from the primitive), instead of overwriting it (creating new objects…)
    • that formatting layer will have more responsibility, adding the floating color to the first layer in dark mode

@mgadewoll
Copy link
Contributor

@mgadewoll regarding adding colors to the shadows key in the theme object, I would love to know your thoughts on the following:

  • I would add only 2 keys: base (or layer) which has mode switch, and darkModeFloatingBorder (or edge) which is a single color

  • would you remove the color from the "primitive" then or just ignore it? I was thinking we should just pass the proper color to the formatting function (and ignore the color from the primitive), instead of overwriting it (creating new objects…)

    • that formatting layer will have more responsibility, adding the floating color to the first layer in dark mode

@acstll We might want to wait to implement colors until we decide on a solution, but regardless my thoughts on the points you brought up:

  1. In terms of solution 1, I'd agree to add only 2 colors: base and edge (additional ones could be added as needed, if needed). For solution 2 we would not need edge.
  2. Let's remove the colors from the shadow primitives. We can use the definition from the theme on shadows.colors and the primitives then are obsolete.

@mgadewoll
Copy link
Contributor

ℹ️ A new PR was opened that implements shadow tokens with an alternative approach based on the comparison done for different solution approaches (comment).

@acstll
Copy link
Contributor Author

acstll commented Aug 27, 2025

Closed in favor of #8962 🥳

@acstll acstll closed this Aug 27, 2025
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.

3 participants