Skip to content
Merged
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
5 changes: 4 additions & 1 deletion src/Dialog/Content/Panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export interface PanelProps extends Omit<IDialogPropTypes, 'getOpenCount'> {
onMouseDown?: React.MouseEventHandler;
onMouseUp?: React.MouseEventHandler;
holderRef?: React.Ref<HTMLDivElement>;
/** Used for focus lock. When true and open, focus will lock into the panel */
isFixedPos?: boolean;
}

export type PanelRef = {
Expand Down Expand Up @@ -43,14 +45,15 @@ const Panel = React.forwardRef<PanelRef, PanelProps>((props, ref) => {
height,
classNames: modalClassNames,
styles: modalStyles,
isFixedPos,
} = props;

// ================================= Refs =================================
const { panel: panelRef } = React.useContext(RefContext);
const internalRef = useRef<HTMLDivElement>(null);
const mergedRef = useComposeRef(holderRef, panelRef, internalRef);

useLockFocus(visible, () => internalRef.current);
useLockFocus(visible && isFixedPos, () => internalRef.current);

React.useImperativeHandle(ref, () => ({
focus: () => {
Expand Down
10 changes: 9 additions & 1 deletion src/Dialog/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ const Dialog: React.FC<IDialogPropTypes> = (props) => {
const contentRef = useRef<ContentRef>(null);

const [animatedVisible, setAnimatedVisible] = React.useState(visible);
const [isFixedPos, setIsFixedPos] = React.useState(false);

// ========================== Init ==========================
const ariaId = useId();
Expand Down Expand Up @@ -116,7 +117,7 @@ const Dialog: React.FC<IDialogPropTypes> = (props) => {
const contentClickRef = useRef(false);
const contentTimeoutRef = useRef<ReturnType<typeof setTimeout>>(null);

// We need record content click incase content popup out of dialog
// We need record content click in case content popup out of dialog

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

这是一个小小的拼写错误,将 "incase" 改为 "in case" 会更符合英语习惯。

Suggested change
// We need record content click in case content popup out of dialog
// We need record content click in case content popup out of dialog

const onContentMouseDown: React.MouseEventHandler = () => {
clearTimeout(contentTimeoutRef.current);
contentClickRef.current = true;
Expand Down Expand Up @@ -154,6 +155,12 @@ const Dialog: React.FC<IDialogPropTypes> = (props) => {
if (visible) {
setAnimatedVisible(true);
saveLastOutSideActiveElementRef();

// Calc the position style
if (wrapperRef.current) {
const computedWrapStyle = getComputedStyle(wrapperRef.current);
setIsFixedPos(computedWrapStyle.position === 'fixed');
Comment on lines +159 to +162

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

isFixedPos 的计算只在 visible 变为 true 时执行。如果 wrapperRef.currentposition 样式在对话框可见期间动态改变,isFixedPos 的值将不会更新,这可能导致焦点锁定行为不符合预期。建议考虑 wrapperRef.currentposition 样式是否可能在对话框打开后发生变化。如果可能,需要更频繁地重新评估 isFixedPos

}
} else if (
animatedVisible &&
contentRef.current.enableMotion() &&
Expand Down Expand Up @@ -203,6 +210,7 @@ const Dialog: React.FC<IDialogPropTypes> = (props) => {
>
<Content
{...props}
isFixedPos={isFixedPos}
onMouseDown={onContentMouseDown}
onMouseUp={onContentMouseUp}
ref={contentRef}
Expand Down
67 changes: 67 additions & 0 deletions tests/focus.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/* eslint-disable react/no-render-return-value, max-classes-per-file, func-names, no-console */
import React from 'react';
import { act, render } from '@testing-library/react';
import Dialog from '../src';

// Mock: import { useLockFocus } from '@rc-component/util/lib/Dom/focus';
jest.mock('@rc-component/util/lib/Dom/focus', () => {
const actual = jest.requireActual('@rc-component/util/lib/Dom/focus');

const useLockFocus = (visible: boolean, ...rest: any[]) => {
globalThis.__useLockFocusVisible = visible;
return actual.useLockFocus(visible, ...rest);
};

return {
...actual,
useLockFocus,
};
});

/**
* Since overflow scroll test need a clear env which may affect by other test.
* Use a clean env instead.
*/
describe('Dialog.Focus', () => {
beforeEach(() => {
jest.useFakeTimers();
});

afterEach(() => {
jest.useRealTimers();
});

it('Should lock when fixed', () => {
render(
<Dialog
visible
styles={{
wrapper: { position: 'fixed' },
}}
/>,
);

act(() => {
jest.runAllTimers();
});

expect(globalThis.__useLockFocusVisible).toBe(true);
});

it('Should not lock when not fixed', () => {
render(
<Dialog
visible
styles={{
wrapper: { position: 'absolute' },
}}
/>,
);

act(() => {
jest.runAllTimers();
});

expect(globalThis.__useLockFocusVisible).toBe(false);
});
});
Loading