-
-
Notifications
You must be signed in to change notification settings - Fork 614
feat: Table support expandedRowOffset #1284
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
Conversation
* fix: sticky event loop * chore: adjust script
* fix: scroll logic (#1239) * fix: scroll logic * fix: ci * chore: fix lint
Walkthrough本次更改为表格组件引入了“展开行偏移量”支持,实现了展开行与粘性列的协同显示。涉及上下文、hooks、主体渲染、单元格合并、展开行定位等多处代码调整,并补充了相关文档与测试用例。 Changes
Sequence Diagram(s)sequenceDiagram
participant 用户
participant Table
participant Body
participant BodyRow
participant ExpandedRow
用户->>Table: 设置 expandedRowOffset, colWidths, expandable
Table->>Body: 传递 context(含 expandedRowOffset, colWidths)
Body->>BodyRow: 传递 rowKeys, expandedRowInfo
BodyRow->>ExpandedRow: 渲染展开行,传递 stickyOffset
ExpandedRow-->>用户: 展开行内容根据 stickyOffset 正确定位
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/Body/BodyRow.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. docs/examples/expandedSticky.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. src/Body/ExpandedRow.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1284 +/- ##
========================================
Coverage 98.08% 98.08%
========================================
Files 77 78 +1
Lines 7609 7745 +136
Branches 1164 1186 +22
========================================
+ Hits 7463 7597 +134
- Misses 140 142 +2
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
docs/examples/expandedSticky.tsx (2)
7-20
: 建议清理未使用的代码
getRowSpan
函数和相关的注释代码(如rowKeys
和rowSpanList
)都未被使用。建议删除这些代码以保持示例的简洁性。-// 合并单元格 -export const getRowSpan = (source: (string | number | undefined)[] = []) => { - const list: { rowSpan?: number }[] = []; - let span = 0; - source.reverse().forEach((key, index) => { - span = span + 1; - if (key !== source[index + 1]) { - list.push({ rowSpan: span }); - span = 0; - } else { - list.push({ rowSpan: 0 }); - } - }); - return list.reverse(); -}; - const Demo = () => { const [expandedRowKeys, setExpandedRowKeys] = useState<readonly React.Key[]>([]); const data = [ { key: 'a', a: '小二', d: '文零西路' }, { key: 'b', a: '张三', d: '文一西路' }, { key: 'c', a: '张三', d: '文二西路' }, ]; - // const rowKeys = data.map(item => item.key); - - // const rowSpanList = getRowSpan(data.map(item => item.a));Also applies to: 30-33
41-56
: 清理注释的代码块
onCell
回调中包含大量注释代码。如果这些代码不再需要,建议删除以提高代码可读性。onCell: (_, index) => { - // const { rowSpan = 1 } = rowSpanList[index]; - // const props: React.TdHTMLAttributes<HTMLTableCellElement> = {}; - // props.rowSpan = rowSpan; - // if (rowSpan >= 1) { - // let currentRowSpan = rowSpan; - // for (let i = index; i < index + rowSpan; i += 1) { - // const rowKey = rowKeys[i]; - // if (expandedRowKeys.includes(rowKey)) { - // currentRowSpan += 1; - // } - // } - // props.rowSpan = currentRowSpan; - // } - // return props; - if (index === 1) { return { rowSpan: 2, }; } else if (index === 2) { return { rowSpan: 0, }; } },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/demo/expandedSticky.md
(1 hunks)docs/examples/expandedSticky.tsx
(1 hunks)src/Body/BodyRow.tsx
(8 hunks)src/Body/ExpandedRow.tsx
(3 hunks)src/Body/index.tsx
(3 hunks)src/Table.tsx
(4 hunks)src/VirtualTable/VirtualCell.tsx
(1 hunks)src/context/TableContext.tsx
(3 hunks)src/hooks/useColumns/index.tsx
(4 hunks)src/hooks/useFlattenRecords.ts
(3 hunks)src/interface.ts
(1 hunks)tests/ExpandedOffset.spec.tsx
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/hooks/useColumns/index.tsx (1)
src/index.ts (1)
EXPAND_COLUMN
(19-19)
src/context/TableContext.tsx (1)
src/interface.ts (1)
ExpandableConfig
(240-260)
src/hooks/useFlattenRecords.ts (1)
src/interface.ts (1)
Key
(21-21)
src/Body/BodyRow.tsx (1)
src/interface.ts (1)
Key
(21-21)
🪛 GitHub Check: codecov/patch
tests/ExpandedOffset.spec.tsx
[warning] 73-74: tests/ExpandedOffset.spec.tsx#L73-L74
Added lines #L73 - L74 were not covered by tests
🔇 Additional comments (14)
src/interface.ts (1)
259-259
: 接口扩展实现良好新增的
expandedRowOffset
属性设计合理,使用可选属性保持了向后兼容性,类型定义也很恰当。src/hooks/useFlattenRecords.ts (2)
14-14
: 优化性能的好改进将 key 计算提取到变量中,避免了重复计算,提高了性能。
20-20
: rowKey 属性添加一致且完整在
FlattenData
接口和所有实现路径中都正确添加了rowKey
属性,确保了数据结构的一致性,这为后续的展开行偏移功能提供了良好的基础。Also applies to: 45-45, 85-85
src/VirtualTable/VirtualCell.tsx (1)
59-59
:❓ Verification inconclusive
需要明确 TODO 项的实现计划
这个 TODO 注释表明虚拟表格还不支持
expandableRowOffset
功能。建议:
- 如果这是计划中的后续工作,请在相关 issue 中跟踪
- 如果应该在本 PR 中实现,请补充相应的功能
- 如果是已知限制,建议在文档中说明
请运行以下脚本检查是否还有其他地方存在类似的未实现:
🏁 Script executed:
#!/bin/bash # 查找代码中与 expandedRowOffset 相关的 TODO 或 FIXME 注释 rg -i "todo.*expand|fixme.*expand|expand.*todo|expand.*fixme" --type ts --type tsx -A 2 -B 2Length of output: 122
我们需要确认是否在其他地方也留下了相同的 TODO/FIXME 注释,请执行以下脚本:
🏁 Script executed:
#!/bin/bash # 查找代码中与 expandableRowOffset 相关的 TODO 或 FIXME 注释 rg -i "todo.*expand|fixme.*expand|expandableRowOffset" -g "*.ts" -g "*.tsx" -A2 -B2Length of output: 457
需要明确
expandableRowOffset
功能的实现计划在
src/VirtualTable/VirtualCell.tsx
的第 59 行存在// TODO: support `expandableRowOffset`
目前该功能尚未实现。请确认并说明:
- 如果属于后续版本的开发计划,请在对应的 issue 中跟踪(并在 PR 描述中添加链接)
- 如果应在本 PR 中完成,请补充该功能的实现代码
- 如果这是已知限制,请在组件文档或变更日志中明确说明
经全仓检索,暂无其他与
expandableRowOffset
相关的 TODO/FIXME 注释。docs/demo/expandedSticky.md (1)
1-9
: 文档结构规范清晰新增的演示文档遵循了项目的文档规范,frontmatter 配置正确,代码引用路径合理,为新功能提供了良好的演示说明。
src/Body/ExpandedRow.tsx (2)
17-17
: 接口扩展正确添加可选的
stickyOffset
属性到ExpandedRowProps
接口,类型定义合理。
34-34
: 粘性定位逻辑实现正确
stickyOffset
的默认值设置和在样式计算中的使用都很合理:
- 宽度计算正确减去了粘性偏移量
- 左侧位置设置为粘性偏移量值
这确保了展开行内容与粘性列的正确对齐。
Also applies to: 49-51
src/Table.tsx (2)
873-873
: 上下文值扩展正确正确地将
expandedRowOffset
和colWidths
添加到 TableContext 值中,为其他组件提供了访问这些属性的能力。Also applies to: 884-884
925-925
: 依赖数组更新正确正确地将新属性添加到 useMemo 的依赖数组中,确保当这些值发生变化时能够正确重新计算上下文值。
Also applies to: 935-935
src/context/TableContext.tsx (2)
6-6
: 类型导入正确添加
ExpandableConfig
导入以支持新的expandedRowOffset
属性类型定义。
65-65
: 上下文接口扩展合理正确扩展了
TableContextProps
接口:
colWidths
定义为number[]
类型,符合列宽度数组的预期expandedRowOffset
使用ExpandableConfig
的对应属性类型,保持了类型安全性Also applies to: 79-79
tests/ExpandedOffset.spec.tsx (1)
41-90
: 测试用例设计合理测试很好地验证了展开行偏移与粘性列的集成功能:
- 正确模拟了 DOM 属性和布局条件
- 验证了展开行单元格的
colspan
属性- 确认了粘性定位样式的正确性
测试覆盖了关键的功能点,符合预期。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 73-74: tests/ExpandedOffset.spec.tsx#L73-L74
Added lines #L73 - L74 were not covered by testssrc/hooks/useColumns/index.tsx (1)
107-107
: 代码实现正确!
expandedRowOffset
参数的添加和相关逻辑实现合理。在展开行时,将偏移量之前的列自动设置为固定列,这样可以确保展开内容与表格列正确对齐。依赖数组也正确更新了。Also applies to: 132-133, 225-234, 243-243
src/Body/index.tsx (1)
74-87
: 展开行信息计算逻辑正确!这段代码正确计算了展开行所需的信息:
colSpan
:展开行应该跨越的列数(总列数减去偏移量)sticky
:粘性定位的起始位置(偏移列的总宽度)使用
|| 0
处理可能的 undefined 列宽值是个好的防御性编程实践。
triggerResize(coll as HTMLElement); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
测试覆盖率提醒
静态分析工具指出这些行没有被测试覆盖。由于这是条件代码块,可能在特定场景下才会执行。
🏁 Script executed:
#!/bin/bash
# 检查测试覆盖率和执行路径
echo "搜索类似的测试模式以了解覆盖率问题:"
rg -A 3 -B 3 "triggerResize.*coll" --type ts
Length of output: 1070
补充 triggerResize 分支的测试覆盖
静态分析显示 triggerResize(coll as HTMLElement)
在以下文件的测试中未被执行:
- tests/ExpandedOffset.spec.tsx(约第73–74行)
- tests/FixedColumn.spec.tsx(相似位置)
当前测试并未插入带有 .rc-table-resize-collection
的元素,导致 if (coll)
分支从未触发。
建议在对应 spec 中向容器中渲染一个带 .rc-table-resize-collection
类的元素,并在 act
中调用 triggerResize
,以确保该分支被覆盖并校验其行为。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 73-74: tests/ExpandedOffset.spec.tsx#L73-L74
Added lines #L73 - L74 were not covered by tests
🤖 Prompt for AI Agents
In tests/ExpandedOffset.spec.tsx around lines 73 to 74, the call to
triggerResize(coll as HTMLElement) is not covered by tests because the condition
checking for an element with class .rc-table-resize-collection is never true. To
fix this, add a test case that renders an element with the class
.rc-table-resize-collection inside the container, then use React's act to call
triggerResize on that element. This will ensure the conditional branch is
executed and its behavior validated, improving test coverage.
if (expandedRowOffset) { | ||
const { rowSpan = 1 } = additionalCellProps; | ||
|
||
// For expandable row with rowSpan, | ||
// We should increase the rowSpan if the row is expanded | ||
if (expandable && rowSpan && colIndex < expandedRowOffset) { | ||
let currentRowSpan = rowSpan; | ||
|
||
for (let i = index; i < index + rowSpan; i += 1) { | ||
const rowKey = rowKeys[i]; | ||
if (expandedKeys.has(rowKey)) { | ||
currentRowSpan += 1; | ||
} | ||
} | ||
additionalCellProps.rowSpan = currentRowSpan; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
建议添加数组边界检查
在第 98 行访问 rowKeys[i]
时,应该确保索引不超出数组范围,以避免潜在的运行时错误。
// Expandable row has offset
if (expandedRowOffset) {
const { rowSpan = 1 } = additionalCellProps;
// For expandable row with rowSpan,
// We should increase the rowSpan if the row is expanded
if (expandable && rowSpan && colIndex < expandedRowOffset) {
let currentRowSpan = rowSpan;
for (let i = index; i < index + rowSpan; i += 1) {
+ if (i >= rowKeys.length) {
+ break;
+ }
const rowKey = rowKeys[i];
if (expandedKeys.has(rowKey)) {
currentRowSpan += 1;
}
}
additionalCellProps.rowSpan = currentRowSpan;
}
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (expandedRowOffset) { | |
const { rowSpan = 1 } = additionalCellProps; | |
// For expandable row with rowSpan, | |
// We should increase the rowSpan if the row is expanded | |
if (expandable && rowSpan && colIndex < expandedRowOffset) { | |
let currentRowSpan = rowSpan; | |
for (let i = index; i < index + rowSpan; i += 1) { | |
const rowKey = rowKeys[i]; | |
if (expandedKeys.has(rowKey)) { | |
currentRowSpan += 1; | |
} | |
} | |
additionalCellProps.rowSpan = currentRowSpan; | |
} | |
} | |
if (expandedRowOffset) { | |
const { rowSpan = 1 } = additionalCellProps; | |
// For expandable row with rowSpan, | |
// We should increase the rowSpan if the row is expanded | |
if (expandable && rowSpan && colIndex < expandedRowOffset) { | |
let currentRowSpan = rowSpan; | |
for (let i = index; i < index + rowSpan; i += 1) { | |
if (i >= rowKeys.length) { | |
break; | |
} | |
const rowKey = rowKeys[i]; | |
if (expandedKeys.has(rowKey)) { | |
currentRowSpan += 1; | |
} | |
} | |
additionalCellProps.rowSpan = currentRowSpan; | |
} | |
} |
🤖 Prompt for AI Agents
In src/Body/BodyRow.tsx around lines 89 to 105, the code accesses rowKeys[i]
without checking if i is within the bounds of the rowKeys array, which can cause
runtime errors. Add a boundary check inside the for loop to ensure i is less
than rowKeys.length before accessing rowKeys[i]. This prevents out-of-range
access and potential crashes.
ref #1283 for master branch
Summary by CodeRabbit
新功能
文档
测试