Skip to content

fix(components): Remove doubled shadow on SideDrawer #2933

Merged
Aiden-Brine merged 1 commit intomasterfrom
JOB-153152/sidedrawer
Mar 2, 2026
Merged

fix(components): Remove doubled shadow on SideDrawer #2933
Aiden-Brine merged 1 commit intomasterfrom
JOB-153152/sidedrawer

Conversation

@Aiden-Brine
Copy link
Contributor

@Aiden-Brine Aiden-Brine commented Feb 27, 2026

Motivations

Here's a PR description based on everything we explored:


fix(components): Remove doubled shadow on SideDrawer header and footer

When the SideDrawer contains scrollable content, a scroll-position shadow is applied to both the header (shadow below) and footer (shadow above) to visually indicate that content is hidden behind them. However, these shadows were also bleeding outside the drawer to the left, creating a doubled shadow alongside the drawer's own elevation shadow.

This happened because box-shadow on a child element is not clipped to its parent's bounds. The .container element carries the drawer's overall elevation shadow (--shadow-base), and when the header/footer received their own box-shadow: var(--shadow-base), that shadow also painted leftward outside the drawer — on top of the existing elevation shadow.

The footer had a custom box-shadow override, but it still bled due to its 12px blur radius spreading in all directions, including leftward.

Both the standard (fixed) and inline variants were affected.


Added

Added overflow: clip to .container:

.container {
  overflow: clip;
}

This clips any descendant paint that extends outside the container's bounds, preventing the header and footer shadows from bleeding outside the drawer. The container's own box-shadow (the elevation shadow) is unaffected, overflow only clips descendants, not the element's own paint.

Why overflow: clip and not overflow: hidden

overflow: hidden creates a scroll container, which breaks position: sticky on descendant elements. Both the header and footer rely on position: sticky to stay anchored at the top and bottom of the drawer. Using overflow: hidden would cause them to scroll away with the content.

overflow: clip was introduced specifically to address this. It clips descendant overflow paint without creating a scroll container, so position: sticky continues to work correctly. overflow: clip is well-supported in all browsers we target (Chrome 90+, Firefox 81+, Safari 16+).

Potential side effects I explored

Focus rings (--shadow-focus)

Focus rings in this design system are implemented as box-shadow with a 4px spread. Any focused element near the container's left edge could have its focus ring clipped. However the .content div already has overflow-y: auto, which per the CSS spec promotes overflow-x: visible to auto as well. So focus rings were already being clipped by .content's overflow for any element at the left edge. overflow: clip on .container changes nothing here.

Inline dropdown components (MultiSelect and DataListItemActions)

These components render their floating UI inline in the DOM (no portal). It is the same story as focus rings , .content's overflow-y: auto was already clipping them. overflow: clip on .container introduces no new behaviour for anything inside .content.

Portalled components (everything else)

Combobox, Menu, Autocomplete, Popover, Tooltip, Modal, LightBox, Toast, DatePicker and DataListActionsMenu all portal their floating UI to document.body. They are completely unaffected.

Testing

Open Storybook and navigate to Components → Overlays → SideDrawer → Web.

  1. Shadow fix — Open the Basic story (which has a footer). Scroll the content so the header and footer shadows appear. Check the left edge of the drawer: there should be a single clean shadow, not a doubled/intensified one.

  2. Header shadow only — Open any story without a footer and scroll down. Same check on the header's left edge.

  3. Inline variant — Open the Inline story. Scroll the content and check both the header and footer shadow edges.

  4. Sticky header/footer — Confirm the header and footer remain sticky (don't scroll away with content) in any story with scrollable content.

Both the standard (fixed) and inline variants were affected.

Changes can be
tested via Pre-release


In Atlantis we use Github's built in pull request reviews.

@Aiden-Brine Aiden-Brine force-pushed the JOB-153152/sidedrawer branch from bea6d9b to 8af013d Compare February 27, 2026 22:52
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Feb 27, 2026

Deploying atlantis with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8af013d
Status: ✅  Deploy successful!
Preview URL: https://41b5e2df.atlantis.pages.dev
Branch Preview URL: https://job-153152-sidedrawer.atlantis.pages.dev

View logs

@Aiden-Brine Aiden-Brine changed the title fix(components): Remove doubled shadow on SideDrawer header and footer fix(components): Remove doubled shadow on SideDrawer Feb 27, 2026
@ZakaryH
Copy link
Contributor

ZakaryH commented Mar 2, 2026

I know we're not doing it now and there's room for improvement, so I was curious what would happen if we started applying a focus-visible style to the container since that appears to be where we programmatically set focus when the SideDrawer opens.

could use a bit of tweaking possibly, but it still works!

image

Copy link
Contributor

@ZakaryH ZakaryH left a comment

Choose a reason for hiding this comment

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

LGTM

@Aiden-Brine Aiden-Brine merged commit 4dda576 into master Mar 2, 2026
17 checks passed
@Aiden-Brine Aiden-Brine deleted the JOB-153152/sidedrawer branch March 2, 2026 23:52
@ZakaryH
Copy link
Contributor

ZakaryH commented Mar 3, 2026

ah one thing I should have mentioned is, does this show up in a visual test?

I see we have a test for SideDrawer. unclear if it has the right conditions, and if the double shadow either didn't trigger a big enough diff, or maybe it's not showing up for some reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants