Skip to content

Commit 3540762

Browse files
authored
fix: lock focus inside preview when opened via keyboard (#505)
* 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. * fix: update focus lock implementation in Preview component
1 parent a2c9ca2 commit 3540762

3 files changed

Lines changed: 53 additions & 1 deletion

File tree

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
"dependencies": {
4343
"@rc-component/motion": "^1.0.0",
4444
"@rc-component/portal": "^2.1.2",
45-
"@rc-component/util": "^1.10.0",
45+
"@rc-component/util": "^1.10.1",
4646
"clsx": "^2.1.1"
4747
},
4848
"devDependencies": {

src/Preview/index.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,8 @@ const Preview: React.FC<PreviewProps> = props => {
197197

198198
const imgRef = useRef<HTMLImageElement>();
199199
const wrapperRef = useRef<HTMLDivElement>(null);
200+
const triggerRef = useRef<HTMLElement>(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,6 +391,12 @@ const Preview: React.FC<PreviewProps> = props => {
385391
};
386392

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

390402
// ========================== Render ==========================

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)