Skip to content
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

Fix:RangePicker the triangle sign was not positioned correctly in some cases #886

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
4 changes: 2 additions & 2 deletions src/PickerInput/Popup/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export default function Popup<DateType extends object = any>(props: PopupProps<D
onSubmit,
} = props;

const { prefixCls } = React.useContext(PickerContext);
const { prefixCls, alignedPlacement } = React.useContext(PickerContext);
const panelPrefixCls = `${prefixCls}-panel`;

const rtl = direction === 'rtl';
Expand Down Expand Up @@ -213,7 +213,7 @@ export default function Popup<DateType extends object = any>(props: PopupProps<D
);

if (range) {
const realPlacement = getRealPlacement(placement, rtl);
const realPlacement = getRealPlacement(alignedPlacement || placement, rtl);
const offsetUnit = getoffsetUnit(realPlacement, rtl);
renderNode = (
<div
Expand Down
15 changes: 14 additions & 1 deletion src/PickerInput/RangePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,9 @@ function RangePicker<DateType extends object = any>(
onKeyDown?.(event, preventDefault);
};

// ======================= popup align =======================
const [alignedPlacement, setAlignedPlacement] = React.useState<string>();

// ======================= Context ========================
const context = React.useMemo(
() => ({
Expand All @@ -674,8 +677,18 @@ function RangePicker<DateType extends object = any>(
generateConfig,
button: components.button,
input: components.input,
alignedPlacement,
setAlignedPlacement,
}),
[prefixCls, locale, generateConfig, components.button, components.input],
[
prefixCls,
locale,
generateConfig,
components.button,
components.input,
alignedPlacement,
setAlignedPlacement,
],
);

// ======================== Effect ========================
Expand Down
6 changes: 3 additions & 3 deletions src/PickerInput/Selector/RangeSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function RangeSelector<DateType extends object = any>(
const rtl = direction === 'rtl';

// ======================== Prefix ========================
const { prefixCls } = React.useContext(PickerContext);
const { prefixCls, alignedPlacement } = React.useContext(PickerContext);

// ========================== Id ==========================
const ids = React.useMemo(() => {
Expand Down Expand Up @@ -173,7 +173,7 @@ function RangeSelector<DateType extends object = any>(
});

// ====================== ActiveBar =======================
const realPlacement = getRealPlacement(placement, rtl);
const realPlacement = getRealPlacement(alignedPlacement || placement, rtl);
const offsetUnit = getoffsetUnit(realPlacement, rtl);
const placementRight = realPlacement?.toLowerCase().endsWith('right');
const [activeBarStyle, setActiveBarStyle] = React.useState<React.CSSProperties>({
Expand All @@ -186,7 +186,7 @@ function RangeSelector<DateType extends object = any>(
if (input) {
const { offsetWidth, offsetLeft, offsetParent } = input.nativeElement;
const parentWidth = (offsetParent as HTMLElement)?.offsetWidth || 0;
const activeOffset = placementRight ? (parentWidth - offsetWidth - offsetLeft) : offsetLeft;
const activeOffset = placementRight ? parentWidth - offsetWidth - offsetLeft : offsetLeft;
setActiveBarStyle((ori) => ({
...ori,
width: offsetWidth,
Expand Down
4 changes: 3 additions & 1 deletion src/PickerInput/context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ export interface PickerContextProps<DateType = any> {
/** Customize button component */
button?: Components['button'];
input?: Components['input'];

/** trigger will change placement while aligining */
alignedPlacement?: string;
setAlignedPlacement?: React.Dispatch<React.SetStateAction<string>>;
}

const PickerContext = React.createContext<PickerContextProps>(null!);
Expand Down
12 changes: 11 additions & 1 deletion src/PickerTrigger/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ function PickerTrigger({
visible,
onClose,
}: PickerTriggerProps) {
const { prefixCls } = React.useContext(PickerContext);
const { prefixCls, setAlignedPlacement } = React.useContext(PickerContext);
const dropdownPrefixCls = `${prefixCls}-dropdown`;

const realPlacement = getRealPlacement(placement, direction === 'rtl');
Expand All @@ -100,6 +100,16 @@ function PickerTrigger({
popupStyle={popupStyle}
stretch="minWidth"
getPopupContainer={getPopupContainer}
onPopupAlign={(_, align) => {
Object.keys(BUILT_IN_PLACEMENTS).forEach((key) => {
if (
BUILT_IN_PLACEMENTS[key].points[0] === align.points[0] &&
BUILT_IN_PLACEMENTS[key].points[1] === align.points[1]
) {
setAlignedPlacement?.(key);
}
});
}}
bbb169 marked this conversation as resolved.
Show resolved Hide resolved
onPopupVisibleChange={(nextVisible) => {
if (!nextVisible) {
onClose();
Expand Down
134 changes: 134 additions & 0 deletions tests/range-aligin.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
import { act, cleanup, render } from '@testing-library/react';
bbb169 marked this conversation as resolved.
Show resolved Hide resolved
import { spyElementPrototypes } from 'rc-util/lib/test/domHook';
import React from 'react';
import { DayRangePicker } from './util/commonUtil';

describe('the popup arrow should be placed in the correct position.', () => {
let rangeRect = { x: 0, y: 0, width: 0, height: 0 };

beforeEach(() => {
rangeRect = {
x: 0,
y: 0,
width: 200,
height: 100,
};

document.documentElement.scrollLeft = 0;
});
bbb169 marked this conversation as resolved.
Show resolved Hide resolved

beforeAll(() => {
jest.spyOn(document.documentElement, 'scrollWidth', 'get').mockReturnValue(1000);

// Viewport size
spyElementPrototypes(HTMLElement, {
clientWidth: {
get: () => 400,
},
clientHeight: {
get: () => 400,
},
});

// Popup size
spyElementPrototypes(HTMLDivElement, {
getBoundingClientRect() {
if (this.className.includes('rc-picker-dropdown')) {
return {
x: 0,
y: 0,
width: 300,
height: 100,
};
}
if (this.className.includes('rc-picker-range')) {
return rangeRect;
}
},
offsetWidth: {
get() {
if (this.className.includes('rc-picker-range-wrapper')) {
return rangeRect.width;
}
if (this.className.includes('rc-picker-range-arrow')) {
return 10;
}
if (this.className.includes('rc-picker-input')) {
return 100;
}
if (this.className.includes('rc-picker-dropdown')) {
return 300;
}
},
},
offsetLeft: {
get() {
if (this.className.includes('rc-picker-input')) {
return 0;
}
},
},
});
spyElementPrototypes(HTMLElement, {
offsetParent: {
get: () => document.body,
},
});
});

beforeEach(() => {
jest.useFakeTimers();
});

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

it('the arrow should be set to `inset-inline-start` when the popup is aligned to `bottomLeft`.', async () => {
render(<DayRangePicker open />);

await act(async () => {
jest.runAllTimers();

await Promise.resolve();
});
expect(document.querySelector('.rc-picker-range-arrow')).toHaveStyle({
'inset-inline-start': '0',
});
});

it('the arrow should be set to `inset-inline-end` when the popup is aligned to `bottomRight`.', async () => {
const mock = spyElementPrototypes(HTMLDivElement, {
getBoundingClientRect() {
if (this.className.includes('rc-picker-dropdown')) {
return {
x: 0,
y: 0,
width: 300,
height: 100,
};
}
if (this.className.includes('rc-picker-range')) {
return {
...rangeRect,
x: 300,
};
}
},
});

render(<DayRangePicker open />);

await act(async () => {
jest.runAllTimers();

await Promise.resolve();
});
expect(document.querySelector('.rc-picker-range-arrow')).toHaveStyle({
'inset-inline-end': '0',
});

mock.mockRestore();
});
});
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

建议扩展测试用例覆盖范围

当前的测试用例仅覆盖了 bottomLeft 和 bottomRight 两种情况。建议添加以下场景的测试:

  1. 窗口大小改变时的箭头位置调整
  2. 不同 RTL(从右到左)设置下的箭头位置
  3. 边界情况处理(如容器宽度极小时)

这将确保组件在各种场景下都能正确定位箭头。

另外,建议将重复的等待逻辑抽取为测试辅助函数:

async function waitForUpdate() {
  await act(async () => {
    jest.runAllTimers();
    await Promise.resolve();
  });
}