-
-
Notifications
You must be signed in to change notification settings - Fork 623
docs: example sortable columns #1415
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
base: master
Are you sure you want to change the base?
docs: example sortable columns #1415
Conversation
|
@FatahChan is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
Summary of ChangesHello @FatahChan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the demo section by introducing a modern, interactive column sorting example with drag-and-drop capabilities. Concurrently, it updates the codebase by refactoring an existing column resizing demo to utilize contemporary React hooks, aligning with current best practices and improving overall demo interactivity and code maintainability. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough新增列拖拽排序示例与文档,重构列宽调整示例为函数式组件,并在 devDependencies 中加入 dnd-kit 相关包以支持拖拽;未更改公共 API 声明。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as 用户
participant UI as 表格 UI
participant Header as 列头 (SortableHeaderCell)
participant DndKit as dnd-kit (DndContext)
participant Demo as Demo 组件
Note over User,Header: 用户在列头上开始拖拽
User->>Header: 拖动列头 (drag start)
Header->>DndKit: 触发 dnd-kit 事件
DndKit->>Demo: 传递 dragEnd 事件与 id/索引
Demo->>Demo: 计算新的列顺序 (arrayMove)
Demo->>UI: 更新 columns 状态并重新渲染
UI->>User: 显示重新排序后的列
Estimated code review effort🎯 3 (中等) | ⏱️ ~20 分钟
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. Comment |
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.
Code Review
This pull request introduces a new sortable columns demo and refactors the column resizing demo to use React hooks. The refactoring is a good improvement, but it seems to have missed a required height prop on the Resizable component. The new sortable columns example is a great addition, but it contains a critical bug in the drag-and-drop logic due to an inconsistency between key and dataIndex usage for identifying columns. I've also pointed out a few other areas for improvement in the new example, such as props not being passed correctly and lack of typing. The dependency changes in package.json look correct.
…improved functionality
…dex for operations column
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/demo/column-sortable.md(1 hunks)docs/examples/column-resize.tsx(1 hunks)docs/examples/column-sortable.tsx(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-08T12:53:09.293Z
Learnt from: bbb169
Repo: react-component/table PR: 1202
File: src/Table.tsx:903-904
Timestamp: 2024-11-08T12:53:09.293Z
Learning: 在 `src/Table.tsx` 文件的 React 组件 `Table` 中,即使 `bodyScrollLeft` 频繁更新,也需要在 `TableContextValue` 的 `useMemo` 依赖数组中包含 `bodyScrollLeft` 和 `headerCellRefs`,因为每次滚动时重新计算 `TableContextValue` 是解决该问题所必须的。
Applied to files:
docs/examples/column-sortable.tsxdocs/examples/column-resize.tsx
🧬 Code graph analysis (1)
docs/examples/column-resize.tsx (1)
src/interface.ts (2)
ColumnsType(135-138)ColumnType(118-133)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (1)
docs/examples/column-sortable.tsx (1)
91-92: 删除 JSX 中多余的分号这里的
<Table {...tableProps} />;会把分号当作文本节点渲染在页面上,示例内容尾部会多出一个;。去掉它即可。- <Table {...tableProps} />; + <Table {...tableProps} /><details> <summary>⛔ Skipped due to learnings</summary>Learnt from: bbb169
Repo: react-component/table PR: 1202
File: src/Table.tsx:903-904
Timestamp: 2024-11-08T12:53:09.293Z
Learning: 在src/Table.tsx文件的 React 组件Table中,即使bodyScrollLeft频繁更新,也需要在TableContextValue的useMemo依赖数组中包含bodyScrollLeft和headerCellRefs,因为每次滚动时重新计算TableContextValue是解决该问题所必须的。</details> </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
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
♻️ Duplicate comments (2)
docs/examples/column-sortable.tsx (2)
10-25: 为SortableHeaderCell增加显式类型声明当前
SortableHeaderCell的props为any,在 TS 示例里不利于提示和约束。可以考虑为它声明更明确的类型(例如基于React.ThHTMLAttributes<HTMLTableCellElement>并扩展 dnd 所需字段),方便后续读者直接复制使用。
34-47: 为columns添加ColumnsType泛型以提升类型安全这里的
columns状态没有显式类型,推断为宽松的any[]。建议参考column-resize示例,显式使用ColumnsType<(typeof data)[number]>,这样 TS 能更好校验dataIndex/key/width等字段,也便于读者照抄:- const [columns, setColumns] = useState([ + const [columns, setColumns] = + useState<import('rc-table').ColumnsType<(typeof data)[number]>>([
🧹 Nitpick comments (3)
docs/examples/column-sortable.tsx (1)
48-76: 拖拽 id 与dataIndex的耦合可适当抽象,避免未来扩展示例时踩坑当前拖拽 id 在三处手动依赖
dataIndex:
onHeaderCell中通过col.dataIndex?.toString()生成idSortableContext.items使用columns.map(col => col.dataIndex?.toString())handleDragEnd中用col.dataIndex === active.id/over.id查找索引在本示例下是工作正常的,但一旦后续示例中某些列使用数组形式的
dataIndex(如['user', 'name'])或key !== dataIndex,这三处就容易再次出现不一致的问题。建议抽一个小工具函数统一计算列 id,例如:const getColumnId = (col: (typeof columns)[number]) => col.dataIndex?.toString() ?? '';然后在
onHeaderCell、SortableContext.items和handleDragEnd中都通过getColumnId来对比,避免未来改动dataIndex结构时出现隐蔽 bug。docs/examples/column-resize.tsx (2)
7-18:width的判空建议区分0与undefined
ResizableTitle中用if (!width)判断是否启用可拖拽,这会把width === 0也当成「无宽度」处理,将直接返回普通<th>。当前示例列宽都大于 0,因此没有问题,但如果后续有人把某列初始宽度设为0,这一列会意外变得不可拖拽。可以改为只在
width == null时禁用:- if (!width) { + if (width == null) { return <th {...restProps} />; }
42-50:handleResize更符合直觉的写法是使用useCallback(当前逻辑正确,仅风格建议)目前通过
useMemo返回一个函数工厂同样可以保证引用稳定,但对大部分 React 开发者来说,用useCallback表达「稳定回调」的语义更直观。可以按需(或在后续重构时)调整为:- const handleResize = useMemo(() => { - return (index: number) => { - return (_: React.MouseEvent<HTMLTableCellElement>, { size }: { size: { width: number } }) => { - setColumns(prevColumns => - prevColumns.map((col, i) => (i === index ? { ...col, width: size.width } : col)), - ); - }; - }; - }, []); + const handleResize = React.useCallback( + (index: number) => + (_: React.MouseEvent<HTMLTableCellElement>, { size }: { size: { width: number } }) => { + setColumns(prevColumns => + prevColumns.map((col, i) => (i === index ? { ...col, width: size.width } : col)), + ); + }, + [], + );功能等价,可读性略好一些。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/examples/column-resize.tsx(1 hunks)docs/examples/column-sortable.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-08T12:53:09.293Z
Learnt from: bbb169
Repo: react-component/table PR: 1202
File: src/Table.tsx:903-904
Timestamp: 2024-11-08T12:53:09.293Z
Learning: 在 `src/Table.tsx` 文件的 React 组件 `Table` 中,即使 `bodyScrollLeft` 频繁更新,也需要在 `TableContextValue` 的 `useMemo` 依赖数组中包含 `bodyScrollLeft` 和 `headerCellRefs`,因为每次滚动时重新计算 `TableContextValue` 是解决该问题所必须的。
Applied to files:
docs/examples/column-resize.tsxdocs/examples/column-sortable.tsx
🧬 Code graph analysis (1)
docs/examples/column-resize.tsx (1)
src/interface.ts (2)
ColumnsType(135-138)ColumnType(118-133)
🔇 Additional comments (1)
docs/examples/column-resize.tsx (1)
20-69: 整体列宽类型、数据建模与 Hook 重构实现良好
data/RecordType的建模、ColumnsType<RecordType>的使用以及columnsWithResizable/tableProps的useMemo拆分都比较清晰,状态更新也保持了不可变更新风格,适合作为文档示例代码。
… cell IDs and sortable items
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1415 +/- ##
=======================================
Coverage 96.09% 96.09%
=======================================
Files 57 57
Lines 3434 3434
Branches 632 632
=======================================
Hits 3300 3300
Misses 129 129
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This pull request introduces a new column sorting demo using drag-and-drop, refactors the column resizing demo to use React hooks, and adds the necessary dependencies for drag-and-drop functionality. The main focus is on improving demo interactivity and modernizing the codebase.
New Features:
docs/examples/column-sortable.tsx, using the@dnd-kitlibrary for interactive column reordering.docs/demo/column-sortable.md.Refactoring and Improvements:
docs/examples/column-resize.tsxfrom a class component to a functional component using React hooks, improving readability and maintainability. [1] [2]Dependency Updates:
@dnd-kit/core,@dnd-kit/modifiers, and@dnd-kit/sortabletopackage.jsonto support drag-and-drop functionality in demos.Summary by CodeRabbit
发布说明
新功能
文档
优化 / 重构
杂项
✏️ Tip: You can customize this high-level summary in your review settings.