facebook / facebook/astryx

Inset rings packed into the shadow tokens are erased by any opaque child (BottomSheet hit this; the pattern is systemic)

Open
#5,308 2 comments 0 reactions 0 assignees View on GitHub
design discussion theming
Dominant language
TypeScript
Stars
13.1k
Forks
1.1k
Avg merge
1d 14h
Merged PRs (30d)
669

Description

Every bundled theme adds a hairline inner ring to the elevation shadow tokens, dark mode only. Neutral's:

```
'--shadow-high':
'0 4px 6px light-dark(oklch(0 0 0 / 10%), oklch(0 0 0 / 50%)), ' +
'0 12px 24px light-dark(oklch(0 0 0 / 15%), oklch(0 0 0 / 70%)), ' +
'inset 0 0 0 1px light-dark(transparent, oklch(1 0 0 / 15%))'
```

The intent is good — in dark mode a drop shadow is black on near-black and gives an elevated surface no edge, so the ring supplies one. But the mechanism is fragile in a way that isn't visible from the theme file.

## The problem

**An inset shadow paints above the element's own background but below its children.** So any opaque child laid flush to the inner edge erases the ring for exactly as far as it extends. `Section` — the standard content wrapper, opaque `--color-background-surface` in its default variant — does precisely that.

The failure mode is worse than "no ring": it's a *partial* ring. Content covers the top of the surface, empty space below it does not, so the ring switches on partway down and reads to a viewer as the border changing width. Measured on the docs site, BottomSheet's left edge in dark mode (neutral theme):

| | outside | edge | just inside |
|---|---|---|---|
| above the content end | `2,2,2` | `60,60,60` | `38,38,38` |
| below the content end | `2,2,2` | `60,60,60` | **`71,71,71`** |

This is what a user reported as "the bottom sheet's border width is not consistent."

## Why a component can't fix it properly

The ring is inside a shadow token, so from a component there is no way to read it separately, lift it above children, or opt a child out. The only local moves are: hide it evenly, leave it half-covered, or make content wrappers transparent (which breaks Section's contract). PR #5305 takes the first — the BottomSheet's scrolling body paints the surface across the whole inner box, hiding the ring, and the sheet draws its own `--color-border` hairline instead. That fixes the symptom for one component and leaves the sheet's edge treatment diverging from the theme's other elevated surfaces.

## Scope

Any elevated surface whose direct content can be opaque and flush to its edge — `Dialog` (same `--shadow-high`, and its inner wrapper sets no background), `Popover`, `HoverCard`, `DropdownMenu`, `Card` with a full-bleed opaque child. Confirmed on BottomSheet; I did not measure the others (the Dialog docs example is fullscreen, and a top-layer dialog didn't pick up the themed shadow in my local harness, so that one needs checking in a real app).

`Card` is the interesting counter-example: it composes `--_card-elevation` and `--_card-ring` into its box-shadow list *and* draws a real `borderWidth`/`borderColor`. A component that owns a border is immune, because a border paints outside the padding box where no child can reach.

## Options worth discussing

1. **A dedicated `--surface-ring` token** that components apply as a border (or outline), not a shadow. Immune to children by construction; themes keep control of colour and width. Costs a token and a pass over the elevated surfaces.
2. **Themes stop putting rings in shadows**, and dark-mode edge definition comes from `--color-border` on each surface. Simplest model; means the elevated surfaces need a border they mostly don't have today.
3. **Status quo plus a documented rule**: "an elevated surface must not let an opaque child reach its inner edge," enforced by each surface painting its own background across its inner box (what #5305 does). Cheapest, but it's a rule nobody will remember, and it silently defeats the theme's ring.

I lean 1 — it keeps the theme's intent, works with an arbitrary child, and matches what Card already does by hand.

Related: #5305 (the BottomSheet fix that motivated this), #5014 (overriding one side of a colour pair voids the contrast guarantee), #2150 (dark mode research/guidelines).

Contributor guide

Open the contributing guide

Research direction

Start by inspecting the bundled theme shadow tokens and the elevated surfaces named here: BottomSheet, Dialog, Popover, HoverCard, DropdownMenu, and Card. Reproduce the dark-mode BottomSheet edge behavior, then compare how Card composes its ring and border. Done requires choosing and applying a consistent edge-treatment approach across affected surfaces, with the behavior verified for opaque children.

Written by the indexing model from the issue text.

Assessment

Tech stack
css
Domain
design, frontend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.