Skip to content
Open
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
1 change: 1 addition & 0 deletions assets/index.less
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
z-index: @zIndex;
// overflow: hidden;
transition: transform @duration;
pointer-events: auto;

&-hidden {
display: none;
Expand Down
2 changes: 2 additions & 0 deletions docs/examples/base.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ const Demo = () => {
{...motionProps}
>
content
<button>Button 1</button>
<button>Button 2</button>
</Drawer>
<div>
<button onClick={onSwitch}>打开</button>
Expand Down
1 change: 1 addition & 0 deletions docs/examples/motion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const maskMotion: DrawerProps['maskMotion'] = {
export const motion: DrawerProps['motion'] = placement => ({
motionAppear: true,
motionName: `panel-motion-${placement}`,
motionDeadline: 1000,
});

const motionProps: Partial<DrawerProps> = {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
},
"dependencies": {
"@rc-component/motion": "^1.1.4",
"@rc-component/portal": "^2.0.0",
"@rc-component/portal": "^2.1.3",
"@rc-component/util": "^1.2.1",
"clsx": "^2.1.1"
},
Expand Down
3 changes: 3 additions & 0 deletions src/Drawer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export interface DrawerProps
onResizeStart?: () => void;
onResizeEnd?: () => void;
};
focusTriggerAfterClose?: boolean;
}

const Drawer: React.FC<DrawerProps> = props => {
Expand Down Expand Up @@ -77,6 +78,7 @@ const Drawer: React.FC<DrawerProps> = props => {
onClose,
resizable,
defaultSize,
focusTriggerAfterClose,

// Refs
panelRef,
Expand Down Expand Up @@ -116,6 +118,7 @@ const Drawer: React.FC<DrawerProps> = props => {

if (
!nextVisible &&
focusTriggerAfterClose !== false &&
lastActiveRef.current &&
!popupRef.current?.contains(lastActiveRef.current)
) {
Expand Down
69 changes: 11 additions & 58 deletions src/DrawerPopup.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { clsx } from 'clsx';
import type { CSSMotionProps } from '@rc-component/motion';
import CSSMotion from '@rc-component/motion';
import KeyCode from '@rc-component/util/lib/KeyCode';
import pickAttrs from '@rc-component/util/lib/pickAttrs';
import * as React from 'react';
import type { DrawerContextProps } from './context';
Expand All @@ -15,14 +14,7 @@ import useDrag from './hooks/useDrag';
import { parseWidthHeight } from './util';
import type { DrawerClassNames, DrawerStyles } from './inter';
import { useEvent } from '@rc-component/util';

const sentinelStyle: React.CSSProperties = {
width: 0,
height: 0,
overflow: 'hidden',
outline: 'none',
position: 'absolute',
};
import useFocusable from './hooks/useFocusable';

export type Placement = 'left' | 'right' | 'top' | 'bottom';

Expand All @@ -37,9 +29,12 @@ export interface DrawerPopupProps
inline?: boolean;
push?: boolean | PushConfig;
forceRender?: boolean;
autoFocus?: boolean;
keyboard?: boolean;

// Focus
autoFocus?: boolean;
focusTrap?: boolean;

// Root
rootClassName?: string;
rootStyle?: React.CSSProperties;
Expand Down Expand Up @@ -104,7 +99,10 @@ const DrawerPopup: React.ForwardRefRenderFunction<
inline,
push,
forceRender,

// Focus
autoFocus,
focusTrap,

// classNames
classNames: drawerClassNames,
Expand Down Expand Up @@ -149,39 +147,11 @@ const DrawerPopup: React.ForwardRefRenderFunction<

// ================================ Refs ================================
const panelRef = React.useRef<HTMLDivElement>(null);
const sentinelStartRef = React.useRef<HTMLDivElement>(null);
const sentinelEndRef = React.useRef<HTMLDivElement>(null);

React.useImperativeHandle(ref, () => panelRef.current);

const onPanelKeyDown: React.KeyboardEventHandler<HTMLDivElement> = event => {
const { keyCode, shiftKey } = event;

switch (keyCode) {
// Tab active
case KeyCode.TAB: {
if (keyCode === KeyCode.TAB) {
if (!shiftKey && document.activeElement === sentinelEndRef.current) {
sentinelStartRef.current?.focus({ preventScroll: true });
} else if (
shiftKey &&
document.activeElement === sentinelStartRef.current
) {
sentinelEndRef.current?.focus({ preventScroll: true });
}
}
break;
}
}
};

// ========================== Control ===========================
// Auto Focus
React.useEffect(() => {
if (open && autoFocus) {
panelRef.current?.focus({ preventScroll: true });
}
}, [open]);
// ========================= Focusable ==========================
useFocusable(() => panelRef.current, open, autoFocus, focusTrap, mask);
Comment on lines +153 to +154
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's check the DrawerPopup.tsx file around the mentioned lines
cat -n src/DrawerPopup.tsx | sed -n '150,160p'

Repository: react-component/drawer

Length of output: 525


🏁 Script executed:

# Check line 374 where panelRef is assigned
cat -n src/DrawerPopup.tsx | sed -n '370,380p'

Repository: react-component/drawer

Length of output: 368


🏁 Script executed:

# Now examine the useFocusable hook
if [ -f "src/hooks/useFocusable.ts" ]; then
  cat -n src/hooks/useFocusable.ts
else
  echo "Checking for useFocusable location..."
  find . -name "*useFocusable*" -type f
fi

Repository: react-component/drawer

Length of output: 754


🏁 Script executed:

# Also search for the DrawerPanel component to understand structure
rg -n "DrawerPanel" src/DrawerPopup.tsx | head -20

Repository: react-component/drawer

Length of output: 305


🏁 Script executed:

# Let's examine DrawerPanel to understand the component structure
fd "DrawerPanel.tsx" -x cat -n {} | head -100

Repository: react-component/drawer

Length of output: 2210


🏁 Script executed:

# Search for other usages of useFocusable to understand the usage pattern
rg -n "useFocusable" --type=ts --type=tsx

Repository: react-component/drawer

Length of output: 93


🏁 Script executed:

# Check if there are any tests that show the expected behavior
fd ".*\.(test|spec)\.(ts|tsx)$" -x grep -l "useFocusable\|focus" {} \;

Repository: react-component/drawer

Length of output: 48


🏁 Script executed:

# Correct the rg search for useFocusable
rg "useFocusable" -A 3 -B 1

Repository: react-component/drawer

Length of output: 938


🏁 Script executed:

# Check for test files
find . -name "*.test.ts" -o -name "*.spec.ts" -o -name "*.test.tsx" -o -name "*.spec.tsx" | head -10

Repository: react-component/drawer

Length of output: 154


🏁 Script executed:

# Look at RefContext to understand the panel ref setup
rg "RefContext" -B 5 -A 10

Repository: react-component/drawer

Length of output: 3740


🏁 Script executed:

# Let's look at the Drawer.tsx to see what refContext contains
cat -n src/Drawer.tsx | grep -A 20 "refContext"

Repository: react-component/drawer

Length of output: 1289


🏁 Script executed:

# Check if panelRef in DrawerPopup is the same as panel ref in RefContext
cat -n src/DrawerPopup.tsx | grep -B 10 "new RefContext\|refContext"

Repository: react-component/drawer

Length of output: 48


🏁 Script executed:

# Look at the entire DrawerPopup.tsx to understand the full structure
wc -l src/DrawerPopup.tsx

Repository: react-component/drawer

Length of output: 89


🏁 Script executed:

# Search around where refContext is created in DrawerPopup
rg "refContext" src/DrawerPopup.tsx -B 5 -A 5

Repository: react-component/drawer

Length of output: 48


🏁 Script executed:

# Let's look at the full DrawerPopup to understand the panelRef assignment and structure better
cat -n src/DrawerPopup.tsx | sed -n '120,160p'

Repository: react-component/drawer

Length of output: 1226


🏁 Script executed:

# Let's also check what panelNode is
cat -n src/DrawerPopup.tsx | sed -n '300,340p'

Repository: react-component/drawer

Length of output: 1539


🏁 Script executed:

# Let's look at the entire focus setup in DrawerPopup
cat -n src/DrawerPopup.tsx | sed -n '100,135p'

Repository: react-component/drawer

Length of output: 778


🏁 Script executed:

# Let's check if there's any documentation or comments about the focus behavior
grep -n "focus" src/DrawerPopup.tsx -B 2 -A 2

Repository: react-component/drawer

Length of output: 468


🏁 Script executed:

# Check the context.ts file to understand RefContext better
cat -n src/context.ts

Repository: react-component/drawer

Length of output: 580


🏁 Script executed:

# Let's verify by examining how panelRef is used throughout DrawerPopup
rg "panelRef" src/DrawerPopup.tsx -B 2 -A 2

Repository: react-component/drawer

Length of output: 574


🏁 Script executed:

# Check if DrawerPanel has any focus-related attributes or handling
rg "focus|tabIndex" src/DrawerPanel.tsx -B 2 -A 2

Repository: react-component/drawer

Length of output: 126


🏁 Script executed:

# Let's check the useLockFocus hook to understand the complete focus behavior
rg "useLockFocus" -B 5 -A 15

Repository: react-component/drawer

Length of output: 1172


🏁 Script executed:

# Let's look for any related issues or comments about focus in the codebase
rg "focus|Focus" src/DrawerPopup.tsx | head -20

Repository: react-component/drawer

Length of output: 356


🏁 Script executed:

# Let's check the complete context.ts to see if there's a separate panel ref
cat -n src/context.ts | tail -20

Repository: react-component/drawer

Length of output: 580


🏁 Script executed:

# Let's look at test files to understand the expected focus behavior
cat -n tests/index.spec.tsx | grep -A 10 -B 2 "focus\|autoFocus" | head -40

Repository: react-component/drawer

Length of output: 480


🏁 Script executed:

# Let's check if there are any other drawer implementations or similar components for reference
find . -name "*.tsx" -o -name "*.ts" | xargs grep -l "useLockFocus\|useFocusable" | head -10

Repository: react-component/drawer

Length of output: 115


🏁 Script executed:

# Let's trace through the complete flow - check what happens in the outer container render
cat -n src/DrawerPopup.tsx | sed -n '360,390p'

Repository: react-component/drawer

Length of output: 936


🏁 Script executed:

# Let's verify one more time by looking at similar focus hook patterns in rc-component libraries
rg "useEffect.*\[open\]" -B 3 -A 3 --type=ts --type=tsx

Repository: react-component/drawer

Length of output: 93


useEffect 缺失 autoFocus 依赖项,导致闭包问题。

useFocusable 钩子(src/hooks/useFocusable.ts 第 17-21 行)的 useEffect 使用了 autoFocus 参数,但依赖数组仅包含 [open]。当抽屉打开状态下 autoFocus 属性发生变化时,副作用不会重新执行,导致焦点行为与预期不符。

useEffect 的依赖数组中添加 autoFocus

  React.useEffect(() => {
    if (open && autoFocus === true) {
      getContainer()?.focus({ preventScroll: true });
    }
- }, [open]);
+ }, [open, autoFocus]);

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/hooks/useFocusable.ts around lines 17 to 21 (and called from
src/DrawerPopup.tsx lines 153-154), the useEffect relies on the autoFocus
parameter but its dependency array only contains [open], causing a stale-closure
when autoFocus changes; update the effect's dependency array to include
autoFocus (and any other used props/refs) so the effect re-runs when autoFocus
changes, ensuring correct focus behavior when the drawer is open.


// ============================ Push ============================
const [pushed, setPushed] = React.useState(false);
Expand Down Expand Up @@ -345,9 +315,7 @@ const DrawerPopup: React.ForwardRefRenderFunction<
{...motionProps}
visible={open}
forceRender={forceRender}
onVisibleChanged={nextVisible => {
afterOpenChange?.(nextVisible);
}}
onVisibleChanged={afterOpenChange}
removeOnLeave={false}
leavedClassName={`${prefixCls}-content-wrapper-hidden`}
>
Expand Down Expand Up @@ -404,24 +372,9 @@ const DrawerPopup: React.ForwardRefRenderFunction<
style={containerStyle}
tabIndex={-1}
ref={panelRef}
onKeyDown={onPanelKeyDown}
>
{maskNode}
<div
tabIndex={0}
ref={sentinelStartRef}
style={sentinelStyle}
aria-hidden="true"
data-sentinel="start"
/>
{panelNode}
<div
tabIndex={0}
ref={sentinelEndRef}
style={sentinelStyle}
aria-hidden="true"
data-sentinel="end"
/>
</div>
</DrawerContext.Provider>
);
Expand Down
22 changes: 22 additions & 0 deletions src/hooks/useFocusable.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import React from 'react';
import { useLockFocus } from '@rc-component/util/lib/Dom/focus';

export default function useFocusable(
getContainer: () => HTMLElement,
open: boolean,
autoFocus?: boolean,
focusTrap?: boolean,
mask?: boolean,
) {
const mergedFocusTrap = focusTrap ?? mask !== false;

// Focus lock
useLockFocus(open && mergedFocusTrap, getContainer);

// Auto Focus
React.useEffect(() => {
if (open && autoFocus === true) {
getContainer()?.focus({ preventScroll: true });
}
}, [open]);
Comment on lines +17 to +21
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

useEffect 依赖数组不完整。

第 17-21 行的 useEffect 依赖数组只包含 [open],但实际上还依赖于 autoFocusgetContainer() 的返回值。

autoFocusfalse 变为 true 时,或者 getContainer() 返回不同的元素时,effect 不会重新执行,可能导致焦点行为不符合预期。

建议的修复方案
  React.useEffect(() => {
    if (open && autoFocus === true) {
      getContainer()?.focus({ preventScroll: true });
    }
-  }, [open]);
+  }, [open, autoFocus]);

注意:getContainer 函数本身应该保持稳定(通过 useCallback 或其他方式),因此不需要添加到依赖数组中。如果 getContainer 不稳定,建议在调用处使用 useCallback 包装。

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
React.useEffect(() => {
if (open && autoFocus === true) {
getContainer()?.focus({ preventScroll: true });
}
}, [open]);
React.useEffect(() => {
if (open && autoFocus === true) {
getContainer()?.focus({ preventScroll: true });
}
}, [open, autoFocus]);
🤖 Prompt for AI Agents
In src/hooks/useFocusable.ts around lines 17 to 21, the useEffect dependency
array only lists [open] but the effect also depends on autoFocus and the element
returned by getContainer(); update the dependency array to include autoFocus and
getContainer (or the stable reference used to obtain the container) so the
effect reruns when autoFocus flips or the container changes, and if getContainer
is not already stable, wrap it with useCallback at its source so it can safely
be included in the deps.

}
32 changes: 0 additions & 32 deletions tests/index.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { cleanup, fireEvent, render, act } from '@testing-library/react';
import KeyCode from '@rc-component/util/lib/KeyCode';
import { resetWarned } from '@rc-component/util/lib/warning';
import React from 'react';
import type { DrawerProps } from '../src';
Expand Down Expand Up @@ -265,37 +264,6 @@ describe('rc-drawer-menu', () => {
expect(container.contains(document.activeElement)).toBeTruthy();
});

it('tab should always in the content', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

这个case不测了吗

Copy link
Member Author

Choose a reason for hiding this comment

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

换成 rc-util 的 useFocusLock 了,这个用不到了。

const { container } = render(
<Drawer open getContainer={false}>
<div>Hello World</div>
</Drawer>,
);

const firstSentinel = container.querySelector<HTMLElement>(
'[data-sentinel="start"]',
);
const lastSentinel = container.querySelector<HTMLElement>(
'[data-sentinel="end"]',
);

// First shift to last
firstSentinel.focus();
fireEvent.keyDown(firstSentinel, {
shiftKey: true,
keyCode: KeyCode.TAB,
which: KeyCode.TAB,
});
expect(document.activeElement).toBe(lastSentinel);

// Last tab to first
fireEvent.keyDown(lastSentinel, {
keyCode: KeyCode.TAB,
which: KeyCode.TAB,
});
expect(document.activeElement).toBe(firstSentinel);
});

describe('keyboard', () => {
it('ESC to exit', () => {
const onClose = jest.fn();
Expand Down