-
-
Notifications
You must be signed in to change notification settings - Fork 105
feat: Support focusable #561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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'; | ||
|
|
@@ -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'; | ||
|
|
||
|
|
@@ -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; | ||
|
|
@@ -104,7 +99,10 @@ const DrawerPopup: React.ForwardRefRenderFunction< | |
| inline, | ||
| push, | ||
| forceRender, | ||
|
|
||
| // Focus | ||
| autoFocus, | ||
| focusTrap, | ||
|
|
||
| // classNames | ||
| classNames: drawerClassNames, | ||
|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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
fiRepository: 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 -20Repository: 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 -100Repository: 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=tsxRepository: 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 1Repository: 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 -10Repository: react-component/drawer Length of output: 154 🏁 Script executed: # Look at RefContext to understand the panel ref setup
rg "RefContext" -B 5 -A 10Repository: 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.tsxRepository: 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 5Repository: 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 2Repository: react-component/drawer Length of output: 468 🏁 Script executed: # Check the context.ts file to understand RefContext better
cat -n src/context.tsRepository: 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 2Repository: 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 2Repository: 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 15Repository: 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 -20Repository: 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 -20Repository: 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 -40Repository: 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 -10Repository: 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=tsxRepository: react-component/drawer Length of output: 93 useEffect 缺失
在 React.useEffect(() => {
if (open && autoFocus === true) {
getContainer()?.focus({ preventScroll: true });
}
- }, [open]);
+ }, [open, autoFocus]);
🤖 Prompt for AI Agents |
||
|
|
||
| // ============================ Push ============================ | ||
| const [pushed, setPushed] = React.useState(false); | ||
|
|
@@ -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`} | ||
| > | ||
|
|
@@ -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> | ||
| ); | ||
|
|
||
| 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]); | ||||||||||||||||||||||
zombieJ marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+17
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. useEffect 依赖数组不完整。 第 17-21 行的 当 建议的修复方案 React.useEffect(() => {
if (open && autoFocus === true) {
getContainer()?.focus({ preventScroll: true });
}
- }, [open]);
+ }, [open, autoFocus]);注意: 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| } | ||||||||||||||||||||||
| 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'; | ||
|
|
@@ -265,37 +264,6 @@ describe('rc-drawer-menu', () => { | |
| expect(container.contains(document.activeElement)).toBeTruthy(); | ||
| }); | ||
|
|
||
| it('tab should always in the content', () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这个case不测了吗
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.