feat: support column.resizable #1106
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
linxianxi
force-pushed
the
feat-resizable
branch
from
02c41ed to
0eab6c4
Compare
|
体验有点怪异，如果是超出 width 的时候缩小应该就是缩小，不应该把右边的撑开才对。 |
linxianxi
force-pushed
the
feat-resizable
branch
from
6ad31d3 to
c50f2da
Compare
linxianxi
force-pushed
the
feat-resizable
branch
from
c50f2da to
97d5903
Compare
|
看起来好像没对齐 
|
|
@linxianxi is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
docs/examples/column-resize.tsx (7)
23-30: 列定义结构良好但可进一步优化列定义清晰，使用
.map()添加resizable属性和动态宽度的方式很灵活。不过，fallback逻辑widthMap.get(i.key ?? i.dataIndex) ?? i.width可能在某些情况下需要额外处理（如宽度值为非数字类型时）。考虑添加类型保护，确保宽度总是数值类型：
- width: widthMap.get(i.key ?? i.dataIndex) ?? i.width, + width: Number(widthMap.get(i.key ?? i.dataIndex) ?? i.width) || i.width,
32-49: 固定列配置合理但默认宽度处理不一致固定列的配置很完善，包含了左固定和右固定列，以及最小宽度限制。但与第一个columns配置相比，这里添加了额外的默认宽度
?? 150，两处的fallback逻辑不一致。虽然不影响功能，但维护时可能引起困惑。考虑统一两处的默认宽度处理逻辑，或添加注释说明不同之处的原因。
- width: widthMap.get(i.key ?? i.dataIndex) ?? i.width ?? 150, + // 固定列需要确保有默认宽度以避免布局问题 + width: widthMap.get(i.key ?? i.dataIndex) ?? i.width ?? 150,
53-53: UI文本混合了中英文在示例代码中混合中英文可能会对不懂中文的开发者造成困扰。考虑使用一种语言（英文）来保持示例的一致性和可访问性。
- table width: 800px {'columns=[{width: 100, width: 100}]'} 情况 + table width: 800px {'columns=[{width: 100, width: 100}]'} case
59-67: 重复的状态更新逻辑可以提取
onColumnResizeEnd回调在两个表格中有完全相同的实现。可以考虑将此逻辑提取为一个单独的函数，以减少代码重复并简化后续维护。+ const handleColumnResizeEnd = ({ columnWidths }) => { + setWidthMap(prev => { + const result = new Map(prev); + columnWidths.forEach(i => { + result.set(i.columnKey, i.width); + }); + return result; + }); + }; <Table // ...其他属性 - onColumnResizeEnd={({ columnWidths }) => { - setWidthMap(prev => { - const result = new Map(prev); - columnWidths.forEach(i => { - result.set(i.columnKey, i.width); - }); - return result; - }); - }} + onColumnResizeEnd={handleColumnResizeEnd} // ...其他属性 />
69-76: 容器宽度计算逻辑可以提取
getContainerWidth函数在两个表格中有完全相同的实现。建议将此逻辑提取为一个单独的函数，以减少代码重复。+ const getTableContainerWidth = (ele, width) => { + // Minus border + const borderWidth = getComputedStyle( + ele.querySelector('.rc-table-body'), + ).borderInlineStartWidth; + const mergedWidth = width - parseInt(borderWidth, 10); + return mergedWidth; + }; <Table // ...其他属性 - getContainerWidth={(ele, width) => { - // Minus border - const borderWidth = getComputedStyle( - ele.querySelector('.rc-table-body'), - ).borderInlineStartWidth; - const mergedWidth = width - parseInt(borderWidth, 10); - return mergedWidth; - }} + getContainerWidth={getTableContainerWidth} // ...其他属性 />
90-98: 与上面相同，函数逻辑重复这与第一个表格中的
onColumnResizeEnd回调完全相同，应该提取为一个共享函数。<Table // ...其他属性 - onColumnResizeEnd={({ columnWidths }) => { - setWidthMap(prev => { - const result = new Map(prev); - columnWidths.forEach(i => { - result.set(i.columnKey, i.width); - }); - return result; - }); - }} + onColumnResizeEnd={handleColumnResizeEnd} // ...其他属性 />
100-107: 与上面相同，函数逻辑重复这与第一个表格中的
getContainerWidth函数完全相同，应该提取为一个共享函数。<Table // ...其他属性 - getContainerWidth={(ele, width) => { - // Minus border - const borderWidth = getComputedStyle( - ele.querySelector('.rc-table-body'), - ).borderInlineStartWidth; - const mergedWidth = width - parseInt(borderWidth, 10); - return mergedWidth; - }} + getContainerWidth={getTableContainerWidth} // ...其他属性 />tests/Resizable.spec.tsx (3)
45-55: 元素属性模拟设置适当使用
spyElementPrototype来模拟DOM元素属性是很好的实践，确保测试环境的一致性。不过，硬编码的宽度值（如offsetWidth为800和400）可能会限制测试的灵活性。考虑使用变量来定义这些宽度值，使它们在测试中更容易调整：
+ const CONTAINER_WIDTH = 800; + const CELL_WIDTH = 400; containerSpy = spyElementPrototype(HTMLDivElement, 'offsetWidth', { - get: () => 800, + get: () => CONTAINER_WIDTH, }); measureCellSpy = spyElementPrototype(HTMLTableCellElement, 'offsetWidth', { - get: () => 400, + get: () => CELL_WIDTH, });
130-174: 总宽度小于组件宽度的场景测试此测试验证了当列的总宽度小于组件宽度时的调整行为，特别是验证了相邻列宽度的自动调整。测试中假设滚动条宽度为15px，这可能在不同环境中有所不同。
建议将滚动条宽度定义为常量，并添加注释说明这是一个假设值：
+ // 假设标准滚动条宽度为15px + const SCROLLBAR_WIDTH = 15; expect(onColumnResizeEnd).toHaveBeenCalledWith({ columnKey: 'a', width: 300, columnWidths: [ { columnKey: 'a', width: 300 }, - { columnKey: 'b', width: 485 }, + { columnKey: 'b', width: 800 - 300 - SCROLLBAR_WIDTH }, // scrollbar 15px ], });
1-221: 测试覆盖全面但可考虑添加更多场景测试文件整体设计合理，覆盖了调整列宽的核心功能。不过，可以考虑添加以下测试场景：
- RTL模式下的列宽调整行为
- 带有嵌套列头的表格调整行为
- 调整过程中的事件（如
onColumnResizing而非仅测试onColumnResizeEnd）- 边缘情况处理，如快速连续调整多列
另外，建议将重复的App组件逻辑提取为一个辅助函数或使用测试上下文共享，可以减少测试代码的重复。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
README.md(1 hunks)assets/index.less(3 hunks)docs/examples/column-resize.tsx(1 hunks)package.json(0 hunks)src/Body/MeasureCell.tsx(1 hunks)src/Body/MeasureRow.tsx(2 hunks)src/Body/index.tsx(3 hunks)src/FixedHolder/index.tsx(1 hunks)src/Header/HeaderCell.tsx(1 hunks)src/Header/HeaderRow.tsx(3 hunks)src/Header/useCellResize.tsx(1 hunks)src/Table.tsx(9 hunks)src/VirtualTable/BodyGrid.tsx(3 hunks)src/context/TableContext.tsx(2 hunks)src/interface.ts(2 hunks)tests/Resizable.spec.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (13)
- src/VirtualTable/BodyGrid.tsx
- src/Body/index.tsx
- src/FixedHolder/index.tsx
- src/interface.ts
- README.md
- src/Header/HeaderCell.tsx
- src/Body/MeasureCell.tsx
- src/Body/MeasureRow.tsx
- assets/index.less
- src/Header/HeaderRow.tsx
- src/context/TableContext.tsx
- src/Table.tsx
- src/Header/useCellResize.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
docs/examples/column-resize.tsx (2)
src/interface.ts (2)
Key(21-21)ColumnType(108-124)docs/examples/utils/useInput.ts (1)
useCheckbox(3-18)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test / react component workflow
🔇 Additional comments (5)
docs/examples/column-resize.tsx (1)
19-21: 钩子状态声明符合最佳实践使用
useState钩子和Map类型来管理列宽的状态非常合适，这样能够高效地追踪和更新每列的宽度。代码结构清晰，使用RTL布局切换功能也很实用。tests/Resizable.spec.tsx (4)
7-12: 触发Resize事件的辅助函数实现良好
triggerResize辅助函数的实现符合测试最佳实践，使用act包装异步操作，并等待Promise解析，确保测试的可靠性。
14-38: 鼠标事件模拟函数设计合理
doMouseMove函数很好地模拟了拖动操作的完整生命周期（mouseDown, mouseMove, mouseUp），包括自定义事件属性的定义。逻辑清晰，参数灵活，支持可选的mouseUp事件。
67-128: 宽度变更测试用例实现完整此测试用例完整验证了列宽调整功能，包括调整一列后对
onColumnResizeEnd回调的验证，以及更新测量单元格宽度后再次调整另一列的验证。测试流程和断言都很完善。
176-220:❓ Verification inconclusive
最小宽度约束测试实现正确
此测试验证了
minWidth属性的工作方式，确保列宽不会小于指定的最小值。测试模拟了尝试将列宽调整到最小值以下的情况，并验证了结果约束在最小宽度。同样，这里也使用了15px的滚动条宽度假设。考虑验证更多的边缘情况，例如：
- 当所有列都设置了最小宽度时，是否会影响调整行为
- 当表格宽度小于所有列最小宽度总和时的行为
🏁 Script executed:
#!/bin/bash # 查找源代码中对于最小宽度处理的逻辑 echo "查找最小宽度处理相关代码..." rg -n "minWidth" --type ts --type tsx src/Length of output: 244
🏁 Script executed:
#!/bin/bash echo "重新搜索 minWidth 相关逻辑..." rg -n "minWidth" -g "*.ts*" src/Length of output: 869
最小宽度约束测试实现正确
验证发现：
- 在
src/Header/useCellResize.tsx中，第 64 行已实现if (newWidth < minWidth) newWidth = minWidth;的约束逻辑。minWidth属性在接口声明（src/interface.ts:119）、ColGroup.tsx、HeaderCell.tsx、HeaderRow.tsx等多处正确传递与应用，确保渲染时会生效。因此当前测试成功覆盖了当拖拽使列宽低于
minWidth时自动约束至最小宽度的场景，行为符合预期。请考虑补充以下边缘用例的测试：
- 所有列均设置
minWidth时，拖拽对其他列宽度的影响- 表格整体宽度小于所有列最小宽度总和时的渲染与调整表现
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Hello |
|
I am following this as I am also waiting for this feature. |
|
Same here |
|
some one ship it out |
|
I am following this as I am also waiting for this feature. |
|
waiting for it😊 |
|
重构table搁置了，处理一下冲突推进得了。 |
那敢情好啊 |
|
waiting for it😊 |
4 similar comments
|
waiting for it😊 |
|
waiting for it😊 |
|
waiting for it😊 |
|
waiting for it😊 |
原先内部
onColumnResize重命名为onColumnWidthChange，进行区分Summary by CodeRabbit