fix(components): add showOverlay prop to Drawer to support non-modal usage#10952
fix(components): add showOverlay prop to Drawer to support non-modal usage#10952ShauryaaSharma wants to merge 1 commit into
Conversation
|
@ShauryaaSharma is attempting to deploy a commit to the shadcn-pro Team on Vercel. A member of the Team first needs to authorize it. |
…l usage Fixes shadcn-ui#10870 - when a Drawer was open, focus was trapped inside making it impossible to focus inputs or textareas on the rest of the page. Root cause: vaul defaults to modal=true which activates Radix UI FocusScope inside DrawerContent, trapping all keyboard focus. Changes: - Add showOverlay prop (default true) to DrawerContent across all registry and generated style variants so the backdrop can be omitted for non-modal drawers - Document modal={false} and showOverlay={false} together in base and radix docs under a new Non-modal Drawer example section - Add drawer-non-modal example files for base and radix previews - Regenerate public/r drawer JSON and examples index via pnpm registry:build Usage: <Drawer modal={false}> <DrawerContent showOverlay={false}>...</DrawerContent> </Drawer>
a10291d to
bde377e
Compare
|
Hey @shadcn — tagging you since the contributing guide points here for questions. This fixes #10870 where an open Drawer traps focus, preventing users from typing into inputs/textareas outside it. The fix is two props working together:
Both default to their existing behaviour so there's no breaking change. Happy to adjust anything — naming, docs wording, or whether you'd prefer this documented differently. |
daltino
left a comment
There was a problem hiding this comment.
Review: fix(components): add showOverlay prop to Drawer to support non-modal usage
Nice work addressing a real pain point! The non-modal drawer pattern is genuinely useful and this PR moves in the right direction. That said, there are a few things worth discussing before merging.
🔍 What I Can See
The patches are truncated, so I'm working with partial visibility into the actual component changes. My comments will flag things I can confirm plus questions raised by what's missing.
✅ What's Working Well
- Clear documentation – The MDX additions explain why you need both
modal={false}andshowOverlay={false}together. That two-prop requirement isn't obvious, so calling it out explicitly is great. - Dual-theme coverage – Both
baseandradixvariants get the example and docs, which is consistent with how the rest of the component suite is structured. - Real-world example – Using an
<Input>and<Label>outside the drawer in the demo directly illustrates the bug being fixed. Good choice.
⚠️ Issues & Suggestions
1. The showOverlay prop isn't visible in the patch — is it actually implemented?
The PR title and description promise a showOverlay prop on DrawerContent, but the changed files shown are only docs, examples, and an index file. The actual component file (e.g. ui/drawer.tsx) doesn't appear in the diff.
Questions:
- Where is
showOverlaywired up inDrawerContent? - Is the overlay conditionally rendered based on this prop?
- Does it interact with vaul's internal overlay rendering, or is it shadcn's own overlay element?
If this is missing, it's a blocker — the docs would document behavior that doesn't exist yet.
2. Coupling two props is fragile UX
The docs say:
set
modal={false}on<Drawer>andshowOverlay={false}on<DrawerContent>
This is awkward. Users forgetting either one will get broken behavior silently:
modal={false}withoutshowOverlay={false}→ focus isn't trapped but the overlay still blocks interactionshowOverlay={false}withoutmodal={false}→ overlay is gone but focus is still trapped
Suggestion: Consider whether modal={false} on <Drawer> could automatically suppress the overlay via context, so users only need one prop. If that's too invasive, at minimum add a console.warn in dev when showOverlay={false} is set but modal is still true (or vice versa).
3. showOverlay naming could be more idiomatic
showOverlay={false} is a double-negative pattern. It's more readable to write:
// Instead of:
<DrawerContent showOverlay={false}>
// Consider:
<DrawerContent overlay={false}>
// or
<DrawerContent hideOverlay>Minor, but prop naming is part of the public API and hard to change later.
4. Example files are truncated — need to verify outside input placement
Both drawer-non-modal.tsx files are cut off at ~24 lines (of 55). Based on the description, there should be an <Input> rendered outside <DrawerContent>. Please confirm:
- The outside
<Input>is rendered as a sibling of<Drawer>, not inside it - The example actually demonstrates focus moving to the outside input while the drawer is open (ideally with a comment explaining this)
5. No accessibility notes in docs
Non-modal drawers have meaningful accessibility implications. When focus isn't trapped:
- Screen reader users may not know the drawer is open
- The drawer might not be announced as a dialog
aria-modalbehavior changes
Suggestion: Add a short
> **Accessibility note:** Non-modal drawers do not trap focus or block background
> interaction. Ensure your use case does not disorient keyboard or screen reader users.
> Consider whether a modal drawer or a different component (e.g. a sidebar) better
> fits your needs.
6. __index__.tsx only registers the radix variant
filePath: "examples/radix/drawer-non-modal.tsx",But the base variant file (examples/base/drawer-non-modal.tsx) is also added. Is the base variant registered separately elsewhere? If not, <ComponentPreview styleName="base-nova" name="drawer-non-modal" /> in the base docs will silently fail or render nothing.
📋 Summary
| Area | Status |
|---|---|
Core showOverlay implementation |
❓ Not visible in diff |
| Documentation | ✅ Good, needs accessibility note |
| Example demos | ✅ Looks right ( |
|
Hello @daltino |
Fixes #10870
What
When a Drawer is open, focus is trapped inside, making it impossible to focus
an
<input>or<textarea>outside the drawer element.Why
vaul defaults
modal={true}, which activates Radix UI'sFocusScopeinsideDrawerContent. This intercepts Tab/Shift-Tab and prevents focus from everleaving the drawer. Additionally,
DrawerContentalways renderedDrawerOverlay(the dark backdrop), blocking the rest of the page visually even in non-modal
scenarios.
How
showOverlay?: boolean(defaulttrue) toDrawerContentacross allregistry source files and generated style variants — fully backward-compatible
modalprop alongside the newshowOverlayprop in both baseand radix docs under a new Non-modal Drawer example section
drawer-non-modalexample files for base and radix previewspnpm registry:buildto regenerate all affectedpublic/rJSON filesand
examples/__index__.tsxUsage