Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
18 changes: 15 additions & 3 deletions src/Preview/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,9 @@ const Preview: React.FC<PreviewProps> = props => {
} = props;

const imgRef = useRef<HTMLImageElement>();
const wrapperRef = useRef<HTMLDivElement>(null);
const triggerRef = useRef<HTMLElement>(null);
const [wrapperEl, setWrapperEl] = useState<HTMLDivElement>(null);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这里有点奇怪,ref 是正解。setState 是因为 useLockFocus 调用的时候拿不到 dom 吗?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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


const groupContext = useContext(PreviewGroupContext);
const showLeftOrRightSwitches = groupContext && count > 1;
const showOperationsProgress = groupContext && count >= 1;
Expand Down Expand Up @@ -366,6 +368,10 @@ const Preview: React.FC<PreviewProps> = props => {
const onVisibleChanged = (nextVisible: boolean) => {
if (!nextVisible) {
setLockScroll(false);

// Restore focus to the trigger element after leave animation
triggerRef.current?.focus?.();
triggerRef.current = null;
}
afterOpenChange?.(nextVisible);
};
Expand All @@ -385,7 +391,13 @@ const Preview: React.FC<PreviewProps> = props => {
};

// =========================== Focus ============================
useLockFocus(open && portalRender, () => wrapperRef.current);
useEffect(() => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

 改成 useLayouEffect,这样就不会强依赖于 useLockFocus 的先后顺序了。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

if (open) {
triggerRef.current = document.activeElement as HTMLElement;
}
}, [open]);

useLockFocus(open && !!wrapperEl, () => wrapperEl);

// ========================== Render ==========================
const bodyStyle: React.CSSProperties = {
Expand Down Expand Up @@ -423,7 +435,7 @@ const Preview: React.FC<PreviewProps> = props => {

return (
<div
ref={wrapperRef}
ref={setWrapperEl}
className={clsx(prefixCls, rootClassName, classNames.root, motionClassName, {
[`${prefixCls}-movable`]: movable,
[`${prefixCls}-moving`]: isMoving,
Expand Down
40 changes: 40 additions & 0 deletions tests/preview.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1256,4 +1256,44 @@ describe('Preview', () => {

expect(document.querySelector('.rc-image-preview')).toBeFalsy();
});

it('Focus should be trapped inside preview after keyboard open and restored on close', () => {
const rectSpy = jest.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockReturnValue({
x: 0, y: 0, width: 100, height: 100,
top: 0, right: 100, bottom: 100, left: 0,
toJSON: () => undefined,
} as DOMRect);

const { container } = render(<Image src="src" alt="focus trap" />);
const wrapper = container.querySelector('.rc-image') as HTMLElement;

// Open preview via keyboard
wrapper.focus();
expect(document.activeElement).toBe(wrapper);

fireEvent.keyDown(wrapper, { key: 'Enter' });
act(() => {
jest.runAllTimers();
});

// Focus should be inside the preview
const preview = document.querySelector('.rc-image-preview') as HTMLElement;
expect(preview).toBeTruthy();
expect(preview.contains(document.activeElement)).toBeTruthy();

// Focus should not escape when trying to focus outside
wrapper.focus();
expect(preview.contains(document.activeElement)).toBeTruthy();

// Close preview via Escape
fireEvent.keyDown(window, { key: 'Escape' });
act(() => {
jest.runAllTimers();
});

// Focus should return to the trigger element
expect(document.activeElement).toBe(wrapper);

rectSpy.mockRestore();
});
});
Loading