Skip to content

Commit 263ebe1

Browse files
committed
fix: ensure focus is properly locked inside preview when opened via keyboard
The preview wrapper DOM element is not immediately available due to CSSMotion's deferred rendering (styleReady='NONE' on first render). Using useRef meant useLockFocus could never re-evaluate after the DOM appeared. Switch to useState callback ref so the component re-renders once the wrapper mounts, allowing useLockFocus to activate correctly. Also restores focus to the trigger element after the preview closes.
1 parent a2c9ca2 commit 263ebe1

2 files changed

Lines changed: 55 additions & 3 deletions

File tree

src/Preview/index.tsx

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,9 @@ const Preview: React.FC<PreviewProps> = props => {
196196
} = props;
197197

198198
const imgRef = useRef<HTMLImageElement>();
199-
const wrapperRef = useRef<HTMLDivElement>(null);
199+
const triggerRef = useRef<HTMLElement>(null);
200+
const [wrapperEl, setWrapperEl] = useState<HTMLDivElement>(null);
201+
200202
const groupContext = useContext(PreviewGroupContext);
201203
const showLeftOrRightSwitches = groupContext && count > 1;
202204
const showOperationsProgress = groupContext && count >= 1;
@@ -366,6 +368,10 @@ const Preview: React.FC<PreviewProps> = props => {
366368
const onVisibleChanged = (nextVisible: boolean) => {
367369
if (!nextVisible) {
368370
setLockScroll(false);
371+
372+
// Restore focus to the trigger element after leave animation
373+
triggerRef.current?.focus?.();
374+
triggerRef.current = null;
369375
}
370376
afterOpenChange?.(nextVisible);
371377
};
@@ -385,7 +391,13 @@ const Preview: React.FC<PreviewProps> = props => {
385391
};
386392

387393
// =========================== Focus ============================
388-
useLockFocus(open && portalRender, () => wrapperRef.current);
394+
useEffect(() => {
395+
if (open) {
396+
triggerRef.current = document.activeElement as HTMLElement;
397+
}
398+
}, [open]);
399+
400+
useLockFocus(open && !!wrapperEl, () => wrapperEl);
389401

390402
// ========================== Render ==========================
391403
const bodyStyle: React.CSSProperties = {
@@ -423,7 +435,7 @@ const Preview: React.FC<PreviewProps> = props => {
423435

424436
return (
425437
<div
426-
ref={wrapperRef}
438+
ref={setWrapperEl}
427439
className={clsx(prefixCls, rootClassName, classNames.root, motionClassName, {
428440
[`${prefixCls}-movable`]: movable,
429441
[`${prefixCls}-moving`]: isMoving,

tests/preview.test.tsx

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1256,4 +1256,44 @@ describe('Preview', () => {
12561256

12571257
expect(document.querySelector('.rc-image-preview')).toBeFalsy();
12581258
});
1259+
1260+
it('Focus should be trapped inside preview after keyboard open and restored on close', () => {
1261+
const rectSpy = jest.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockReturnValue({
1262+
x: 0, y: 0, width: 100, height: 100,
1263+
top: 0, right: 100, bottom: 100, left: 0,
1264+
toJSON: () => undefined,
1265+
} as DOMRect);
1266+
1267+
const { container } = render(<Image src="src" alt="focus trap" />);
1268+
const wrapper = container.querySelector('.rc-image') as HTMLElement;
1269+
1270+
// Open preview via keyboard
1271+
wrapper.focus();
1272+
expect(document.activeElement).toBe(wrapper);
1273+
1274+
fireEvent.keyDown(wrapper, { key: 'Enter' });
1275+
act(() => {
1276+
jest.runAllTimers();
1277+
});
1278+
1279+
// Focus should be inside the preview
1280+
const preview = document.querySelector('.rc-image-preview') as HTMLElement;
1281+
expect(preview).toBeTruthy();
1282+
expect(preview.contains(document.activeElement)).toBeTruthy();
1283+
1284+
// Focus should not escape when trying to focus outside
1285+
wrapper.focus();
1286+
expect(preview.contains(document.activeElement)).toBeTruthy();
1287+
1288+
// Close preview via Escape
1289+
fireEvent.keyDown(window, { key: 'Escape' });
1290+
act(() => {
1291+
jest.runAllTimers();
1292+
});
1293+
1294+
// Focus should return to the trigger element
1295+
expect(document.activeElement).toBe(wrapper);
1296+
1297+
rectSpy.mockRestore();
1298+
});
12591299
});

0 commit comments

Comments
 (0)