feat(headless): add headless Drawer primitive#9056
Conversation
A modal bottom-sheet overlay (`@clerk/headless/drawer`) built on the same Floating UI infrastructure as Dialog, with a hand-rolled pointer/transform drag engine: - Drag-to-dismiss with velocity + distance thresholds - Optional snap points (square-root overshoot damping; resets to the default on close) - Virtual-keyboard awareness, nested drawers, detached triggers - A scroll-aware drag gate that never hijacks form controls (select, range, slider thumbs, [data-cl-drawer-no-drag], and text/input/textarea/contenteditable selections) or scrolled inner content - Ships zero CSS: emits raw --cl-drawer-* custom properties and data-cl-* attributes 59 tests (drag, gate branches, snap lifecycle, nested counting, a11y).
🦋 Changeset detectedLatest commit: 2eb1809 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a new headless Drawer primitive with drag, snap, nesting, input repositioning, subcomponents, exports, docs, tests, and build/docs registry wiring. ChangesDrawer Primitive
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
packages/headless/src/primitives/drawer/drawer-handle.ts (1)
3-17: 📐 Maintainability & Code Quality | 🔵 TrivialPlease route this new handle API through a Docs review.
DrawerHandle/createDrawerHandle()look reference-facing, and this JSDoc will likely flow into generated/object/**docs. A Docs-team pass here would help confirm the detached-trigger contract is documented the way we want before release. As per path instructions, "Clerk Docs now generates/object/**reference documentation from JSDoc comments in theclerk/javascriptsource code" and "If JSDoc changes may affect generated Clerk Docs content, leave a review note reminding the contributor that the Docs team may need to review the change."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/drawer/drawer-handle.ts` around lines 3 - 17, This new reference-facing DrawerHandle/createDrawerHandle API and its JSDoc should be routed through Docs review because it will surface in generated /object/** documentation. Add a review note or equivalent reminder near the DrawerHandle interface and createDrawerHandle() implementation indicating that the Docs team should confirm the detached-trigger contract wording before release.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/wise-drawers-appear.md:
- Around line 1-2: The changeset is empty even though this PR adds the new
public primitive and export subpath for `@clerk/headless/drawer`, so it will not
generate a release entry. Update the changeset to include a minor version bump
for `@clerk/headless` and add a short summary describing the new drawer
primitive/export, or confirm that release tracking is being handled separately
before leaving it empty.
In `@packages/headless/package.json`:
- Around line 44-47: The conditional export entry for the drawer subpath in
package.json has "import" before "types", which should be reordered to put
"types" first. Update the entire exports map in the package.json exports object
so every conditional subpath follows the same order, using the drawer entry as
the pattern to fix and keeping the rest of the export targets unchanged.
In `@packages/headless/src/primitives/drawer/drawer-popup.tsx`:
- Around line 65-76: The popup’s `ownProps` in `drawer-popup.tsx` applies a
blanket `touchAction: 'none'`, which blocks native scrolling inside the entire
drawer. Update `DrawerPopup` so `touch-action` is only applied to the actual
drag handle or toggled during an active drag, and keep the rest of the popup
scrollable on touch devices.
In `@packages/headless/src/primitives/drawer/drawer-root.tsx`:
- Around line 87-109: The Drawer.Root state logic is mixing two sources of
truth: when a `handle` is present, `handleOpen` overrides `internalOpen`, so
`props.open`/`props.defaultOpen` no longer control the rendered state and
`onOpenChange` can be skipped. Update `DrawerRoot` to either keep `handle`
synchronized with the controllable state in `useControllableState`/`setOpen`,
including propagating changes through `onOpenChange`, or explicitly disallow
passing both `handle` and `open`/`defaultOpen` together. Use the `handleOpen`
and `setOpen` paths in `drawer-root.tsx` as the place to reconcile the contract.
In `@packages/headless/src/primitives/drawer/drawer.test.tsx`:
- Line 31: The type annotations in DrawerFixture, ControlsFixture, and
AncestorScrollFixture currently reference React.ComponentProps without importing
React, so the test file will not type-check. Update these signatures to use
ComponentProps imported from react, or alternatively add the missing React
import, and make sure all three fixture definitions are adjusted consistently.
In `@packages/headless/src/primitives/drawer/helpers.ts`:
- Around line 76-83: Guard the pointer-capture call in safeCapture before
invoking el[method](id), because setPointerCapture/releasePointerCapture may be
missing and cause a TypeError that bypasses the current NotFoundError handling.
Update safeCapture in helpers.ts to check that the selected method exists on the
Element (or otherwise safely no-op) before calling it, while keeping the
existing DOMException NotFoundError suppression for supported browsers.
In `@packages/headless/src/primitives/drawer/use-drawer-drag.ts`:
- Around line 249-254: The release path in use-drawer-drag is discarding
velocity direction by converting the sampled value with Math.abs, which causes
snap and dismiss logic to treat upward and downward flicks the same. Keep the
velocity signed when computing the release value and pass that signed velocity
through snap.onRelease and the downstream logic around the release checks so
direction-sensitive behavior stays correct. Update the SnapReleaseArgs contract
in use-snap-points to document the signed velocity consistently and ensure any
release handling that compares dist and v uses the signed value rather than only
magnitude.
In `@packages/headless/src/primitives/drawer/use-snap-points.ts`:
- Around line 55-64: Normalize snap-point bounds in useSnapPoints: treat an
empty snapPoints array as having no valid indices, and clamp both
opts.defaultActiveSnapPoint and opts.activeSnapPoint into the valid 0..lastIndex
range before passing them to useControllableState. Update the
lastIndex/defaultIndex logic so [] does not produce -1, and ensure restOffset
and snapTo always receive a safe index/offset. Apply the same sanitization in
the related snap-point handling around snapTo/restOffset so external values
cannot produce NaNpx.
---
Nitpick comments:
In `@packages/headless/src/primitives/drawer/drawer-handle.ts`:
- Around line 3-17: This new reference-facing DrawerHandle/createDrawerHandle
API and its JSDoc should be routed through Docs review because it will surface
in generated /object/** documentation. Add a review note or equivalent reminder
near the DrawerHandle interface and createDrawerHandle() implementation
indicating that the Docs team should confirm the detached-trigger contract
wording before release.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: d9c9de39-a770-45d5-9170-df08a70ca8a5
📒 Files selected for processing (29)
.changeset/wise-drawers-appear.mdpackages/headless/package.jsonpackages/headless/src/primitives/drawer/README.mdpackages/headless/src/primitives/drawer/constants.tspackages/headless/src/primitives/drawer/css-vars.tspackages/headless/src/primitives/drawer/drawer-backdrop.tsxpackages/headless/src/primitives/drawer/drawer-close.tsxpackages/headless/src/primitives/drawer/drawer-context.tspackages/headless/src/primitives/drawer/drawer-description.tsxpackages/headless/src/primitives/drawer/drawer-handle-grip.tsxpackages/headless/src/primitives/drawer/drawer-handle.tspackages/headless/src/primitives/drawer/drawer-popup.tsxpackages/headless/src/primitives/drawer/drawer-portal.tsxpackages/headless/src/primitives/drawer/drawer-root.tsxpackages/headless/src/primitives/drawer/drawer-title.tsxpackages/headless/src/primitives/drawer/drawer-trigger.tsxpackages/headless/src/primitives/drawer/drawer-viewport.tsxpackages/headless/src/primitives/drawer/drawer.test.tsxpackages/headless/src/primitives/drawer/helpers.tspackages/headless/src/primitives/drawer/index.tspackages/headless/src/primitives/drawer/parts.tspackages/headless/src/primitives/drawer/use-drawer-drag.tspackages/headless/src/primitives/drawer/use-reposition-inputs.tspackages/headless/src/primitives/drawer/use-snap-points.tspackages/headless/vite.config.tspackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/drawer.mdxpackages/swingset/src/stories/drawer.stories.tsx
| "./drawer": { | ||
| "import": "./dist/primitives/drawer/index.js", | ||
| "types": "./dist/primitives/drawer/index.d.ts" | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check ordering of "types" vs "import"/"require" across all export subpaths in headless package.json
jq '.exports | to_entries[] | select(.value | type == "object") | {key: .key, order: (.value | keys)}' packages/headless/package.jsonRepository: clerk/javascript
Length of output: 980
🌐 Web query:
TypeScript package.json exports "types" condition should come first resolution order documentation
💡 Result:
In the package.json "exports" field, the "types" condition must always be placed first within each conditional object [1][2]. This requirement exists because module resolution for "exports" is order-dependent [3][4]. Resolvers (including TypeScript's when using node16, nodenext, or bundler resolution modes) traverse the conditions in the order they are defined and stop at the first match [1][2]. If conditions like "import", "require", or "default" are placed before "types", the resolver may match a JavaScript file first and terminate, never evaluating the "types" condition [1][2]. This results in TypeScript failing to locate the corresponding declaration (.d.ts) files, which often leads to "any" types or resolution errors [1][2]. While the TypeScript compiler attempts to be robust, relying on correct ordering is essential for consistent and correct behavior [1][2]. This rule applies not only to the root entry but to every nested conditional block within your "exports" map [1][2]. Example of correct ordering: { "exports": { ".": { "types": "./dist/index.d.ts", "import": "./dist/index.js", "require": "./dist/index.cjs" } } }
Citations:
- 1: https://www.javascript-package-publishing.com/core-javascript-package-workflows/typescript-declaration-publishing/
- 2: https://dev.to/gabrielanhaia/the-packagejson-exports-map-is-the-most-important-file-youre-writing-wrong-5a0o
- 3: https://nodejs.org/api/packages.html
- 4: https://stackoverflow.com/questions/75153633/order-of-keys-in-package-json-exports
Reorder all conditional exports entries so types comes first.
Every subpath in packages/headless/package.json uses "import" before "types", which can cause TypeScript node16/nodenext/bundler resolution to skip the declaration file and surface any types or resolution errors. Apply the fix across the whole exports map.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/headless/package.json` around lines 44 - 47, The conditional export
entry for the drawer subpath in package.json has "import" before "types", which
should be reordered to put "types" first. Update the entire exports map in the
package.json exports object so every conditional subpath follows the same order,
using the drawer entry as the pattern to fix and keeping the rest of the export
targets unchanged.
There was a problem hiding this comment.
Valid in principle (types-first is the documented order), but holding off in this PR: all 11 existing subpaths in this file use the same import-before-types order, so it's a repo-wide convention rather than something specific to ./drawer. The package is private/workspace-consumed with moduleResolution: bundler, where tsc currently resolves types cleanly. Reordering only ./drawer would be inconsistent; reordering the whole map is a separate cleanup. Leaving open for a maintainer call.
There was a problem hiding this comment.
@alexcarpenter Thanks for the detailed context — that makes sense. Since this is a pre-existing repo-wide convention across all 11 subpaths rather than something introduced by the drawer entry, and tsc resolves types cleanly today with moduleResolution: bundler, I agree it's better scoped as a separate cleanup rather than a one-off fix here. I'll leave this open for a maintainer decision as you suggested.
If it'd be helpful, I'm happy to file a follow-up issue to track reordering types first across the whole exports map in packages/headless/package.json — just let me know.
✏️ Learnings added
Learnt from: alexcarpenter
Repo: clerk/javascript PR: 9056
File: packages/headless/package.json:44-47
Timestamp: 2026-07-01T01:38:56.726Z
Learning: In `packages/headless/package.json`, all `exports` subpath entries (e.g., `./accordion`, `./dialog`, `./drawer`, etc.) consistently order conditions as `import` before `types`. This is an established repo-wide convention, not an oversight specific to any single entry. The package is private/workspace-consumed and uses `moduleResolution: bundler`, under which `tsc` resolves types correctly despite `import` preceding `types`. Any reordering to put `types` first (per Node/TypeScript documented resolution order for `node16`/`nodenext`/`bundler`) should be applied to the entire exports map at once as a standalone cleanup, not to individual new subpath entries, to avoid inconsistency.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| const ownProps = { | ||
| ref: combinedRef, | ||
| tabIndex: -1, | ||
| 'aria-labelledby': labelId, | ||
| 'aria-describedby': descriptionId, | ||
| style: { touchAction: 'none' as const }, | ||
| // Always attached; `handleOnly` is enforced inside the engine's pointer-down gate. | ||
| onPointerDown: drag.onPointerDown, | ||
| onPointerMove: drag.onPointerMove, | ||
| onPointerUp: drag.onPointerUp, | ||
| onPointerCancel: drag.onPointerCancel, | ||
| } satisfies DefaultProps<'div'>; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Blanket touch-action: none on the sheet will block touch scrolling.
Line 70 applies touch-action: none to the entire popup, so the browser will not start native panning anywhere inside the drawer. That defeats the scroll-aware drag gating described for this primitive and will make scrollable content/forms inside the sheet feel stuck on touch devices. Please scope this to the actual drag handle or only enable it while an active drag is in progress.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/headless/src/primitives/drawer/drawer-popup.tsx` around lines 65 -
76, The popup’s `ownProps` in `drawer-popup.tsx` applies a blanket `touchAction:
'none'`, which blocks native scrolling inside the entire drawer. Update
`DrawerPopup` so `touch-action` is only applied to the actual drag handle or
toggled during an active drag, and keep the rest of the popup scrollable on
touch devices.
There was a problem hiding this comment.
Deferring as a follow-up — this is a UX call worth making deliberately. The drag engine's shouldDrag gate is built to coexist with native scroll (it bails once inner content is scrolled), but blanket touch-action: none does suppress native touch panning. Scoping it to the handle / active-drag-only is a real behavioral change; flagging rather than fixing blind in this pass.
There was a problem hiding this comment.
@alexcarpenter, that's a fair call — deferring this for deliberate UX design makes sense given the tradeoffs between native scroll and the drag engine's shouldDrag gating. Thanks for the context on how shouldDrag is intended to coexist with native scrolling.
Let me know if you'd like me to open a follow-up issue to track scoping touch-action to the handle or an active-drag-only state, and I'll include the rationale from this thread.
✏️ Learnings added
Learnt from: alexcarpenter
Repo: clerk/javascript PR: 9056
File: packages/headless/src/primitives/drawer/drawer-popup.tsx:65-76
Timestamp: 2026-07-01T01:38:56.539Z
Learning: In `packages/headless/src/primitives/drawer/drawer-popup.tsx`, the `DrawerPopup` component intentionally applies a blanket `touchAction: 'none'` style to the entire sheet container (around the `ownProps` object). This is a known tradeoff: the drag engine's `shouldDrag` gate (in `use-drawer-drag.ts`) is designed to bail out and coexist with native scrolling once inner content has scrolled, but the blanket `touch-action: none` still suppresses native touch panning at the CSS level. Scoping `touch-action` to only the drag handle or toggling it during an active drag is considered a deliberate, deferred follow-up UX decision rather than an oversight — do not flag this as a bug in future reviews unless revisiting the deferred follow-up.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: alexcarpenter
Repo: clerk/javascript PR: 8475
File: packages/headless/src/primitives/accordion/accordion-header.tsx:7-7
Timestamp: 2026-06-09T19:55:19.318Z
Learning: In `packages/headless` primitives, it’s an intentional convention to omit explicit return type annotations and JSDoc on compound sub-components like `AccordionHeader`, `AccordionTrigger`, `DialogTitle`, and `DialogTrigger`. Reviews should not require explicit “primitive boundary” typings here when the return type is already fully inferred from `renderElement` and there is no `explicit-module-boundary-types` lint rule enforced in this package. If you want to change this (e.g., add typed/documented primitive boundaries), treat it as a package-wide convention update and apply it consistently across all primitives (e.g., dialog + accordion), rather than making a one-off change to a single component.
Learnt from: alexcarpenter
Repo: clerk/javascript PR: 8475
File: packages/headless/src/primitives/accordion/accordion-item.tsx:15-15
Timestamp: 2026-06-09T19:55:22.051Z
Learning: In `packages/headless` primitives, compound sub-components (e.g., `DialogTitle`, `DialogTrigger`, `AccordionItem`, `AccordionTrigger`, etc.) intentionally rely on inferred return types from the shared `renderElement` utility and omit explicit return type annotations and JSDoc comments. During code reviews for these primitive files, do not require explicit return types/JSDoc solely to match a general convention that is not enforced in this package (e.g., no `explicit-module-boundary-types` ESLint rule for this package). If the team decides to change this convention, apply it package-wide across all primitives (dialog, accordion, etc.), not as a one-off per PR.
Learnt from: alexcarpenter
Repo: clerk/javascript PR: 8475
File: packages/headless/src/primitives/accordion/accordion-trigger.tsx:11-61
Timestamp: 2026-06-09T19:55:23.254Z
Learning: In `packages/headless`, compound sub-components for headless primitives (e.g., AccordionTrigger/Header, DialogTitle/Trigger) should be exempt from reviews that would otherwise require explicit function return types or JSDoc. These sub-components intentionally rely on return type inference from `renderElement`, and this package does not enforce boundary types via `explicit-module-boundary-types`. When reviewing files under `packages/headless/src/primitives/`, do not flag missing explicit return types or missing JSDoc for these intentionally-inferred sub-components; if the team wants to add them, it should be done as a package-wide, consistent convention across all primitives (dialog, accordion, etc.).
Learnt from: alexcarpenter
Repo: clerk/javascript PR: 8475
File: packages/headless/src/primitives/accordion/accordion-panel.tsx:12-98
Timestamp: 2026-06-09T19:55:27.412Z
Learning: In `packages/headless` primitives (e.g., accordion/dialog sub-components), it’s intentional to omit explicit return type annotations and JSDoc when the component’s return type is already inferred from `renderElement`, and this package doesn’t enforce `explicit-module-boundary-types`. During code review, do not request per-component addition of explicit boundary return types or JSDoc. If explicit return types or JSDoc should be added, it must be done package-wide across all relevant primitives (accordion, dialog, etc.), not as a one-off change to a single component/PR.
Learnt from: alexcarpenter
Repo: clerk/javascript PR: 8790
File: packages/headless/src/primitives/popover/popover-arrow.tsx:0-0
Timestamp: 2026-06-09T22:10:54.087Z
Learning: In `packages/headless` primitive components (under `packages/headless/src/primitives/`), React 18 function components must not read a consumer ref from `props.ref` (it won’t be passed through as a normal prop). When you need to combine a consumer-provided ref with an internal ref (e.g., via `useMergeRefs`), implement the component with `React.forwardRef` and merge the forwarded ref with your internal ref. During code review, flag and avoid attempts to access `props.ref` directly in these primitive components (e.g., arrow/trigger/positioner components like `PopoverArrow`, tooltip/select/menu/autocomplete/collapsible/tabs primitives).
Learnt from: alexcarpenter
Repo: clerk/javascript PR: 8790
File: packages/headless/src/primitives/tabs/tabs-root.tsx:8-17
Timestamp: 2026-06-09T22:10:58.358Z
Learning: In `packages/headless`, treat missing JSDoc on exported public primitive APIs (e.g., exported `*Props` interfaces and `*Root`/equivalent primitive components) as intentionally deferred. During code review, do not mark missing JSDoc on individual primitives in this primitives area as a blocking issue; instead, leave a non-blocking note and recommend capturing it in a single, consistent package-wide JSDoc documentation pass (e.g., by referencing/creating a follow-up tracking issue for comprehensive docs).
Learnt from: alexcarpenter
Repo: clerk/javascript PR: 8790
File: packages/headless/src/primitives/tabs/tabs-panel.tsx:0-0
Timestamp: 2026-06-09T22:11:08.401Z
Learning: In packages/headless (used with both React 18 and React 19), set the HTML `inert` attribute as the string literal "true"—not boolean `true` and not an empty string. React 18/19 differ in how boolean/falsy attribute values are dropped; `inert="true"` ensures the attribute is rendered in both versions. When conditionally rendering, use the established pattern `inert: !isSelected ? 'true' : undefined` (i.e., omit the attribute when false by passing `undefined`).
useSnapPoints computed the resting offset from window.innerHeight during render, crashing server renders of a Drawer with snapPoints (ReferenceError: window is not defined). Guard the read so it returns 0 until the client can measure; the popup mount-effect writes the real offset. Adds a node-environment renderToString regression test.
…signals Settle the parent's live scale toward the rest it is heading to on release (scaled-back if the child stays open, full on dismiss), matching the open-count drop so the styled scale animates one direction with no backward flicker. Restores the childOpen signal on onNestedRelease; snap release now returns it. Reset --cl-drawer-nested-drag-progress to 0 when a nested child opens so a prior dismiss does not leave the next parent un-scaled. Drop the unused --cl-drawer-frontmost-height var (vaul uses a fixed displacement, not a height-based peek) and document the full nested styled-layer recipe.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/headless/src/primitives/drawer/drawer.ssr.test.tsx (1)
15-27: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider asserting on rendered output, not just "no throw".
Both tests only check
.not.toThrow(). Capturing the returned HTML string and asserting it contains the expected content (e.g.,"hi") would make the tests stronger regression guards, catching cases where SSR silently swallows an error or renders empty markup.♻️ Optional strengthening
- it('renders a plain drawer on the server', () => { - expect(() => - renderToString( + it('renders a plain drawer on the server', () => { + let html = ''; + expect(() => { + html = renderToString( <Drawer.Root open> <Drawer.Portal> <Drawer.Viewport> <Drawer.Popup>hi</Drawer.Popup> </Drawer.Viewport> </Drawer.Portal> </Drawer.Root>, - ), - ).not.toThrow(); + ); + }).not.toThrow(); + expect(html).toContain('hi'); });Also applies to: 31-46
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/drawer/drawer.ssr.test.tsx` around lines 15 - 27, The SSR tests in Drawer should do more than assert renderToString does not throw; update the relevant test cases in Drawer.Root/Drawer.Portal/Drawer.Viewport/Drawer.Popup to capture the returned HTML string and assert it includes the expected rendered content such as “hi”. Keep the existing no-throw check if desired, but strengthen the test by verifying output from renderToString so the assertions fail when markup is missing or empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/headless/src/primitives/drawer/drawer.ssr.test.tsx`:
- Around line 15-27: The SSR tests in Drawer should do more than assert
renderToString does not throw; update the relevant test cases in
Drawer.Root/Drawer.Portal/Drawer.Viewport/Drawer.Popup to capture the returned
HTML string and assert it includes the expected rendered content such as “hi”.
Keep the existing no-throw check if desired, but strengthen the test by
verifying output from renderToString so the assertions fail when markup is
missing or empty.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a2445bb-0d1b-4b4b-8d71-9793d06c84ee
📒 Files selected for processing (2)
packages/headless/src/primitives/drawer/drawer.ssr.test.tsxpackages/headless/src/primitives/drawer/use-snap-points.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/headless/src/primitives/drawer/use-snap-points.ts
Address CodeRabbit review on PR #9056: - use-drawer-drag/use-snap-points: keep release velocity signed so a fast upward flick that ends net-downward settles upward (or expands a snap point) instead of being read as a downward dismiss. - use-snap-points: treat an empty `snapPoints` array as "no snap points" and clamp externally supplied indices, preventing `-1`/`NaNpx` offsets. - drawer-popup: don't surface snap state for an empty `snapPoints` array. - helpers: guard `safeCapture` against environments missing set/releasePointerCapture (an absent method threw TypeError past the NotFoundError catch). - use-reposition-inputs: clear the popup lift when the keyboard closes so the sheet doesn't stay raised until unmount.
…ched handle Following Base UI's dialog-handle model: the component's controllable open state is the one source of truth, and every transition — including imperative handle calls — flows through a single `setOpen` that fires `onOpenChange`. Previously, passing a `handle` made the handle store the source of truth, so `open`/`defaultOpen` were ignored and handle-driven transitions (detached trigger clicks, `handle.open()`) bypassed `onOpenChange`. Now `Drawer.Root` owns open state via `useControllableState`, and the handle is a bridge: `connect()` routes its imperative calls back through the root's `setOpen`, `isOpen` reflects the root, and the root `emit()`s on change so detached triggers re-read. An open requested before the root mounts is adopted on connect without clobbering `defaultOpen`. Fixes the CodeRabbit contract finding on PR #9056.
1 participant
Summary
Adds a headless Drawer primitive to
@clerk/headless(@clerk/headless/drawer): a modal bottom-sheet overlay with drag-to-dismiss, optional snap points, virtual-keyboard awareness, nested drawers, and detached triggers. It reuses the same Floating UI infrastructure asDialog(portal, focus trap, scroll lock, dismiss,FloatingTreenesting, enter/exit transitions) with a hand-rolled pointer/transform drag engine layered on top.What's included
Root,Trigger,Portal,Backdrop,Viewport,Popup,Handle,Title,Description,Close.getComputedStyle), decisions driven off refs + an injectable clock so it's deterministic under happy-dom.<select>, nativerangeinputs,[role="slider"]thumbs,[data-cl-drawer-no-drag], and text /input/textarea/contenteditableselections) or scrolled inner content.autoFocusdefaults tofalse(unlike Dialog) so opening on touch does not summon the keyboardSummary by CodeRabbit
./drawerpublic entry point, plus a styling contract via registered CSS variables anddata-cl-*state attributes.