Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/drawer-select-escape.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/ui': patch
---

Fix pressing `Escape` while a `Select` is open inside a `Drawer` (for example the payment method picker in Checkout) dismissing the entire Drawer. `Escape` now closes only the open `Select` and leaves the Drawer open. The `Select` now wires up its floating interaction props so it handles `Escape` itself, and the `Drawer` roots a floating tree so nested floating elements are recognized as its children.
61 changes: 42 additions & 19 deletions packages/ui/src/elements/Drawer.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,10 +2,14 @@ import { usePortalRoot, useSafeLayoutEffect } from '@clerk/shared/react/index';
import type { UseDismissProps, UseFloatingOptions, UseRoleProps } from '@floating-ui/react';
import {
FloatingFocusManager,
FloatingNode,
FloatingPortal,
FloatingTree,
useClick,
useDismiss,
useFloating,
useFloatingNodeId,
useFloatingParentNodeId,
useInteractions,
useMergeRefs,
useRole,
Expand DownExpand Up@@ -78,7 +82,22 @@ interface RootProps {
dismissProps?: UseDismissProps;
}

function Root({
function Root(props: RootProps) {
// The Drawer must be the root of a FloatingTree so that nested floating
// elements (e.g. a Select) register as its children. Without this, pressing
// Escape to close a nested popover also dismisses the Drawer.
const parentNodeId = useFloatingParentNodeId();
if (parentNodeId == null) {
return (
<FloatingTree>
<RootContent {...props} />
</FloatingTree>
);
}
return <RootContent {...props} />;
}

function RootContent({
children,
open,
onOpenChange,
Expand All@@ -90,10 +109,12 @@ function Root({
const direction = useDirection();
const portalRoot = usePortalRoot();
const effectivePortalRoot = portalProps?.root ?? portalRoot?.() ?? undefined;
const nodeId = useFloatingNodeId();

const { refs, context } = useFloating({
open,
onOpenChange,
nodeId,
transform: false,
strategy,
placement: direction === 'ltr' ? 'right' : 'left',
Expand All@@ -107,25 +128,27 @@ function Root({
]);

return (
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
<FloatingNode id={nodeId}>
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
</FloatingNode>
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/ui/src/elements/Select.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -289,7 +289,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
} = useSelectState();
const { filteredItems: options, searchInputProps } = searchInputCtx;
const [focusedIndex, setFocusedIndex] = useState(0);
const { isOpen, floating, styles, nodeId, context } = popoverCtx;
const { isOpen, floating, styles, nodeId, context, getFloatingProps } = popoverCtx;
const containerRef = React.useRef<HTMLDivElement>(null);
const effectiveListboxId = id ?? generatedListboxId;
const effectiveAriaLabelledBy = ariaLabelledBy ?? (ariaLabel ? undefined : triggerId);
Expand DownExpand Up@@ -361,7 +361,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
elementDescriptor={descriptors.selectOptionsContainer}
elementId={descriptors.selectOptionsContainer.setId(elementId)}
ref={floating}
onKeyDown={onKeyDown}
{...getFloatingProps({ onKeyDown })}
direction='col'
justify='start'
sx={[
Expand Down
72 changes: 72 additions & 0 deletions packages/ui/src/elements/__tests__/Drawer.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
import { ClerkInstanceContext } from '@clerk/shared/react';
import { fireEvent, render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { PropsWithChildren } from 'react';
import { describe, expect, it, vi } from 'vitest';

import { EnvironmentProvider } from '../../contexts';
import { AppearanceProvider } from '../../customizables';
import { InternalThemeProvider } from '../../styledSystem';
import { Drawer } from '../Drawer';
import { Select, SelectButton, SelectOptionList } from '../Select';

const options = [
{ value: 'one', label: 'One' },
{ value: 'two', label: 'Two' },
];

const TestProviders = ({ children }: PropsWithChildren) => (
<ClerkInstanceContext.Provider value={{ value: { client: {}, user: {} } as any }}>
<EnvironmentProvider value={{ displayConfig: { applicationName: 'TestApp' } } as any}>
Comment on lines +19 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== Drawer.test.tsx ==\n'
cat -n packages/ui/src/elements/__tests__/Drawer.test.tsx
printf'\n== Search ClerkInstanceContext / EnvironmentProvider definitions ==\n'
rg -n "ClerkInstanceContext|EnvironmentProvider|displayConfig|applicationName|value: \{\s*client" packages/ui/src packages -g '!**/dist/**' -g '!**/build/**'printf'\n== Type outlines for likely provider files ==\n'
fd -a "ClerkInstanceContext" packages/ui/src packages ||true
fd -a "EnvironmentProvider" packages/ui/src packages ||true

Repository: clerk/javascript

Length of output: 50373


🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== packages/ui/src/contexts/EnvironmentContext.tsx ==\n'
cat -n packages/ui/src/contexts/EnvironmentContext.tsx
printf'\n== packages/ui/src/test/create-fixtures.tsx (relevant slice) ==\n'
sed -n '1,220p' packages/ui/src/test/create-fixtures.tsx
printf'\n== Search for reusable clerk/environment test fixtures in packages/ui ==\n'
rg -n "create-fixtures|environmentMock|ClerkInstanceContext.Provider|EnvironmentProvider value=" packages/ui/src -g '!**/dist/**' -g '!**/build/**'

Repository: clerk/javascript

Length of output: 20221


Use typed fixtures for these providers. Replace the as any casts with a typed Clerk/environment mock so ClerkInstanceContext and EnvironmentProvider shape changes surface here instead of being hidden by the test setup.

🤖 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/ui/src/elements/__tests__/Drawer.test.tsx` around lines 19 - 20,
Replace the as any casts in the ClerkInstanceContext.Provider and
EnvironmentProvider fixtures within Drawer.test.tsx with properly typed Clerk
and environment mocks. Ensure both provider values satisfy their respective
context types while preserving the existing test data and exposing future
provider shape changes to TypeScript.

Source: Coding guidelines

<AppearanceProvider>
<InternalThemeProvider>{children}</InternalThemeProvider>
</AppearanceProvider>
</EnvironmentProvider>
</ClerkInstanceContext.Provider>
);

describe('Drawer', () => {
it('does not close the Drawer when Escape dismisses an open nested Select', async () => {
const user = userEvent.setup({ delay: null });
const onOpenChange = vi.fn();

render(
<Drawer.Root
open
onOpenChange={onOpenChange}
>
<Drawer.Content>
<Select
options={options}
value={null}
onChange={vi.fn()}
portal
>
<SelectButton />
<SelectOptionList />
</Select>
</Drawer.Content>
</Drawer.Root>,
{ wrapper: TestProviders },
);

await user.click(screen.getByRole('button', { name: 'Select an option' }));
const listbox = await screen.findByRole('listbox');

// A real browser moves focus into the open Select; jsdom does not, so focus
// it explicitly before dispatching Escape from within it.
listbox.focus();
fireEvent.keyDown(listbox, { key: 'Escape', code: 'Escape' });

// Escape closes the nested Select but must not bubble up to dismiss the
// Drawer itself.
expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
expect(onOpenChange).not.toHaveBeenCalled();
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// With the Select closed, a second Escape now dismisses the Drawer.
// floating-ui invokes onOpenChange(open, event, reason), so assert on the open arg.
fireEvent.keyDown(document.body, { key: 'Escape', code: 'Escape' });
expect(onOpenChange).toHaveBeenCalled();
expect(onOpenChange.mock.lastCall?.[0]).toBe(false);
});
});
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/drawer-select-escape.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/ui': patch
---

Fix pressing `Escape` while a `Select` is open inside a `Drawer` (for example the payment method picker in Checkout) dismissing the entire Drawer. `Escape` now closes only the open `Select` and leaves the Drawer open. The `Select` now wires up its floating interaction props so it handles `Escape` itself, and the `Drawer` roots a floating tree so nested floating elements are recognized as its children.
61 changes: 42 additions & 19 deletions packages/ui/src/elements/Drawer.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,10 +2,14 @@ import { usePortalRoot, useSafeLayoutEffect } from '@clerk/shared/react/index';
import type { UseDismissProps, UseFloatingOptions, UseRoleProps } from '@floating-ui/react';
import {
FloatingFocusManager,
FloatingNode,
FloatingPortal,
FloatingTree,
useClick,
useDismiss,
useFloating,
useFloatingNodeId,
useFloatingParentNodeId,
useInteractions,
useMergeRefs,
useRole,
Expand DownExpand Up@@ -78,7 +82,22 @@ interface RootProps {
dismissProps?: UseDismissProps;
}

function Root({
function Root(props: RootProps) {
// The Drawer must be the root of a FloatingTree so that nested floating
// elements (e.g. a Select) register as its children. Without this, pressing
// Escape to close a nested popover also dismisses the Drawer.
const parentNodeId = useFloatingParentNodeId();
if (parentNodeId == null) {
return (
<FloatingTree>
<RootContent {...props} />
</FloatingTree>
);
}
return <RootContent {...props} />;
}

function RootContent({
children,
open,
onOpenChange,
Expand All@@ -90,10 +109,12 @@ function Root({
const direction = useDirection();
const portalRoot = usePortalRoot();
const effectivePortalRoot = portalProps?.root ?? portalRoot?.() ?? undefined;
const nodeId = useFloatingNodeId();

const { refs, context } = useFloating({
open,
onOpenChange,
nodeId,
transform: false,
strategy,
placement: direction === 'ltr' ? 'right' : 'left',
Expand All@@ -107,25 +128,27 @@ function Root({
]);

return (
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
<FloatingNode id={nodeId}>
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
</FloatingNode>
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/ui/src/elements/Select.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -289,7 +289,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
} = useSelectState();
const { filteredItems: options, searchInputProps } = searchInputCtx;
const [focusedIndex, setFocusedIndex] = useState(0);
const { isOpen, floating, styles, nodeId, context } = popoverCtx;
const { isOpen, floating, styles, nodeId, context, getFloatingProps } = popoverCtx;
const containerRef = React.useRef<HTMLDivElement>(null);
const effectiveListboxId = id ?? generatedListboxId;
const effectiveAriaLabelledBy = ariaLabelledBy ?? (ariaLabel ? undefined : triggerId);
Expand DownExpand Up@@ -361,7 +361,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
elementDescriptor={descriptors.selectOptionsContainer}
elementId={descriptors.selectOptionsContainer.setId(elementId)}
ref={floating}
onKeyDown={onKeyDown}
{...getFloatingProps({ onKeyDown })}
direction='col'
justify='start'
sx={[
Expand Down
72 changes: 72 additions & 0 deletions packages/ui/src/elements/__tests__/Drawer.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
import { ClerkInstanceContext } from '@clerk/shared/react';
import { fireEvent, render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { PropsWithChildren } from 'react';
import { describe, expect, it, vi } from 'vitest';

import { EnvironmentProvider } from '../../contexts';
import { AppearanceProvider } from '../../customizables';
import { InternalThemeProvider } from '../../styledSystem';
import { Drawer } from '../Drawer';
import { Select, SelectButton, SelectOptionList } from '../Select';

const options = [
{ value: 'one', label: 'One' },
{ value: 'two', label: 'Two' },
];

const TestProviders = ({ children }: PropsWithChildren) => (
<ClerkInstanceContext.Provider value={{ value: { client: {}, user: {} } as any }}>
<EnvironmentProvider value={{ displayConfig: { applicationName: 'TestApp' } } as any}>
Comment on lines +19 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== Drawer.test.tsx ==\n'
cat -n packages/ui/src/elements/__tests__/Drawer.test.tsx
printf'\n== Search ClerkInstanceContext / EnvironmentProvider definitions ==\n'
rg -n "ClerkInstanceContext|EnvironmentProvider|displayConfig|applicationName|value: \{\s*client" packages/ui/src packages -g '!**/dist/**' -g '!**/build/**'printf'\n== Type outlines for likely provider files ==\n'
fd -a "ClerkInstanceContext" packages/ui/src packages ||true
fd -a "EnvironmentProvider" packages/ui/src packages ||true

Repository: clerk/javascript

Length of output: 50373


🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== packages/ui/src/contexts/EnvironmentContext.tsx ==\n'
cat -n packages/ui/src/contexts/EnvironmentContext.tsx
printf'\n== packages/ui/src/test/create-fixtures.tsx (relevant slice) ==\n'
sed -n '1,220p' packages/ui/src/test/create-fixtures.tsx
printf'\n== Search for reusable clerk/environment test fixtures in packages/ui ==\n'
rg -n "create-fixtures|environmentMock|ClerkInstanceContext.Provider|EnvironmentProvider value=" packages/ui/src -g '!**/dist/**' -g '!**/build/**'

Repository: clerk/javascript

Length of output: 20221


Use typed fixtures for these providers. Replace the as any casts with a typed Clerk/environment mock so ClerkInstanceContext and EnvironmentProvider shape changes surface here instead of being hidden by the test setup.

🤖 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/ui/src/elements/__tests__/Drawer.test.tsx` around lines 19 - 20,
Replace the as any casts in the ClerkInstanceContext.Provider and
EnvironmentProvider fixtures within Drawer.test.tsx with properly typed Clerk
and environment mocks. Ensure both provider values satisfy their respective
context types while preserving the existing test data and exposing future
provider shape changes to TypeScript.

Source: Coding guidelines

<AppearanceProvider>
<InternalThemeProvider>{children}</InternalThemeProvider>
</AppearanceProvider>
</EnvironmentProvider>
</ClerkInstanceContext.Provider>
);

describe('Drawer', () => {
it('does not close the Drawer when Escape dismisses an open nested Select', async () => {
const user = userEvent.setup({ delay: null });
const onOpenChange = vi.fn();

render(
<Drawer.Root
open
onOpenChange={onOpenChange}
>
<Drawer.Content>
<Select
options={options}
value={null}
onChange={vi.fn()}
portal
>
<SelectButton />
<SelectOptionList />
</Select>
</Drawer.Content>
</Drawer.Root>,
{ wrapper: TestProviders },
);

await user.click(screen.getByRole('button', { name: 'Select an option' }));
const listbox = await screen.findByRole('listbox');

// A real browser moves focus into the open Select; jsdom does not, so focus
// it explicitly before dispatching Escape from within it.
listbox.focus();
fireEvent.keyDown(listbox, { key: 'Escape', code: 'Escape' });

// Escape closes the nested Select but must not bubble up to dismiss the
// Drawer itself.
expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
expect(onOpenChange).not.toHaveBeenCalled();
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// With the Select closed, a second Escape now dismisses the Drawer.
// floating-ui invokes onOpenChange(open, event, reason), so assert on the open arg.
fireEvent.keyDown(document.body, { key: 'Escape', code: 'Escape' });
expect(onOpenChange).toHaveBeenCalled();
expect(onOpenChange.mock.lastCall?.[0]).toBe(false);
});
});
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/drawer-select-escape.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/ui': patch
---

Fix pressing `Escape` while a `Select` is open inside a `Drawer` (for example the payment method picker in Checkout) dismissing the entire Drawer. `Escape` now closes only the open `Select` and leaves the Drawer open. The `Select` now wires up its floating interaction props so it handles `Escape` itself, and the `Drawer` roots a floating tree so nested floating elements are recognized as its children.
61 changes: 42 additions & 19 deletions packages/ui/src/elements/Drawer.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,10 +2,14 @@ import { usePortalRoot, useSafeLayoutEffect } from '@clerk/shared/react/index';
import type { UseDismissProps, UseFloatingOptions, UseRoleProps } from '@floating-ui/react';
import {
FloatingFocusManager,
FloatingNode,
FloatingPortal,
FloatingTree,
useClick,
useDismiss,
useFloating,
useFloatingNodeId,
useFloatingParentNodeId,
useInteractions,
useMergeRefs,
useRole,
Expand DownExpand Up@@ -78,7 +82,22 @@ interface RootProps {
dismissProps?: UseDismissProps;
}

function Root({
function Root(props: RootProps) {
// The Drawer must be the root of a FloatingTree so that nested floating
// elements (e.g. a Select) register as its children. Without this, pressing
// Escape to close a nested popover also dismisses the Drawer.
const parentNodeId = useFloatingParentNodeId();
if (parentNodeId == null) {
return (
<FloatingTree>
<RootContent {...props} />
</FloatingTree>
);
}
return <RootContent {...props} />;
}

function RootContent({
children,
open,
onOpenChange,
Expand All@@ -90,10 +109,12 @@ function Root({
const direction = useDirection();
const portalRoot = usePortalRoot();
const effectivePortalRoot = portalProps?.root ?? portalRoot?.() ?? undefined;
const nodeId = useFloatingNodeId();

const { refs, context } = useFloating({
open,
onOpenChange,
nodeId,
transform: false,
strategy,
placement: direction === 'ltr' ? 'right' : 'left',
Expand All@@ -107,25 +128,27 @@ function Root({
]);

return (
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
<FloatingNode id={nodeId}>
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
</FloatingNode>
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/ui/src/elements/Select.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -289,7 +289,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
} = useSelectState();
const { filteredItems: options, searchInputProps } = searchInputCtx;
const [focusedIndex, setFocusedIndex] = useState(0);
const { isOpen, floating, styles, nodeId, context } = popoverCtx;
const { isOpen, floating, styles, nodeId, context, getFloatingProps } = popoverCtx;
const containerRef = React.useRef<HTMLDivElement>(null);
const effectiveListboxId = id ?? generatedListboxId;
const effectiveAriaLabelledBy = ariaLabelledBy ?? (ariaLabel ? undefined : triggerId);
Expand DownExpand Up@@ -361,7 +361,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
elementDescriptor={descriptors.selectOptionsContainer}
elementId={descriptors.selectOptionsContainer.setId(elementId)}
ref={floating}
onKeyDown={onKeyDown}
{...getFloatingProps({ onKeyDown })}
direction='col'
justify='start'
sx={[
Expand Down
72 changes: 72 additions & 0 deletions packages/ui/src/elements/__tests__/Drawer.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
import { ClerkInstanceContext } from '@clerk/shared/react';
import { fireEvent, render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { PropsWithChildren } from 'react';
import { describe, expect, it, vi } from 'vitest';

import { EnvironmentProvider } from '../../contexts';
import { AppearanceProvider } from '../../customizables';
import { InternalThemeProvider } from '../../styledSystem';
import { Drawer } from '../Drawer';
import { Select, SelectButton, SelectOptionList } from '../Select';

const options = [
{ value: 'one', label: 'One' },
{ value: 'two', label: 'Two' },
];

const TestProviders = ({ children }: PropsWithChildren) => (
<ClerkInstanceContext.Provider value={{ value: { client: {}, user: {} } as any }}>
<EnvironmentProvider value={{ displayConfig: { applicationName: 'TestApp' } } as any}>
Comment on lines +19 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== Drawer.test.tsx ==\n'
cat -n packages/ui/src/elements/__tests__/Drawer.test.tsx
printf'\n== Search ClerkInstanceContext / EnvironmentProvider definitions ==\n'
rg -n "ClerkInstanceContext|EnvironmentProvider|displayConfig|applicationName|value: \{\s*client" packages/ui/src packages -g '!**/dist/**' -g '!**/build/**'printf'\n== Type outlines for likely provider files ==\n'
fd -a "ClerkInstanceContext" packages/ui/src packages ||true
fd -a "EnvironmentProvider" packages/ui/src packages ||true

Repository: clerk/javascript

Length of output: 50373


🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== packages/ui/src/contexts/EnvironmentContext.tsx ==\n'
cat -n packages/ui/src/contexts/EnvironmentContext.tsx
printf'\n== packages/ui/src/test/create-fixtures.tsx (relevant slice) ==\n'
sed -n '1,220p' packages/ui/src/test/create-fixtures.tsx
printf'\n== Search for reusable clerk/environment test fixtures in packages/ui ==\n'
rg -n "create-fixtures|environmentMock|ClerkInstanceContext.Provider|EnvironmentProvider value=" packages/ui/src -g '!**/dist/**' -g '!**/build/**'

Repository: clerk/javascript

Length of output: 20221


Use typed fixtures for these providers. Replace the as any casts with a typed Clerk/environment mock so ClerkInstanceContext and EnvironmentProvider shape changes surface here instead of being hidden by the test setup.

🤖 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/ui/src/elements/__tests__/Drawer.test.tsx` around lines 19 - 20,
Replace the as any casts in the ClerkInstanceContext.Provider and
EnvironmentProvider fixtures within Drawer.test.tsx with properly typed Clerk
and environment mocks. Ensure both provider values satisfy their respective
context types while preserving the existing test data and exposing future
provider shape changes to TypeScript.

Source: Coding guidelines

<AppearanceProvider>
<InternalThemeProvider>{children}</InternalThemeProvider>
</AppearanceProvider>
</EnvironmentProvider>
</ClerkInstanceContext.Provider>
);

describe('Drawer', () => {
it('does not close the Drawer when Escape dismisses an open nested Select', async () => {
const user = userEvent.setup({ delay: null });
const onOpenChange = vi.fn();

render(
<Drawer.Root
open
onOpenChange={onOpenChange}
>
<Drawer.Content>
<Select
options={options}
value={null}
onChange={vi.fn()}
portal
>
<SelectButton />
<SelectOptionList />
</Select>
</Drawer.Content>
</Drawer.Root>,
{ wrapper: TestProviders },
);

await user.click(screen.getByRole('button', { name: 'Select an option' }));
const listbox = await screen.findByRole('listbox');

// A real browser moves focus into the open Select; jsdom does not, so focus
// it explicitly before dispatching Escape from within it.
listbox.focus();
fireEvent.keyDown(listbox, { key: 'Escape', code: 'Escape' });

// Escape closes the nested Select but must not bubble up to dismiss the
// Drawer itself.
expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
expect(onOpenChange).not.toHaveBeenCalled();
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// With the Select closed, a second Escape now dismisses the Drawer.
// floating-ui invokes onOpenChange(open, event, reason), so assert on the open arg.
fireEvent.keyDown(document.body, { key: 'Escape', code: 'Escape' });
expect(onOpenChange).toHaveBeenCalled();
expect(onOpenChange.mock.lastCall?.[0]).toBe(false);
});
});
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/drawer-select-escape.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/ui': patch
---

Fix pressing `Escape` while a `Select` is open inside a `Drawer` (for example the payment method picker in Checkout) dismissing the entire Drawer. `Escape` now closes only the open `Select` and leaves the Drawer open. The `Select` now wires up its floating interaction props so it handles `Escape` itself, and the `Drawer` roots a floating tree so nested floating elements are recognized as its children.
61 changes: 42 additions & 19 deletions packages/ui/src/elements/Drawer.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,10 +2,14 @@ import { usePortalRoot, useSafeLayoutEffect } from '@clerk/shared/react/index';
import type { UseDismissProps, UseFloatingOptions, UseRoleProps } from '@floating-ui/react';
import {
FloatingFocusManager,
FloatingNode,
FloatingPortal,
FloatingTree,
useClick,
useDismiss,
useFloating,
useFloatingNodeId,
useFloatingParentNodeId,
useInteractions,
useMergeRefs,
useRole,
Expand DownExpand Up@@ -78,7 +82,22 @@ interface RootProps {
dismissProps?: UseDismissProps;
}

function Root({
function Root(props: RootProps) {
// The Drawer must be the root of a FloatingTree so that nested floating
// elements (e.g. a Select) register as its children. Without this, pressing
// Escape to close a nested popover also dismisses the Drawer.
const parentNodeId = useFloatingParentNodeId();
if (parentNodeId == null) {
return (
<FloatingTree>
<RootContent {...props} />
</FloatingTree>
);
}
return <RootContent {...props} />;
}

function RootContent({
children,
open,
onOpenChange,
Expand All@@ -90,10 +109,12 @@ function Root({
const direction = useDirection();
const portalRoot = usePortalRoot();
const effectivePortalRoot = portalProps?.root ?? portalRoot?.() ?? undefined;
const nodeId = useFloatingNodeId();

const { refs, context } = useFloating({
open,
onOpenChange,
nodeId,
transform: false,
strategy,
placement: direction === 'ltr' ? 'right' : 'left',
Expand All@@ -107,25 +128,27 @@ function Root({
]);

return (
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
<FloatingNode id={nodeId}>
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
</FloatingNode>
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/ui/src/elements/Select.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -289,7 +289,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
} = useSelectState();
const { filteredItems: options, searchInputProps } = searchInputCtx;
const [focusedIndex, setFocusedIndex] = useState(0);
const { isOpen, floating, styles, nodeId, context } = popoverCtx;
const { isOpen, floating, styles, nodeId, context, getFloatingProps } = popoverCtx;
const containerRef = React.useRef<HTMLDivElement>(null);
const effectiveListboxId = id ?? generatedListboxId;
const effectiveAriaLabelledBy = ariaLabelledBy ?? (ariaLabel ? undefined : triggerId);
Expand DownExpand Up@@ -361,7 +361,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
elementDescriptor={descriptors.selectOptionsContainer}
elementId={descriptors.selectOptionsContainer.setId(elementId)}
ref={floating}
onKeyDown={onKeyDown}
{...getFloatingProps({ onKeyDown })}
direction='col'
justify='start'
sx={[
Expand Down
72 changes: 72 additions & 0 deletions packages/ui/src/elements/__tests__/Drawer.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
import { ClerkInstanceContext } from '@clerk/shared/react';
import { fireEvent, render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { PropsWithChildren } from 'react';
import { describe, expect, it, vi } from 'vitest';

import { EnvironmentProvider } from '../../contexts';
import { AppearanceProvider } from '../../customizables';
import { InternalThemeProvider } from '../../styledSystem';
import { Drawer } from '../Drawer';
import { Select, SelectButton, SelectOptionList } from '../Select';

const options = [
{ value: 'one', label: 'One' },
{ value: 'two', label: 'Two' },
];

const TestProviders = ({ children }: PropsWithChildren) => (
<ClerkInstanceContext.Provider value={{ value: { client: {}, user: {} } as any }}>
<EnvironmentProvider value={{ displayConfig: { applicationName: 'TestApp' } } as any}>
Comment on lines +19 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== Drawer.test.tsx ==\n'
cat -n packages/ui/src/elements/__tests__/Drawer.test.tsx
printf'\n== Search ClerkInstanceContext / EnvironmentProvider definitions ==\n'
rg -n "ClerkInstanceContext|EnvironmentProvider|displayConfig|applicationName|value: \{\s*client" packages/ui/src packages -g '!**/dist/**' -g '!**/build/**'printf'\n== Type outlines for likely provider files ==\n'
fd -a "ClerkInstanceContext" packages/ui/src packages ||true
fd -a "EnvironmentProvider" packages/ui/src packages ||true

Repository: clerk/javascript

Length of output: 50373


🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== packages/ui/src/contexts/EnvironmentContext.tsx ==\n'
cat -n packages/ui/src/contexts/EnvironmentContext.tsx
printf'\n== packages/ui/src/test/create-fixtures.tsx (relevant slice) ==\n'
sed -n '1,220p' packages/ui/src/test/create-fixtures.tsx
printf'\n== Search for reusable clerk/environment test fixtures in packages/ui ==\n'
rg -n "create-fixtures|environmentMock|ClerkInstanceContext.Provider|EnvironmentProvider value=" packages/ui/src -g '!**/dist/**' -g '!**/build/**'

Repository: clerk/javascript

Length of output: 20221


Use typed fixtures for these providers. Replace the as any casts with a typed Clerk/environment mock so ClerkInstanceContext and EnvironmentProvider shape changes surface here instead of being hidden by the test setup.

🤖 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/ui/src/elements/__tests__/Drawer.test.tsx` around lines 19 - 20,
Replace the as any casts in the ClerkInstanceContext.Provider and
EnvironmentProvider fixtures within Drawer.test.tsx with properly typed Clerk
and environment mocks. Ensure both provider values satisfy their respective
context types while preserving the existing test data and exposing future
provider shape changes to TypeScript.

Source: Coding guidelines

<AppearanceProvider>
<InternalThemeProvider>{children}</InternalThemeProvider>
</AppearanceProvider>
</EnvironmentProvider>
</ClerkInstanceContext.Provider>
);

describe('Drawer', () => {
it('does not close the Drawer when Escape dismisses an open nested Select', async () => {
const user = userEvent.setup({ delay: null });
const onOpenChange = vi.fn();

render(
<Drawer.Root
open
onOpenChange={onOpenChange}
>
<Drawer.Content>
<Select
options={options}
value={null}
onChange={vi.fn()}
portal
>
<SelectButton />
<SelectOptionList />
</Select>
</Drawer.Content>
</Drawer.Root>,
{ wrapper: TestProviders },
);

await user.click(screen.getByRole('button', { name: 'Select an option' }));
const listbox = await screen.findByRole('listbox');

// A real browser moves focus into the open Select; jsdom does not, so focus
// it explicitly before dispatching Escape from within it.
listbox.focus();
fireEvent.keyDown(listbox, { key: 'Escape', code: 'Escape' });

// Escape closes the nested Select but must not bubble up to dismiss the
// Drawer itself.
expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
expect(onOpenChange).not.toHaveBeenCalled();
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// With the Select closed, a second Escape now dismisses the Drawer.
// floating-ui invokes onOpenChange(open, event, reason), so assert on the open arg.
fireEvent.keyDown(document.body, { key: 'Escape', code: 'Escape' });
expect(onOpenChange).toHaveBeenCalled();
expect(onOpenChange.mock.lastCall?.[0]).toBe(false);
});
});
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/drawer-select-escape.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/ui': patch
---

Fix pressing `Escape` while a `Select` is open inside a `Drawer` (for example the payment method picker in Checkout) dismissing the entire Drawer. `Escape` now closes only the open `Select` and leaves the Drawer open. The `Select` now wires up its floating interaction props so it handles `Escape` itself, and the `Drawer` roots a floating tree so nested floating elements are recognized as its children.
61 changes: 42 additions & 19 deletions packages/ui/src/elements/Drawer.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,10 +2,14 @@ import { usePortalRoot, useSafeLayoutEffect } from '@clerk/shared/react/index';
import type { UseDismissProps, UseFloatingOptions, UseRoleProps } from '@floating-ui/react';
import {
FloatingFocusManager,
FloatingNode,
FloatingPortal,
FloatingTree,
useClick,
useDismiss,
useFloating,
useFloatingNodeId,
useFloatingParentNodeId,
useInteractions,
useMergeRefs,
useRole,
Expand DownExpand Up@@ -78,7 +82,22 @@ interface RootProps {
dismissProps?: UseDismissProps;
}

function Root({
function Root(props: RootProps) {
// The Drawer must be the root of a FloatingTree so that nested floating
// elements (e.g. a Select) register as its children. Without this, pressing
// Escape to close a nested popover also dismisses the Drawer.
const parentNodeId = useFloatingParentNodeId();
if (parentNodeId == null) {
return (
<FloatingTree>
<RootContent {...props} />
</FloatingTree>
);
}
return <RootContent {...props} />;
}

function RootContent({
children,
open,
onOpenChange,
Expand All@@ -90,10 +109,12 @@ function Root({
const direction = useDirection();
const portalRoot = usePortalRoot();
const effectivePortalRoot = portalProps?.root ?? portalRoot?.() ?? undefined;
const nodeId = useFloatingNodeId();

const { refs, context } = useFloating({
open,
onOpenChange,
nodeId,
transform: false,
strategy,
placement: direction === 'ltr' ? 'right' : 'left',
Expand All@@ -107,25 +128,27 @@ function Root({
]);

return (
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
<FloatingNode id={nodeId}>
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
</FloatingNode>
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/ui/src/elements/Select.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -289,7 +289,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
} = useSelectState();
const { filteredItems: options, searchInputProps } = searchInputCtx;
const [focusedIndex, setFocusedIndex] = useState(0);
const { isOpen, floating, styles, nodeId, context } = popoverCtx;
const { isOpen, floating, styles, nodeId, context, getFloatingProps } = popoverCtx;
const containerRef = React.useRef<HTMLDivElement>(null);
const effectiveListboxId = id ?? generatedListboxId;
const effectiveAriaLabelledBy = ariaLabelledBy ?? (ariaLabel ? undefined : triggerId);
Expand DownExpand Up@@ -361,7 +361,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
elementDescriptor={descriptors.selectOptionsContainer}
elementId={descriptors.selectOptionsContainer.setId(elementId)}
ref={floating}
onKeyDown={onKeyDown}
{...getFloatingProps({ onKeyDown })}
direction='col'
justify='start'
sx={[
Expand Down
72 changes: 72 additions & 0 deletions packages/ui/src/elements/__tests__/Drawer.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
import { ClerkInstanceContext } from '@clerk/shared/react';
import { fireEvent, render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { PropsWithChildren } from 'react';
import { describe, expect, it, vi } from 'vitest';

import { EnvironmentProvider } from '../../contexts';
import { AppearanceProvider } from '../../customizables';
import { InternalThemeProvider } from '../../styledSystem';
import { Drawer } from '../Drawer';
import { Select, SelectButton, SelectOptionList } from '../Select';

const options = [
{ value: 'one', label: 'One' },
{ value: 'two', label: 'Two' },
];

const TestProviders = ({ children }: PropsWithChildren) => (
<ClerkInstanceContext.Provider value={{ value: { client: {}, user: {} } as any }}>
<EnvironmentProvider value={{ displayConfig: { applicationName: 'TestApp' } } as any}>
Comment on lines +19 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== Drawer.test.tsx ==\n'
cat -n packages/ui/src/elements/__tests__/Drawer.test.tsx
printf'\n== Search ClerkInstanceContext / EnvironmentProvider definitions ==\n'
rg -n "ClerkInstanceContext|EnvironmentProvider|displayConfig|applicationName|value: \{\s*client" packages/ui/src packages -g '!**/dist/**' -g '!**/build/**'printf'\n== Type outlines for likely provider files ==\n'
fd -a "ClerkInstanceContext" packages/ui/src packages ||true
fd -a "EnvironmentProvider" packages/ui/src packages ||true

Repository: clerk/javascript

Length of output: 50373


🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== packages/ui/src/contexts/EnvironmentContext.tsx ==\n'
cat -n packages/ui/src/contexts/EnvironmentContext.tsx
printf'\n== packages/ui/src/test/create-fixtures.tsx (relevant slice) ==\n'
sed -n '1,220p' packages/ui/src/test/create-fixtures.tsx
printf'\n== Search for reusable clerk/environment test fixtures in packages/ui ==\n'
rg -n "create-fixtures|environmentMock|ClerkInstanceContext.Provider|EnvironmentProvider value=" packages/ui/src -g '!**/dist/**' -g '!**/build/**'

Repository: clerk/javascript

Length of output: 20221


Use typed fixtures for these providers. Replace the as any casts with a typed Clerk/environment mock so ClerkInstanceContext and EnvironmentProvider shape changes surface here instead of being hidden by the test setup.

🤖 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/ui/src/elements/__tests__/Drawer.test.tsx` around lines 19 - 20,
Replace the as any casts in the ClerkInstanceContext.Provider and
EnvironmentProvider fixtures within Drawer.test.tsx with properly typed Clerk
and environment mocks. Ensure both provider values satisfy their respective
context types while preserving the existing test data and exposing future
provider shape changes to TypeScript.

Source: Coding guidelines

<AppearanceProvider>
<InternalThemeProvider>{children}</InternalThemeProvider>
</AppearanceProvider>
</EnvironmentProvider>
</ClerkInstanceContext.Provider>
);

describe('Drawer', () => {
it('does not close the Drawer when Escape dismisses an open nested Select', async () => {
const user = userEvent.setup({ delay: null });
const onOpenChange = vi.fn();

render(
<Drawer.Root
open
onOpenChange={onOpenChange}
>
<Drawer.Content>
<Select
options={options}
value={null}
onChange={vi.fn()}
portal
>
<SelectButton />
<SelectOptionList />
</Select>
</Drawer.Content>
</Drawer.Root>,
{ wrapper: TestProviders },
);

await user.click(screen.getByRole('button', { name: 'Select an option' }));
const listbox = await screen.findByRole('listbox');

// A real browser moves focus into the open Select; jsdom does not, so focus
// it explicitly before dispatching Escape from within it.
listbox.focus();
fireEvent.keyDown(listbox, { key: 'Escape', code: 'Escape' });

// Escape closes the nested Select but must not bubble up to dismiss the
// Drawer itself.
expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
expect(onOpenChange).not.toHaveBeenCalled();
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// With the Select closed, a second Escape now dismisses the Drawer.
// floating-ui invokes onOpenChange(open, event, reason), so assert on the open arg.
fireEvent.keyDown(document.body, { key: 'Escape', code: 'Escape' });
expect(onOpenChange).toHaveBeenCalled();
expect(onOpenChange.mock.lastCall?.[0]).toBe(false);
});
});
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/drawer-select-escape.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/ui': patch
---

Fix pressing `Escape` while a `Select` is open inside a `Drawer` (for example the payment method picker in Checkout) dismissing the entire Drawer. `Escape` now closes only the open `Select` and leaves the Drawer open. The `Select` now wires up its floating interaction props so it handles `Escape` itself, and the `Drawer` roots a floating tree so nested floating elements are recognized as its children.
61 changes: 42 additions & 19 deletions packages/ui/src/elements/Drawer.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,10 +2,14 @@ import { usePortalRoot, useSafeLayoutEffect } from '@clerk/shared/react/index';
import type { UseDismissProps, UseFloatingOptions, UseRoleProps } from '@floating-ui/react';
import {
FloatingFocusManager,
FloatingNode,
FloatingPortal,
FloatingTree,
useClick,
useDismiss,
useFloating,
useFloatingNodeId,
useFloatingParentNodeId,
useInteractions,
useMergeRefs,
useRole,
Expand DownExpand Up@@ -78,7 +82,22 @@ interface RootProps {
dismissProps?: UseDismissProps;
}

function Root({
function Root(props: RootProps) {
// The Drawer must be the root of a FloatingTree so that nested floating
// elements (e.g. a Select) register as its children. Without this, pressing
// Escape to close a nested popover also dismisses the Drawer.
const parentNodeId = useFloatingParentNodeId();
if (parentNodeId == null) {
return (
<FloatingTree>
<RootContent {...props} />
</FloatingTree>
);
}
return <RootContent {...props} />;
}

function RootContent({
children,
open,
onOpenChange,
Expand All@@ -90,10 +109,12 @@ function Root({
const direction = useDirection();
const portalRoot = usePortalRoot();
const effectivePortalRoot = portalProps?.root ?? portalRoot?.() ?? undefined;
const nodeId = useFloatingNodeId();

const { refs, context } = useFloating({
open,
onOpenChange,
nodeId,
transform: false,
strategy,
placement: direction === 'ltr' ? 'right' : 'left',
Expand All@@ -107,25 +128,27 @@ function Root({
]);

return (
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
<FloatingNode id={nodeId}>
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
</FloatingNode>
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/ui/src/elements/Select.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -289,7 +289,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
} = useSelectState();
const { filteredItems: options, searchInputProps } = searchInputCtx;
const [focusedIndex, setFocusedIndex] = useState(0);
const { isOpen, floating, styles, nodeId, context } = popoverCtx;
const { isOpen, floating, styles, nodeId, context, getFloatingProps } = popoverCtx;
const containerRef = React.useRef<HTMLDivElement>(null);
const effectiveListboxId = id ?? generatedListboxId;
const effectiveAriaLabelledBy = ariaLabelledBy ?? (ariaLabel ? undefined : triggerId);
Expand DownExpand Up@@ -361,7 +361,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
elementDescriptor={descriptors.selectOptionsContainer}
elementId={descriptors.selectOptionsContainer.setId(elementId)}
ref={floating}
onKeyDown={onKeyDown}
{...getFloatingProps({ onKeyDown })}
direction='col'
justify='start'
sx={[
Expand Down
72 changes: 72 additions & 0 deletions packages/ui/src/elements/__tests__/Drawer.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
import { ClerkInstanceContext } from '@clerk/shared/react';
import { fireEvent, render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { PropsWithChildren } from 'react';
import { describe, expect, it, vi } from 'vitest';

import { EnvironmentProvider } from '../../contexts';
import { AppearanceProvider } from '../../customizables';
import { InternalThemeProvider } from '../../styledSystem';
import { Drawer } from '../Drawer';
import { Select, SelectButton, SelectOptionList } from '../Select';

const options = [
{ value: 'one', label: 'One' },
{ value: 'two', label: 'Two' },
];

const TestProviders = ({ children }: PropsWithChildren) => (
<ClerkInstanceContext.Provider value={{ value: { client: {}, user: {} } as any }}>
<EnvironmentProvider value={{ displayConfig: { applicationName: 'TestApp' } } as any}>
Comment on lines +19 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== Drawer.test.tsx ==\n'
cat -n packages/ui/src/elements/__tests__/Drawer.test.tsx
printf'\n== Search ClerkInstanceContext / EnvironmentProvider definitions ==\n'
rg -n "ClerkInstanceContext|EnvironmentProvider|displayConfig|applicationName|value: \{\s*client" packages/ui/src packages -g '!**/dist/**' -g '!**/build/**'printf'\n== Type outlines for likely provider files ==\n'
fd -a "ClerkInstanceContext" packages/ui/src packages ||true
fd -a "EnvironmentProvider" packages/ui/src packages ||true

Repository: clerk/javascript

Length of output: 50373


🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== packages/ui/src/contexts/EnvironmentContext.tsx ==\n'
cat -n packages/ui/src/contexts/EnvironmentContext.tsx
printf'\n== packages/ui/src/test/create-fixtures.tsx (relevant slice) ==\n'
sed -n '1,220p' packages/ui/src/test/create-fixtures.tsx
printf'\n== Search for reusable clerk/environment test fixtures in packages/ui ==\n'
rg -n "create-fixtures|environmentMock|ClerkInstanceContext.Provider|EnvironmentProvider value=" packages/ui/src -g '!**/dist/**' -g '!**/build/**'

Repository: clerk/javascript

Length of output: 20221


Use typed fixtures for these providers. Replace the as any casts with a typed Clerk/environment mock so ClerkInstanceContext and EnvironmentProvider shape changes surface here instead of being hidden by the test setup.

🤖 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/ui/src/elements/__tests__/Drawer.test.tsx` around lines 19 - 20,
Replace the as any casts in the ClerkInstanceContext.Provider and
EnvironmentProvider fixtures within Drawer.test.tsx with properly typed Clerk
and environment mocks. Ensure both provider values satisfy their respective
context types while preserving the existing test data and exposing future
provider shape changes to TypeScript.

Source: Coding guidelines

<AppearanceProvider>
<InternalThemeProvider>{children}</InternalThemeProvider>
</AppearanceProvider>
</EnvironmentProvider>
</ClerkInstanceContext.Provider>
);

describe('Drawer', () => {
it('does not close the Drawer when Escape dismisses an open nested Select', async () => {
const user = userEvent.setup({ delay: null });
const onOpenChange = vi.fn();

render(
<Drawer.Root
open
onOpenChange={onOpenChange}
>
<Drawer.Content>
<Select
options={options}
value={null}
onChange={vi.fn()}
portal
>
<SelectButton />
<SelectOptionList />
</Select>
</Drawer.Content>
</Drawer.Root>,
{ wrapper: TestProviders },
);

await user.click(screen.getByRole('button', { name: 'Select an option' }));
const listbox = await screen.findByRole('listbox');

// A real browser moves focus into the open Select; jsdom does not, so focus
// it explicitly before dispatching Escape from within it.
listbox.focus();
fireEvent.keyDown(listbox, { key: 'Escape', code: 'Escape' });

// Escape closes the nested Select but must not bubble up to dismiss the
// Drawer itself.
expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
expect(onOpenChange).not.toHaveBeenCalled();
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// With the Select closed, a second Escape now dismisses the Drawer.
// floating-ui invokes onOpenChange(open, event, reason), so assert on the open arg.
fireEvent.keyDown(document.body, { key: 'Escape', code: 'Escape' });
expect(onOpenChange).toHaveBeenCalled();
expect(onOpenChange.mock.lastCall?.[0]).toBe(false);
});
});
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/drawer-select-escape.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/ui': patch
---

Fix pressing `Escape` while a `Select` is open inside a `Drawer` (for example the payment method picker in Checkout) dismissing the entire Drawer. `Escape` now closes only the open `Select` and leaves the Drawer open. The `Select` now wires up its floating interaction props so it handles `Escape` itself, and the `Drawer` roots a floating tree so nested floating elements are recognized as its children.
61 changes: 42 additions & 19 deletions packages/ui/src/elements/Drawer.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,10 +2,14 @@ import { usePortalRoot, useSafeLayoutEffect } from '@clerk/shared/react/index';
import type { UseDismissProps, UseFloatingOptions, UseRoleProps } from '@floating-ui/react';
import {
FloatingFocusManager,
FloatingNode,
FloatingPortal,
FloatingTree,
useClick,
useDismiss,
useFloating,
useFloatingNodeId,
useFloatingParentNodeId,
useInteractions,
useMergeRefs,
useRole,
Expand DownExpand Up@@ -78,7 +82,22 @@ interface RootProps {
dismissProps?: UseDismissProps;
}

function Root({
function Root(props: RootProps) {
// The Drawer must be the root of a FloatingTree so that nested floating
// elements (e.g. a Select) register as its children. Without this, pressing
// Escape to close a nested popover also dismisses the Drawer.
const parentNodeId = useFloatingParentNodeId();
if (parentNodeId == null) {
return (
<FloatingTree>
<RootContent {...props} />
</FloatingTree>
);
}
return <RootContent {...props} />;
}

function RootContent({
children,
open,
onOpenChange,
Expand All@@ -90,10 +109,12 @@ function Root({
const direction = useDirection();
const portalRoot = usePortalRoot();
const effectivePortalRoot = portalProps?.root ?? portalRoot?.() ?? undefined;
const nodeId = useFloatingNodeId();

const { refs, context } = useFloating({
open,
onOpenChange,
nodeId,
transform: false,
strategy,
placement: direction === 'ltr' ? 'right' : 'left',
Expand All@@ -107,25 +128,27 @@ function Root({
]);

return (
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
<FloatingNode id={nodeId}>
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
</FloatingNode>
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/ui/src/elements/Select.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -289,7 +289,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
} = useSelectState();
const { filteredItems: options, searchInputProps } = searchInputCtx;
const [focusedIndex, setFocusedIndex] = useState(0);
const { isOpen, floating, styles, nodeId, context } = popoverCtx;
const { isOpen, floating, styles, nodeId, context, getFloatingProps } = popoverCtx;
const containerRef = React.useRef<HTMLDivElement>(null);
const effectiveListboxId = id ?? generatedListboxId;
const effectiveAriaLabelledBy = ariaLabelledBy ?? (ariaLabel ? undefined : triggerId);
Expand DownExpand Up@@ -361,7 +361,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
elementDescriptor={descriptors.selectOptionsContainer}
elementId={descriptors.selectOptionsContainer.setId(elementId)}
ref={floating}
onKeyDown={onKeyDown}
{...getFloatingProps({ onKeyDown })}
direction='col'
justify='start'
sx={[
Expand Down
72 changes: 72 additions & 0 deletions packages/ui/src/elements/__tests__/Drawer.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
import { ClerkInstanceContext } from '@clerk/shared/react';
import { fireEvent, render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { PropsWithChildren } from 'react';
import { describe, expect, it, vi } from 'vitest';

import { EnvironmentProvider } from '../../contexts';
import { AppearanceProvider } from '../../customizables';
import { InternalThemeProvider } from '../../styledSystem';
import { Drawer } from '../Drawer';
import { Select, SelectButton, SelectOptionList } from '../Select';

const options = [
{ value: 'one', label: 'One' },
{ value: 'two', label: 'Two' },
];

const TestProviders = ({ children }: PropsWithChildren) => (
<ClerkInstanceContext.Provider value={{ value: { client: {}, user: {} } as any }}>
<EnvironmentProvider value={{ displayConfig: { applicationName: 'TestApp' } } as any}>
Comment on lines +19 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== Drawer.test.tsx ==\n'
cat -n packages/ui/src/elements/__tests__/Drawer.test.tsx
printf'\n== Search ClerkInstanceContext / EnvironmentProvider definitions ==\n'
rg -n "ClerkInstanceContext|EnvironmentProvider|displayConfig|applicationName|value: \{\s*client" packages/ui/src packages -g '!**/dist/**' -g '!**/build/**'printf'\n== Type outlines for likely provider files ==\n'
fd -a "ClerkInstanceContext" packages/ui/src packages ||true
fd -a "EnvironmentProvider" packages/ui/src packages ||true

Repository: clerk/javascript

Length of output: 50373


🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== packages/ui/src/contexts/EnvironmentContext.tsx ==\n'
cat -n packages/ui/src/contexts/EnvironmentContext.tsx
printf'\n== packages/ui/src/test/create-fixtures.tsx (relevant slice) ==\n'
sed -n '1,220p' packages/ui/src/test/create-fixtures.tsx
printf'\n== Search for reusable clerk/environment test fixtures in packages/ui ==\n'
rg -n "create-fixtures|environmentMock|ClerkInstanceContext.Provider|EnvironmentProvider value=" packages/ui/src -g '!**/dist/**' -g '!**/build/**'

Repository: clerk/javascript

Length of output: 20221


Use typed fixtures for these providers. Replace the as any casts with a typed Clerk/environment mock so ClerkInstanceContext and EnvironmentProvider shape changes surface here instead of being hidden by the test setup.

🤖 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/ui/src/elements/__tests__/Drawer.test.tsx` around lines 19 - 20,
Replace the as any casts in the ClerkInstanceContext.Provider and
EnvironmentProvider fixtures within Drawer.test.tsx with properly typed Clerk
and environment mocks. Ensure both provider values satisfy their respective
context types while preserving the existing test data and exposing future
provider shape changes to TypeScript.

Source: Coding guidelines

<AppearanceProvider>
<InternalThemeProvider>{children}</InternalThemeProvider>
</AppearanceProvider>
</EnvironmentProvider>
</ClerkInstanceContext.Provider>
);

describe('Drawer', () => {
it('does not close the Drawer when Escape dismisses an open nested Select', async () => {
const user = userEvent.setup({ delay: null });
const onOpenChange = vi.fn();

render(
<Drawer.Root
open
onOpenChange={onOpenChange}
>
<Drawer.Content>
<Select
options={options}
value={null}
onChange={vi.fn()}
portal
>
<SelectButton />
<SelectOptionList />
</Select>
</Drawer.Content>
</Drawer.Root>,
{ wrapper: TestProviders },
);

await user.click(screen.getByRole('button', { name: 'Select an option' }));
const listbox = await screen.findByRole('listbox');

// A real browser moves focus into the open Select; jsdom does not, so focus
// it explicitly before dispatching Escape from within it.
listbox.focus();
fireEvent.keyDown(listbox, { key: 'Escape', code: 'Escape' });

// Escape closes the nested Select but must not bubble up to dismiss the
// Drawer itself.
expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
expect(onOpenChange).not.toHaveBeenCalled();
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// With the Select closed, a second Escape now dismisses the Drawer.
// floating-ui invokes onOpenChange(open, event, reason), so assert on the open arg.
fireEvent.keyDown(document.body, { key: 'Escape', code: 'Escape' });
expect(onOpenChange).toHaveBeenCalled();
expect(onOpenChange.mock.lastCall?.[0]).toBe(false);
});
});
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/drawer-select-escape.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/ui': patch
---

Fix pressing `Escape` while a `Select` is open inside a `Drawer` (for example the payment method picker in Checkout) dismissing the entire Drawer. `Escape` now closes only the open `Select` and leaves the Drawer open. The `Select` now wires up its floating interaction props so it handles `Escape` itself, and the `Drawer` roots a floating tree so nested floating elements are recognized as its children.
61 changes: 42 additions & 19 deletions packages/ui/src/elements/Drawer.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,10 +2,14 @@ import { usePortalRoot, useSafeLayoutEffect } from '@clerk/shared/react/index';
import type { UseDismissProps, UseFloatingOptions, UseRoleProps } from '@floating-ui/react';
import {
FloatingFocusManager,
FloatingNode,
FloatingPortal,
FloatingTree,
useClick,
useDismiss,
useFloating,
useFloatingNodeId,
useFloatingParentNodeId,
useInteractions,
useMergeRefs,
useRole,
Expand DownExpand Up@@ -78,7 +82,22 @@ interface RootProps {
dismissProps?: UseDismissProps;
}

function Root({
function Root(props: RootProps) {
// The Drawer must be the root of a FloatingTree so that nested floating
// elements (e.g. a Select) register as its children. Without this, pressing
// Escape to close a nested popover also dismisses the Drawer.
const parentNodeId = useFloatingParentNodeId();
if (parentNodeId == null) {
return (
<FloatingTree>
<RootContent {...props} />
</FloatingTree>
);
}
return <RootContent {...props} />;
}

function RootContent({
children,
open,
onOpenChange,
Expand All@@ -90,10 +109,12 @@ function Root({
const direction = useDirection();
const portalRoot = usePortalRoot();
const effectivePortalRoot = portalProps?.root ?? portalRoot?.() ?? undefined;
const nodeId = useFloatingNodeId();

const { refs, context } = useFloating({
open,
onOpenChange,
nodeId,
transform: false,
strategy,
placement: direction === 'ltr' ? 'right' : 'left',
Expand All@@ -107,25 +128,27 @@ function Root({
]);

return (
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
<FloatingNode id={nodeId}>
<DrawerContext.Provider
value={{
isOpen: open,
setIsOpen: onOpenChange,
strategy,
portalProps: { ...portalProps, root: effectivePortalRoot },
refs,
context,
getFloatingProps,
direction,
}}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
<FloatingPortal
{...portalProps}
root={effectivePortalRoot}
>
{children}
</FloatingPortal>
</DrawerContext.Provider>
</FloatingNode>
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/ui/src/elements/Select.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -289,7 +289,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
} = useSelectState();
const { filteredItems: options, searchInputProps } = searchInputCtx;
const [focusedIndex, setFocusedIndex] = useState(0);
const { isOpen, floating, styles, nodeId, context } = popoverCtx;
const { isOpen, floating, styles, nodeId, context, getFloatingProps } = popoverCtx;
const containerRef = React.useRef<HTMLDivElement>(null);
const effectiveListboxId = id ?? generatedListboxId;
const effectiveAriaLabelledBy = ariaLabelledBy ?? (ariaLabel ? undefined : triggerId);
Expand DownExpand Up@@ -361,7 +361,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
elementDescriptor={descriptors.selectOptionsContainer}
elementId={descriptors.selectOptionsContainer.setId(elementId)}
ref={floating}
onKeyDown={onKeyDown}
{...getFloatingProps({ onKeyDown })}
direction='col'
justify='start'
sx={[
Expand Down
72 changes: 72 additions & 0 deletions packages/ui/src/elements/__tests__/Drawer.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
import { ClerkInstanceContext } from '@clerk/shared/react';
import { fireEvent, render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { PropsWithChildren } from 'react';
import { describe, expect, it, vi } from 'vitest';

import { EnvironmentProvider } from '../../contexts';
import { AppearanceProvider } from '../../customizables';
import { InternalThemeProvider } from '../../styledSystem';
import { Drawer } from '../Drawer';
import { Select, SelectButton, SelectOptionList } from '../Select';

const options = [
{ value: 'one', label: 'One' },
{ value: 'two', label: 'Two' },
];

const TestProviders = ({ children }: PropsWithChildren) => (
<ClerkInstanceContext.Provider value={{ value: { client: {}, user: {} } as any }}>
<EnvironmentProvider value={{ displayConfig: { applicationName: 'TestApp' } } as any}>
Comment on lines +19 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== Drawer.test.tsx ==\n'
cat -n packages/ui/src/elements/__tests__/Drawer.test.tsx
printf'\n== Search ClerkInstanceContext / EnvironmentProvider definitions ==\n'
rg -n "ClerkInstanceContext|EnvironmentProvider|displayConfig|applicationName|value: \{\s*client" packages/ui/src packages -g '!**/dist/**' -g '!**/build/**'printf'\n== Type outlines for likely provider files ==\n'
fd -a "ClerkInstanceContext" packages/ui/src packages ||true
fd -a "EnvironmentProvider" packages/ui/src packages ||true

Repository: clerk/javascript

Length of output: 50373


🏁 Script executed:

#!/bin/bashset -euo pipefail
printf'\n== packages/ui/src/contexts/EnvironmentContext.tsx ==\n'
cat -n packages/ui/src/contexts/EnvironmentContext.tsx
printf'\n== packages/ui/src/test/create-fixtures.tsx (relevant slice) ==\n'
sed -n '1,220p' packages/ui/src/test/create-fixtures.tsx
printf'\n== Search for reusable clerk/environment test fixtures in packages/ui ==\n'
rg -n "create-fixtures|environmentMock|ClerkInstanceContext.Provider|EnvironmentProvider value=" packages/ui/src -g '!**/dist/**' -g '!**/build/**'

Repository: clerk/javascript

Length of output: 20221


Use typed fixtures for these providers. Replace the as any casts with a typed Clerk/environment mock so ClerkInstanceContext and EnvironmentProvider shape changes surface here instead of being hidden by the test setup.

🤖 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/ui/src/elements/__tests__/Drawer.test.tsx` around lines 19 - 20,
Replace the as any casts in the ClerkInstanceContext.Provider and
EnvironmentProvider fixtures within Drawer.test.tsx with properly typed Clerk
and environment mocks. Ensure both provider values satisfy their respective
context types while preserving the existing test data and exposing future
provider shape changes to TypeScript.

Source: Coding guidelines

<AppearanceProvider>
<InternalThemeProvider>{children}</InternalThemeProvider>
</AppearanceProvider>
</EnvironmentProvider>
</ClerkInstanceContext.Provider>
);

describe('Drawer', () => {
it('does not close the Drawer when Escape dismisses an open nested Select', async () => {
const user = userEvent.setup({ delay: null });
const onOpenChange = vi.fn();

render(
<Drawer.Root
open
onOpenChange={onOpenChange}
>
<Drawer.Content>
<Select
options={options}
value={null}
onChange={vi.fn()}
portal
>
<SelectButton />
<SelectOptionList />
</Select>
</Drawer.Content>
</Drawer.Root>,
{ wrapper: TestProviders },
);

await user.click(screen.getByRole('button', { name: 'Select an option' }));
const listbox = await screen.findByRole('listbox');

// A real browser moves focus into the open Select; jsdom does not, so focus
// it explicitly before dispatching Escape from within it.
listbox.focus();
fireEvent.keyDown(listbox, { key: 'Escape', code: 'Escape' });

// Escape closes the nested Select but must not bubble up to dismiss the
// Drawer itself.
expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
expect(onOpenChange).not.toHaveBeenCalled();
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// With the Select closed, a second Escape now dismisses the Drawer.
// floating-ui invokes onOpenChange(open, event, reason), so assert on the open arg.
fireEvent.keyDown(document.body, { key: 'Escape', code: 'Escape' });
expect(onOpenChange).toHaveBeenCalled();
expect(onOpenChange.mock.lastCall?.[0]).toBe(false);
});
});
Loading