Fix：RangePicker the triangle sign was not positioned correctly in some cases #886
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改涉及多个组件的更新，主要是为了增强弹出窗口的对齐功能。 Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
src/PickerInput/context.tsx (1)
14-14: 建议完善 setAlignedPlacement 的文档说明为了保持文档的一致性和完整性，建议为
setAlignedPlacement属性添加类似的注释说明。建议添加如下注释：
/** trigger will change placement while aligining */ alignedPlacement?: string; + /** Callback to update the aligned placement state */ setAlignedPlacement?: React.Dispatch<React.SetStateAction<string>>;tests/range-aligin.spec.tsx (1)
20-77: 建议为复杂的模拟设置添加注释说明虽然模拟设置很完整，但建议添加注释来解释以下配置的用途：
- viewport 尺寸的设置原因
- 不同元素类名对应的尺寸值的计算依据
- getBoundingClientRect 返回值的设计考虑
这将帮助其他开发者更好地理解和维护测试代码。
src/PickerInput/RangePicker.tsx (1)
680-681: 确保对齐状态在上下文中可用将
alignedPlacement和setAlignedPlacement添加到上下文中是合理的，这样子组件就能够访问和更新对齐信息。依赖项列表的更新也很完整。不过建议添加类型注释以提高代码可维护性：
- const [alignedPlacement, setAlignedPlacement] = React.useState<string>(); + const [alignedPlacement, setAlignedPlacement] = React.useState<'top' | 'bottom'>();Also applies to: 689-690
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- src/PickerInput/Popup/index.tsx (2 hunks)
- src/PickerInput/RangePicker.tsx (2 hunks)
- src/PickerInput/Selector/RangeSelector.tsx (3 hunks)
- src/PickerInput/context.tsx (1 hunks)
- src/PickerTrigger/index.tsx (2 hunks)
- tests/range-aligin.spec.tsx (1 hunks)
🧰 Additional context used
🪛 Biome
tests/range-aligin.spec.tsx
[error] 79-81: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
🔇 Additional comments (9)
src/PickerInput/context.tsx (1)
12-13: 代码结构清晰，文档说明充分！
alignedPlacement属性的添加和其注释很好地解释了其用途，有助于解决三角标记定位问题。src/PickerTrigger/index.tsx (1)
80-80: 从上下文中获取 setAlignedPlacement 的实现正确！通过 React Context 获取 setAlignedPlacement 方法的方式符合最佳实践，这为动态更新三角形位置提供了必要的状态管理能力。
tests/range-aligin.spec.tsx (1)
1-4: 导入语句看起来不错！测试所需的所有必要库都已正确导入。
src/PickerInput/Popup/index.tsx (2)
87-87: 从上下文中获取 alignedPlacement 属性的实现正确！通过 PickerContext 获取 alignedPlacement 属性的方式符合 React 最佳实践，这为弹出框的定位提供了必要的上下文信息。
Line range hint
216-232: 请验证三角形箭头在不同场景下的位置正确性实现逻辑看起来合理，但建议验证以下场景：
- RTL 和 LTR 布局下的箭头位置
- 不同 placement 值的展示效果
- flex-end 布局下的对齐情况
✅ Verification successful
Let me gather more information about the placement and arrow styling implementation.
Let me gather one final piece of information about the placement calculation implementation.
箭头位置计算逻辑验证完成，实现正确
通过代码分析和测试用例验证，确认以下几点：
- RTL 和 LTR 布局下箭头位置计算正确，通过
getoffsetUnit函数自动切换- 已覆盖
bottomLeft和bottomRight两种主要场景的测试- 样式定义完善，包括箭头大小、位置和过渡动画
测试用例完整验证了：
- 默认
bottomLeft对齐时箭头位于左侧- 当弹出框位于右侧时自动切换为
bottomRight对齐，箭头位于右侧🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 搜索与箭头位置相关的样式定义 rg -l "range-arrow" --type css # 查找所有使用 RangePicker 的测试用例 rg -l "RangePicker.*test" --type typescript # 检查是否有其他组件依赖 placement 的计算 ast-grep --pattern 'getRealPlacement($_, $_)'Length of output: 736
Script:
#!/bin/bash # 查找样式文件 fd -e less -e css # 检查 getRealPlacement 的具体实现 ast-grep --pattern 'function getRealPlacement($$$)' # 查找测试文件 fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts # 检查箭头相关的样式类定义 rg "range-arrow" -A 5 -B 5Length of output: 8680
Script:
# 检查 getRealPlacement 函数的具体实现 rg -A 10 "function getRealPlacement" # 检查 getoffsetUnit 函数的实现 rg -A 5 "function getoffsetUnit" # 查看完整的测试用例内容 cat tests/range-aligin.spec.tsxLength of output: 4514
src/PickerInput/Selector/RangeSelector.tsx (2)
123-123: 从上下文中获取 alignedPlacement从 PickerContext 中解构出 alignedPlacement，用于优化弹出窗口的定位逻辑。
Line range hint
176-189: 优化了活动条的位置计算逻辑代码改进：
- 使用 alignedPlacement 作为首选定位策略
- 简化了 activeOffset 的计算逻辑
- 修复了三角形标记在某些情况下位置不正确的问题
建议：考虑添加单元测试以验证不同布局场景下的位置计算。
src/PickerInput/RangePicker.tsx (2)
669-671: 新增状态用于控制弹出层对齐方式新增的
alignedPlacement状态变量将用于跟踪和控制弹出层的对齐位置，这是解决三角标记定位问题的关键部分。
683-691: 验证上下文依赖项的完整性上下文的依赖项列表包含了所有必要的值，这确保了当任何相关值发生变化时，上下文会正确地更新。
tests/range-aligin.spec.tsx
Outdated
| 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(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议扩展测试用例覆盖范围
当前的测试用例仅覆盖了 bottomLeft 和 bottomRight 两种情况。建议添加以下场景的测试：
- 窗口大小改变时的箭头位置调整
- 不同 RTL（从右到左）设置下的箭头位置
- 边界情况处理（如容器宽度极小时）
这将确保组件在各种场景下都能正确定位箭头。
另外，建议将重复的等待逻辑抽取为测试辅助函数：
async function waitForUpdate() {
await act(async () => {
jest.runAllTimers();
await Promise.resolve();
});
}
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #886 +/- ##
==========================================
+ Coverage 95.43% 95.46% +0.02%
==========================================
Files 64 64
Lines 2719 2735 +16
Branches 762 773 +11
==========================================
+ Hits 2595 2611 +16
Misses 121 121
Partials 3 3 ☔ View full report in Codecov by Sentry. |
|
|
是的 我再看看 |
已修复 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
tests/range-align.spec.tsx (2)
6-85: 建议改进测试设置的组织结构当前的测试设置代码较为复杂，建议进行以下优化：
- 将模拟配置提取到单独的辅助函数中
- 为复杂的模拟添加更详细的注释，解释每个值的用途
- 考虑使用常量来存储固定的尺寸值
建议重构示例：
// 视口尺寸常量 const VIEWPORT_SIZE = { width: 400, height: 400, }; // 弹出框尺寸常量 const POPUP_SIZE = { width: 300, height: 100, }; // 设置视口尺寸 const mockViewport = () => { spyElementPrototypes(HTMLElement, { clientWidth: { get: () => VIEWPORT_SIZE.width }, clientHeight: { get: () => VIEWPORT_SIZE.height }, }); }; // 设置元素尺寸 const mockElementSizes = () => { spyElementPrototypes(HTMLDivElement, { // ... 现有的模拟代码 ... }); };
87-90: 建议增强清理过程当前的清理过程可以更加完整。建议添加对所有模拟的显式还原。
建议添加：
afterEach(() => { cleanup(); jest.useRealTimers(); // 还原所有的 spy jest.restoreAllMocks(); // 清除所有的元素原型模拟 document.documentElement.scrollLeft = 0; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/PickerInput/Selector/RangeSelector.tsx(4 hunks)tests/range-align.spec.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/PickerInput/Selector/RangeSelector.tsx
🔇 Additional comments (1)
tests/range-align.spec.tsx (1)
1-5: 导入声明看起来很合适！
测试文件引入了必要的测试库和组件，包括 React Testing Library 用于组件测试，rc-util 用于 DOM 模拟，以及自定义的测试工具组件。
tests/range-align.spec.tsx
Outdated
| 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': '100px', | ||
| }); | ||
|
|
||
| expect(document.querySelector('.rc-picker-active-bar')).toHaveStyle({ | ||
| 'inset-inline-end': '100px', | ||
| }); | ||
|
|
||
| mock.mockRestore(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议扩展测试用例覆盖范围
当前测试用例仅覆盖了 bottomLeft 和 bottomRight 两种情况。建议增加以下测试场景：
- 窗口大小变化时的箭头位置
- 滚动时的箭头位置
- 边界情况（如容器太小）
- 动态内容变化时的位置更新
示例测试用例：
it('应该在窗口调整大小时正确更新箭头位置', async () => {
render(<DayRangePicker open />);
// 模拟窗口调整
act(() => {
global.innerWidth = 800;
global.dispatchEvent(new Event('resize'));
});
await act(async () => {
jest.runAllTimers();
});
// 验证箭头位置是否更新
});
it('应该在容器空间不足时正确处理箭头位置', async () => {
// 模拟一个很小的容器
rangeRect.width = 50;
render(<DayRangePicker open />);
await act(async () => {
jest.runAllTimers();
});
// 验证箭头位置处理
});
|
还有一个问题，这个问题好像之前就有，上右对齐的时候，箭头不在下方 |
|
这个问题跟本次的问题是一个解决方式 ，加个alignedPlacement就能解决了 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
src/PickerInput/RangePicker.tsx (1)
680-681: Context 依赖项更新需要优化虽然新增的
alignedPlacement相关状态已正确添加到 context 中，但建议将这些新增的状态与其他布局相关的属性组合在一起，以提高代码的组织性。建议重构如下：
const context = React.useMemo( () => ({ prefixCls, locale, generateConfig, button: components.button, input: components.input, + // Layout related props alignedPlacement, setAlignedPlacement, }), [ prefixCls, locale, generateConfig, components.button, components.input, + // Layout related deps alignedPlacement, setAlignedPlacement, ], );Also applies to: 683-691
tests/new-range.spec.tsx (2)
60-127: 建议优化元素模拟的代码组织当前的元素模拟代码结构较为复杂，建议将相关的模拟逻辑进行分组和注释。
建议按照以下方式重构：
beforeAll(() => { + // Mock viewport dimensions jest.spyOn(document.documentElement, 'scrollWidth', 'get').mockReturnValue(1000); - // Viewport size spyElementPrototypes(HTMLElement, { clientWidth: { get: () => 400, }, clientHeight: { get: () => 400, }, }); - // Popup size + // Mock component dimensions and positioning spyElementPrototypes(HTMLDivElement, { getBoundingClientRect() { + // Dropdown dimensions if (this.className.includes('rc-picker-dropdown')) { return { x: 0, y: 0, width: 300, height: 100, }; } + // Range picker dimensions if (this.className.includes('rc-picker-range')) { return rangeRect; } + // Picker dimensions if (this.className.includes('rc-picker')) { return rangeRect; } }, // ... rest of the code }); });
1425-1476: 建议增强弹出框对齐测试用例当前的测试用例已经覆盖了基本场景，但建议添加更多边界情况的测试。
建议考虑添加以下测试场景：
- 窗口边缘对齐的情况
- 不同 RTL 设置下的对齐情况
- 弹出框大小动态变化时的对齐行为
示例：
it('should handle alignment when popup size changes', async () => { const { rerender } = render(<DayRangePicker open />); // 修改弹出框大小 rangeRect.width = 400; rerender(<DayRangePicker open />); await act(async () => { jest.runAllTimers(); await Promise.resolve(); }); // 验证箭头位置是否正确调整 expect(document.querySelector('.rc-picker-range-arrow')).toHaveStyle({ 'inset-inline-end': '200px', }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/PickerInput/RangePicker.tsx(3 hunks)src/PickerTrigger/index.tsx(3 hunks)tests/new-range.spec.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/PickerTrigger/index.tsx
🧰 Additional context used
🪛 Biome
tests/new-range.spec.tsx
[error] 48-58: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
🔇 Additional comments (2)
src/PickerInput/RangePicker.tsx (2)
669-671: 新增状态用于管理弹出框对齐方式
引入 alignedPlacement 状态变量来追踪弹出框的实际对齐位置，这是解决三角标记定位问题的关键部分。
755-755: 验证 alignedPlacement 在不同场景下的表现
需要确保在各种布局场景下（特别是使用 flex-end 的搜索表单中）三角标记的位置都能正确对齐。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
tests/new-range.spec.tsx (1)
1422-1473: 建议优化测试用例的实现方式测试用例本身验证了正确的功能点，但有以下几点建议改进：
- 建议将 mock 实现抽取为可重用的辅助函数
- 建议为魔法数字（如 100px）添加注释说明其计算方式
建议按照以下方式重构：
+ // 抽取共用的 mock 函数 + const mockPickerRect = (x: number) => { + return 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 }; + } + }, + }); + }; describe('pupop aligned position', () => { it('the arrow should be set to `inset-inline-start`...', 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`...', 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 }; - } - }, - }); + // 使用抽取的 mock 函数 + const mock = mockPickerRect(300); + // 100px = rangeRect.width / 2 render(<DayRangePicker open />); await act(async () => { jest.runAllTimers(); await Promise.resolve(); }); expect(document.querySelector('.rc-picker-range-arrow')) .toHaveStyle({ 'inset-inline-end': '100px' }); expect(document.querySelector('.rc-picker-active-bar')) .toHaveStyle({ 'inset-inline-end': '100px' }); mock.mockRestore(); }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
tests/new-range.spec.tsx(5 hunks)
🔇 Additional comments (1)
tests/new-range.spec.tsx (1)
1181-1181: 测试用例实现正确！
tabIndex 属性传递的测试用例实现简洁明了，验证了预期功能。
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
src/PickerInput/Selector/util.ts (1)
17-19: 建议添加空值检查以提高代码健壮性函数实现简洁明了，但建议添加空值检查以防止可能的运行时错误。
建议修改如下：
export function getWin(ele: HTMLElement) { - return ele.ownerDocument.defaultView; + return ele?.ownerDocument?.defaultView || window; }这样可以：
- 防止
ele为null的情况- 防止
ownerDocument为null的情况- 提供
window作为降级方案tests/new-range.spec.tsx (1)
1430-1511: 建议优化测试代码结构为提高代码的可维护性，建议：
- 将共同的设置代码抽取为辅助函数，如
mockComputedStyle和mockElementRect- 为魔法数字添加注释说明，如
borderRightWidth: '4px'和inset-inline-end: '100px'的计算依据建议重构示例：
+ // 模拟计算样式 + const mockComputedStyle = () => { + const oriGetComputedStyle = window.getComputedStyle; + window.getComputedStyle = (ele: HTMLElement) => { + const retObj = oriGetComputedStyle(ele); + // 模拟边框宽度用于箭头位置计算 + retObj.borderRightWidth = '4px'; + retObj.borderLeftWidth = '2px'; + return retObj; + }; + return oriGetComputedStyle; + }; describe('pupop aligned position', () => { let oriGetComputedStyle; beforeEach(() => { + oriGetComputedStyle = mockComputedStyle(); }); afterEach(() => { + window.getComputedStyle = oriGetComputedStyle; });tests/range.spec.tsx (1)
544-546: 建议同时检查第二个输入框的 tabIndex 属性在当前测试中，只验证了第一个输入框的
tabIndex属性是否正确。由于DayRangePicker组件包含两个输入框（开始日期和结束日期），建议同时检查第二个输入框的tabIndex属性，确保两个输入框的tabIndex都被正确设置。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/PickerInput/Selector/RangeSelector.tsx(4 hunks)src/PickerInput/Selector/util.ts(1 hunks)tests/new-range.spec.tsx(5 hunks)tests/range.spec.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/PickerInput/Selector/RangeSelector.tsx
🔇 Additional comments (3)
tests/new-range.spec.tsx (2)
1189-1189: 测试用例实现正确！
tabIndex 属性传递的测试用例实现简单明了。
1430-1511: 测试实现完整且覆盖了关键场景！
测试用例完整验证了弹出层箭头在不同对齐方式下的定位，包括：
- bottomLeft 对齐时的箭头位置
- bottomRight 对齐时的箭头位置
- 激活条的位置计算
测试代码正确模拟了所需的浏览器 API 并确保了适当的清理。
tests/range.spec.tsx (1)
708-709: 确认前缀元素正确渲染
测试用例成功验证了 DayRangePicker 组件的 prefix 属性，前缀元素正确渲染至文档中，代码实现无误。
| ); | ||
| const offsetLeft = x - pX; | ||
|
|
||
| const activeOffset = placementRight ? parentWidth - inputWidth - offsetLeft : offsetLeft; |
There was a problem hiding this comment.
想办法给这种自动更换 placement 情况下的 offsetUnit 去反应该就行。
fix: ant-design/ant-design#51071
picker/src/PickerInput/Popup/index.tsx
Line 217 in 281bf37
主要是这处的问题，trrigger自己改了popupAligin为tr，但是外面没有收到，依旧是以tl来判断offsetUnit，所以只要在popup重新调整placement的时候更新就行
Summary by CodeRabbit
新功能
测试
DayRangePicker组件的测试，增加了对弹出窗口对齐和尺寸的验证，确保组件在不同条件下的布局和行为准确无误。