-
-
Notifications
You must be signed in to change notification settings - Fork 611
expanded&sticky support expandedRowOffset #1281
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
@crazyair is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
Walkthrough本次变更引入了对表格可展开行的 expandedRowOffset 支持,调整了相关组件的属性和渲染逻辑,并增强了 sticky 滚动条的滚动事件监听机制。同时,新增了一个带有 sticky 和可展开行的示例文档,并更新了工作流配置和部分依赖脚本。 Changes
Sequence Diagram(s)sequenceDiagram
participant 用户
participant Demo组件
participant Table
participant Body
participant BodyRow
participant ExpandedRow
用户->>Demo组件: 渲染 sticky+可展开行表格
Demo组件->>Table: 传递 expandedRowOffset 配置
Table->>Body: 传递 expandedRowOffset
Body->>BodyRow: 渲染每一行,传递 expandedRowOffset
BodyRow->>ExpandedRow: 渲染展开行,计算 offsetWidth 并传递
ExpandedRow->>ExpandedRow: 使用 offsetWidth 调整内容偏移
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/MeasureCell.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/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.
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. ✨ 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 (
|
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: 1
🧹 Nitpick comments (2)
src/Body/BodyRow.tsx (1)
189-195
: 偏移宽度计算逻辑基本正确,但存在小的冗余。计算逻辑是正确的,但第193行中
item.width ?? 0
是多余的,因为前面已经通过typeof item.width === 'number'
确保了item.width
是数字类型。建议简化这行代码:
- offsetWidth = offsetWidth + (item.width ?? 0); + offsetWidth = offsetWidth + item.width;docs/examples/expandedSticky.tsx (1)
40-55
: 单元格合并逻辑正确但可以优化可读性
onCell
函数中的逻辑正确地处理了展开行对 rowSpan 的影响,但代码可读性可以提升。考虑提取逻辑到单独的函数以提高可读性:
+const calculateRowSpanWithExpanded = ( + index: number, + originalRowSpan: number, + expandedRowKeys: readonly React.Key[], + rowKeys: string[] +) => { + let currentRowSpan = originalRowSpan; + for (let i = index; i < index + originalRowSpan; i += 1) { + const rowKey = rowKeys[i]; + if (expandedRowKeys.includes(rowKey)) { + currentRowSpan += 1; + } + } + return currentRowSpan; +}; 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; + props.rowSpan = calculateRowSpanWithExpanded(index, rowSpan, expandedRowKeys, rowKeys); + } else { + props.rowSpan = rowSpan; } return props; },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
tests/__snapshots__/FixedColumn.spec.tsx.snap
is excluded by!**/*.snap
tests/__snapshots__/Table.spec.jsx.snap
is excluded by!**/*.snap
📒 Files selected for processing (13)
.github/workflows/main.yml
(1 hunks)docs/demo/expandedSticky.md
(1 hunks)docs/examples/expandedSticky.tsx
(1 hunks)package.json
(2 hunks)src/Body/BodyRow.tsx
(5 hunks)src/Body/ExpandedRow.tsx
(3 hunks)src/Body/MeasureCell.tsx
(2 hunks)src/Body/MeasureRow.tsx
(2 hunks)src/Body/index.tsx
(3 hunks)src/Table.tsx
(7 hunks)src/context/TableContext.tsx
(2 hunks)src/interface.ts
(1 hunks)src/stickyScrollBar.tsx
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/context/TableContext.tsx (1)
src/interface.ts (1)
ExpandableConfig
(236-256)
src/Body/BodyRow.tsx (1)
src/interface.ts (1)
ExpandableConfig
(236-256)
docs/examples/expandedSticky.tsx (2)
src/interface.ts (2)
Key
(21-21)ColumnType
(105-120)tests/FixedColumn-IE.spec.jsx (1)
columns
(28-41)
🔇 Additional comments (31)
package.json (2)
3-3
: 版本更新看起来合理版本从 7.50.2 升级到 7.50.5,这与引入
expandedRowOffset
功能和其他改进相符。
45-45
: 增加 --any-branch 标志提高了发布灵活性添加
--any-branch
标志允许从任何 git 分支进行发布,而不仅限于默认分支,这为发布流程提供了更多灵活性。src/interface.ts (1)
255-255
: 良好的接口扩展设计新增的
expandedRowOffset
属性为可展开行提供了偏移量配置选项。该属性是可选的,保持了向后兼容性,类型定义也很合适。.github/workflows/main.yml (1)
5-5
: 工作流更新与包管理策略一致将工作流引用从
test.yml@main
更改为test-npm.yml@main
,这与 package.json 中的更改保持一致,使 CI 流水线与新的包管理和测试策略保持同步。src/stickyScrollBar.tsx (3)
10-10
: 导入 getDOM 用于新的祖先元素遍历新增的
getDOM
导入是为了支持后续的祖先元素遍历逻辑,用于收集所有可滚动的父元素。
96-97
: 优化动画帧取消逻辑将
raf.cancel
移动到checkScrollBarVisible
函数开始处,这样可以在调度新的动画帧之前取消任何待处理的帧,避免冗余调用,提高性能。
150-172
: 显著改进的滚动事件处理机制新的实现通过遍历 DOM 树收集所有祖先元素,并为每个元素添加滚动事件监听器,同时也监听窗口的 resize 和 scroll 事件。这种方法大大改善了在嵌套滚动上下文中的响应性,确保粘性滚动条能够准确响应各级滚动变化。
清理逻辑也很完善,正确移除了所有添加的事件监听器。
docs/demo/expandedSticky.md (1)
1-9
: 文档结构正确,符合约定文档的结构和元数据配置看起来正确,为新的 expandedRowOffset 功能提供了适当的示例文档入口。
src/Body/index.tsx (3)
34-35
: 正确添加了 expandedRowOffset 上下文支持从 TableContext 中正确提取了 expandedRowOffset 属性,遵循了现有的模式。
44-45
: 依赖数组配置正确将 expandedRowOffset 添加到依赖数组中,确保上下文优化正常工作。
79-79
: 属性传递实现正确将 expandedRowOffset 正确传递给 BodyRow 组件,使其能够在行渲染中使用此偏移配置。
src/Body/MeasureCell.tsx (2)
3-3
: 导入 useLayoutEffect 提升性能使用 rc-util 的 useLayoutEffect 替代标准的 useEffect,这是正确的优化。
13-17
: 使用 useLayoutEffect 改善布局测量时机将 useEffect 替换为 useLayoutEffect 确保列宽度测量在 DOM 变更后、浏览器绘制前同步执行,这为 expandedRowOffset 等需要精确测量的功能提供了更准确的布局计算。
src/context/TableContext.tsx (2)
6-6
: 正确导入 ExpandableConfig 类型从 interface 模块导入 ExpandableConfig 类型,为新的 expandedRowOffset 属性提供类型支持。
53-53
: 类型安全的属性定义使用索引访问类型
ExpandableConfig<RecordType>['expandedRowOffset']
定义 expandedRowOffset 属性,确保类型安全并与 interface.ts 中的定义保持一致。src/Body/MeasureRow.tsx (3)
4-4
: 新增导入看起来合理。新增的
isVisible
导入用于在后续代码中检查测量行的可见性,这是必要的依赖。
13-13
: React ref 实现正确。正确添加了对测量行元素的引用,类型定义准确且使用方式合理。
Also applies to: 20-20
24-28
: 可见性检查优化合理。将列宽调整回调的触发限制在测量行可见时,这是很好的性能优化。这个变更与
Table.tsx
中移除表格级别可见性检查的变更形成了良好的互补,将检查逻辑下沉到更具体的组件层级。src/Body/ExpandedRow.tsx (2)
17-17
: 接口扩展设计合理。新增的
offsetWidth
属性是可选的且有合理的默认值,保持了向后兼容性。类型定义清晰准确。Also applies to: 34-34
48-48
: 宽度计算逻辑正确。在 sticky 包装器宽度计算中正确地减去了偏移宽度,确保展开行内容能够准确对齐到表格布局。计算顺序合理,支持了
expandedRowOffset
功能的实现。src/Table.tsx (4)
50-50
: 导入变更合理。新增的
useLayoutEffect
导入用于同步布局更新,useTimeoutLock
导入支持了重构。这些变更与代码逻辑的改进保持一致。Also applies to: 78-78
352-352
: 状态管理简化合理。使用标准的
useState
替代之前的useLayoutState
简化了代码。onColumnResize
回调中移除可见性检查是合理的,因为检查逻辑已下沉到MeasureRow
组件。新的实现只在宽度实际变化时更新状态,效率更高。Also applies to: 405-414
525-525
: 使用 useLayoutEffect 更合适。对于滚动条大小和 sticky 支持的设置,使用
useLayoutEffect
确保在浏览器绘制前同步执行,可以避免布局闪烁问题。这是正确的优化。
825-825
: 上下文传递实现正确。正确地将
expandedRowOffset
从expandableConfig
传递到表格上下文中,并在依赖数组中包含该值。这确保了子组件能够访问到这个配置,是完整功能实现的必要部分。Also applies to: 876-876
src/Body/BodyRow.tsx (2)
7-7
: 接口扩展实现正确。正确导入了类型定义,新增的
expandedRowOffset
属性使用了适当的泛型类型,并设置了合理的默认值。接口设计保持了向后兼容性。Also applies to: 22-22, 106-106
208-209
: 属性传递实现正确。正确地将计算出的
offsetWidth
传递给ExpandedRow
组件,并相应地调整了colSpan
。这确保了展开行能够正确对齐并且不会占据被偏移列的空间。docs/examples/expandedSticky.tsx (5)
1-4
: 导入语句正确且合适导入语句结构良好,包含了必要的 React hooks、类型定义、组件和样式文件。
23-23
: 状态管理类型定义正确使用
readonly React.Key[]
类型定义展开行状态是正确的做法,符合 React 的最佳实践。
25-32
: 测试数据结构合理测试数据包含了用于演示单元格合并的重复值(张三),数据结构简洁且适合演示目的。
70-75
: 新的 expandedRowOffset 功能演示正确
expandable
配置正确演示了新的expandedRowOffset
功能,设置为 1 表示展开行向右偏移一列,这与表格的列结构(固定列 + 展开列 + 普通列)相符。
62-78
: 容器样式和表格配置合适高度设置为 10000px 有效演示了 sticky 功能,表格配置包含了所有必要的属性来展示 sticky 列和展开行的组合效果。
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(); | ||
}; |
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.
修复数组变异的副作用问题
getRowSpan
函数存在副作用问题:直接调用 source.reverse()
会修改原始数组,这可能导致意外的行为。
应用以下修改来避免副作用:
-export const getRowSpan = (source: (string | number | undefined)[] = []) => {
+export const getRowSpan = (source: (string | number | undefined)[] = []) => {
const list: { rowSpan?: number }[] = [];
let span = 0;
- source.reverse().forEach((key, index) => {
+ const reversedSource = [...source].reverse();
+ reversedSource.forEach((key, index) => {
span = span + 1;
- if (key !== source[index + 1]) {
+ if (key !== reversedSource[index + 1]) {
list.push({ rowSpan: span });
span = 0;
} else {
list.push({ rowSpan: 0 });
}
});
return list.reverse();
};
📝 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.
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(); | |
}; | |
export const getRowSpan = (source: (string | number | undefined)[] = []) => { | |
const list: { rowSpan?: number }[] = []; | |
let span = 0; | |
const reversedSource = [...source].reverse(); | |
reversedSource.forEach((key, index) => { | |
span = span + 1; | |
if (key !== reversedSource[index + 1]) { | |
list.push({ rowSpan: span }); | |
span = 0; | |
} else { | |
list.push({ rowSpan: 0 }); | |
} | |
}); | |
return list.reverse(); | |
}; |
🤖 Prompt for AI Agents
In docs/examples/expandedSticky.tsx around lines 7 to 20, the getRowSpan
function mutates the original source array by calling source.reverse(), causing
side effects. To fix this, create a shallow copy of the source array before
reversing it, for example by using source.slice().reverse(), so the original
array remains unchanged and side effects are avoided.
Summary by CodeRabbit
新功能
修复与优化
文档
杂项