-
-
Notifications
You must be signed in to change notification settings - Fork 623
fix: Due to precision issues, scrollLeft cannot reach the maximum scrollable width, causing the shadow to remain visible #1404
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
…ollable width, causing the shadow to remain visible
|
Someone is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
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调整表格固定列右侧阴影的显示阈值:在 Cell 组件中将 end-fixed 阴影判断从 >=1 改为 >1(即需严格大于 1),在 Table 组件中将横向滚动判断向内收 1 像素(absScrollStart < scrollWidth - clientWidth - 1)。 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 用户
participant Table as Table 组件
participant Cell as Cell 单元格渲染
note over User,Table `#f3f4f6`: 用户横向滚动表格
User->>Table: 触发 scroll 事件 (absScrollStart)
Table->>Table: 计算是否显示右侧固定列阴影\n检查 absScrollStart < scrollWidth - clientWidth - 1
alt 显示 end-shadow
Table->>Cell: 传递 end-shadow 可见性
Cell->>Cell: 计算 offsetFixedEndShadow (> 1)
Cell->>Table: 渲染带阴影的固定列
else 隐藏 end-shadow
Table->>Cell: 传递 end-shadow 不可见
Cell->>Table: 渲染无阴影的固定列
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
Summary of ChangesHello @codewizardTM, 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 resolves a persistent visual bug in tables where fixed column shadows would incorrectly remain visible even when the table was scrolled to its absolute end. The root cause was identified as floating-point precision limitations in 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
|
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 addresses a bug where fixed column shadows remain visible due to floating-point precision issues in scroll position calculations. The fix introduces a buffer when checking if the scroll has reached the end. The change in Table.tsx seems correct. However, I've noticed a potential inconsistency in the threshold used in Cell/index.tsx compared to Table.tsx, which could lead to visual discrepancies. My review includes a comment to address this.
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: 0
🧹 Nitpick comments (2)
src/Table.tsx (1)
510-510: 修复精度问题的正确方案此改动通过减去 1 像素来容忍浮点数精度误差,当滚动位置在最大值 1 像素范围内时隐藏阴影。这与
src/Cell/index.tsx第 167 行的改动(从>= 1改为>= 2)协调一致,有效解决了阴影始终可见的问题。可选改进:考虑将魔法数字
1提取为命名常量(如SCROLL_PRECISION_THRESHOLD)并添加注释说明精度问题,以提高代码可维护性:+// Threshold to accommodate floating-point precision issues in scroll position +const SCROLL_PRECISION_THRESHOLD = 1; + const onInternalScroll = useEvent( ({ currentTarget, scrollLeft }: { currentTarget: HTMLElement; scrollLeft?: number }) => { // ... setShadowStart(absScrollStart > 0); - setShadowEnd(absScrollStart < scrollWidth - clientWidth - 1); + setShadowEnd(absScrollStart < scrollWidth - clientWidth - SCROLL_PRECISION_THRESHOLD); } );src/Cell/index.tsx (1)
167-167: 修复精度问题的阈值调整将阈值从
>= 1改为>= 2使阴影在距离右侧边缘 2 像素以内时隐藏,有效解决了精度问题导致的阴影残留。这与src/Table.tsx第 510 行的改动协同工作。可选说明:注意到
Table.tsx使用 1 像素阈值而此处使用 2 像素,可能因为此处需要考虑offsetFixedEndShadow的额外影响。如果方便的话,可以在注释中说明两个阈值不同的原因,或考虑提取为共享常量:const showEndShadow = (isFixEnd && fixedEndShadow && scrollWidth - absScroll) - - // Same as above + // Use threshold of 2 to account for offsetFixedEndShadow and precision issues (offsetFixedEndShadow as number) >= 2;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Cell/index.tsx(1 hunks)src/Table.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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` 是解决该问题所必须的。
📚 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:
src/Table.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1404 +/- ##
=======================================
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 is a ...
🔗 Related Issues
fix ant-design/ant-design#55310
💡 Background and Solution
In some cases, due to precision issues, the maximum value of the scroll container’s scrollLeft may still be less than scrollWidth - clientWidth.


To handle this, we subtract 1 from scrollWidth - clientWidth to smooth out the precision discrepancy.
Before:
After
📝 Change Log
Summary by CodeRabbit
发布说明