[Bug] lab/Drawer hardcodes its close delay against a themeable hold
- Dominant language
- TypeScript
- Stars
- 13.1k
- Forks
- 1.1k
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 687
Description
### Description
`lab/Drawer` defers `dialog.close()` so its slide-out can play, and holds the drawer rendered meanwhile by transitioning `display` with `allow-discrete`. The hold is a theme value; the delay is a literal. They are the same number today by coincidence, and drift apart as soon as a theme changes.
`Drawer.tsx`:
```ts
// the hold — themeable
transitionProperty: 'transform, max-width, display',
transitionDuration: durationVars['--duration-medium'],
transitionBehavior: 'allow-discrete',
'@media (prefers-reduced-motion: reduce)': {
transitionDuration: '0.01s',
},
// the delay — literal
const duration = window.matchMedia('(prefers-reduced-motion: reduce)').matches
? 10
: 250;
closeTimeoutRef.current = setTimeout(() => {
dialog.close();
...
}, duration);
```
Four cases:
| theme | hold | delay | result |
|---|---|---|---|
| default (`--duration-medium: 410ms`) | 410ms | 250ms | inside the hold |
| `prefers-reduced-motion` | 10ms | 10ms | exactly on the boundary |
| shipped **y2k** (`motion.medium: 250`) | 250ms | 250ms | exactly on the boundary |
| any theme below 250ms | < 250ms | 250ms | **outside the hold** |
In the last row `close()` runs after `display` has already flipped to `none`. A `` opened with `showModal()` blocks the whole document for as long as it is in the top layer whether or not it is rendered, so that window is a page that looks normal and is not interactive. A browser that then fails to un-block on close leaves it that way, with no JavaScript error — which is #4290, the same failure `MobileNav` had.
Nothing validates motion values, so `defineTheme({motion: {medium: 200}})` is enough to reach the last row.
### Suggested fix
Derive the delay from the hold actually in effect rather than assuming it, as `MobileNav` now does in #4388 (`resolveCloseDelay`, reading `getComputedStyle(dialog).transitionDuration` and closing at a fraction of it). Note browsers serialise computed `
Reduced motion should shorten the *delay*, not the hold. Shortening both leaves no slack at all — one slow frame between the commit and the close macrotask and the drawer has already stopped being rendered.
### Wider point
Both components would be safe without per-component guards if theme motion values were floored where themes are defined (`expandMotionScale` / `defineTheme`). That is one change covering every component rather than one patch per component, and it also closes the `medium: 0` case that no per-component fix can reach. Raised here rather than in #4388 because it is an API call.
### Astryx Version
`main` (`packages/lab/src/Drawer/Drawer.tsx`)
Contributor guide
Research direction
Start in packages/lab/src/Drawer/Drawer.tsx and compare MobileNav’s resolveCloseDelay from #4388, then inspect expandMotionScale and defineTheme where motion values are defined. Confirm the chosen fix keeps dialog.close() within the computed transition hold for custom and reduced-motion themes, including very small durations.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100