Conversation
|
你好 @Alexzjt，非常感谢你的贡献. Hello, @Alexzjt, Thanks for your contribution. In order to make the code more robust, please add the corresponding unit tests, and update the docs if there are API changes. |
Summary of ChangesHello @Alexzjt, 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! 此拉取请求引入了 @antv/s2 的完整表头吸顶功能，解决了大型表格在长页面滚动时表头易丢失的问题。通过双实例架构和 DOM 代理，实现了表头在滚动时吸附在视口顶部，并支持丰富的交互操作。这一改进显著提升了用户在浏览长表格数据时的体验，确保了列信息的持续可见性和可操作性。 Highlights
Changelog
Activity
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
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #3323 +/- ##
==========================================
+ Coverage 75.77% 78.66% +2.89%
==========================================
Files 257 224 -33
Lines 11994 12485 +491
Branches 2464 2836 +372
==========================================
+ Hits 9088 9821 +733
+ Misses 1398 957 -441
- Partials 1508 1707 +199 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
本次 PR 实现了表头吸顶（Sticky Header）功能，这是一个非常实用的特性，尤其对于大数据量表格。整体实现方案采用了双 S2 实例的架构，考虑了滚动容器、交互桥接、性能优化（数据精简）等多个方面，代码质量很高。
我主要提出了几点小建议：
- 在
sticky-header/index.ts中有一处重复的 JSDoc 注释，建议移除。 - 在
sticky-header.ts示例代码中，建议移除不必要的as any类型断言和未使用的SpreadSheet导入，以提升代码的类型安全性和整洁度。
除此之外，代码逻辑清晰，考虑周全，是一次出色的功能增强。
|
Size Change: +1.98 kB (+0.27%) Total Size: 744 kB
ℹ️ View Unchanged
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This PR introduces a sticky header feature for the @antv/s2 library, which significantly enhances the user experience for large data tables by keeping the header visible during scrolling. The implementation utilizes a complex but effective dual-instance architecture, demonstrating clear code structure and robust handling of secondary instance lifecycles, state synchronization, and interaction bridging. While no security vulnerabilities were identified, and the code adheres to secure coding practices, a potential bug was found in the data minimization logic that could lead to incorrect header rendering in certain edge cases. Overall, this is a high-quality contribution.
| const values = columns.map((column) => { | ||
| const key = isString(column) | ||
| ? column | ||
| : (column as { field: string }).field; | ||
|
|
||
| return item[key]?.toString?.(); | ||
| }); | ||
|
|
||
| const cacheKey = values.join(''); | ||
|
|
||
| if (!cache[cacheKey]) { | ||
| cache[cacheKey] = item; | ||
| } |
There was a problem hiding this comment.
minimizeDataCfg 方法中生成 cacheKey 的方式存在两个问题，可能导致 cacheKey 冲突，从而使得吸顶表头渲染不正确：
item[key]?.toString?.()对null和undefined的处理结果都是undefined，这会导致它们与空字符串''等产生冲突。例如，当列值为null时，values数组中对应的项会是undefined，join之后可能产生与其他值相同的cacheKey。values.join('')没有使用分隔符，当不同列的值组合后可能产生相同的字符串。例如，两列数据['1', '23']和['12', '3']会产生相同的cacheKey"123"。
建议修改 cacheKey 的生成逻辑，使用 String() 进行类型转换并添加一个特殊的分隔符，以确保其唯一性。
const values = columns.map((column) => {
const key = isString(column)
? column
: (column as { field: string }).field;
return String(item[key]);
});
const cacheKey = values.join('\u0000');
if (!cache[cacheKey]) {
cache[cacheKey] = item;
}2 participants
👀 PR includes
✨ Feature
🎨 Enhance
🐛 Bugfix
🔧 Chore
📝 Description
背景与解决的问题：
大型表格在长页面滚动时，用户往往希望表头能够吸附在屏幕顶部，以便在向下滚动查看数据时依然能够清晰识别列的含义。本项目此前缺乏在
window/body层级滚动时的外部表头吸顶能力。主要改动点：
本 PR 为
@antv/s2带来了完整的表头吸顶 (Sticky Header) 功能及配套交互：interaction.stickyHeader，并支持自定义offsetTop（适配存在顶部导航栏的系统）。enableInteraction)：RANGE_SORT）、树节点及角头全量展开/折叠（ROW_CELL_COLLAPSED）。scroll-loop.ts)，包含 React Playground 案例支持。🖼️ Screenshot
🔗 Related issue link
🔍 Self-Check before the merge