-
-
Notifications
You must be signed in to change notification settings - Fork 237
fix: popup container scroll due to precision #602
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
fix: popup container scroll due to precision #602
Conversation
Walkthrough在对齐计算中,当 popup 在水平方向或垂直方向未缩放(scaleX === 1 / scaleY === 1)时,将用于计算偏移的两处数值取整从四舍五入(Math.round)改为向下取整(Math.floor);相应测试断言已更新以验证向下取整行为。 Changes
Sequence Diagram(s)(此变更为小范围取整行为调整,未引入新的跨组件控制流,故省略序列图。) Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 分钟 Possibly related PRs
Suggested reviewers
诗
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 @aojunhao123, 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! 此拉取请求旨在解决Ant Design组件库中一个由浮层定位精度问题引起的UI抖动bug。该问题主要影响Tooltip和FloatButton等组件,在特定屏幕尺寸和交互模式下,由于水平偏移量计算不精确,导致容器出现不必要的滚动条,从而造成视觉上的闪烁。通过调整核心对齐逻辑中的舍入方法,确保浮层始终在预期范围内渲染,从而提升了用户体验和组件的稳定性。 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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
你好,感谢你的贡献。这个 PR 旨在修复一个由于浮点数精度问题导致的弹窗容器出现滚动条,进而引发无限循环抖动的 bug。你通过将 Math.round 修改为 Math.floor 来处理 X 轴上的偏移量,确保计算结果不会因为向上取整而超出容器宽度,这是一个很直接且有效的解决方案。
我在代码中发现,Y 轴方向的偏移量计算仍然在使用 Math.round,这可能会在特定情况下导致类似的高度溢出问题。我在具体的代码行上留下了评论,建议为了逻辑一致性和健壮性,将 Y 轴也统一使用 Math.floor 处理。
总体来说,这是一个高质量的修复,很好地解决了描述中的问题。合并前请考虑一下我提出的建议。
| nextOffsetX = Math.round(nextOffsetX); | ||
| offsetX4Right = Math.round(offsetX4Right); | ||
| nextOffsetX = Math.floor(nextOffsetX); | ||
| offsetX4Right = Math.floor(offsetX4Right); |
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.
round改为floor我理解没什么副作用,唯一的弊端可能就是精度差异会变大,原来最多0.5px的差异,现在最大精度差可能为0.99px,但用户真能感知到吗(
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #602 +/- ##
=======================================
Coverage 97.26% 97.26%
=======================================
Files 17 17
Lines 949 949
Branches 278 278
=======================================
Hits 923 923
Misses 26 26 ☔ 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: 0
🧹 Nitpick comments (1)
src/hooks/useAlign.ts (1)
694-708:Math.floor可能在负偏移场景引入额外 1px 偏移;建议改为“向 0 截断”并对scaleX用 epsilon 判断。
Math.floor能避免正数偏移“进位”导致的越界(你们要修的点),但在nextOffsetX/offsetX4Right为负数时会把-0.2变成-1,相比之前Math.round可能产生额外 1px 的反向位移/间隙,极端情况下也可能把溢出问题从右侧挪到左侧(取决于 placement/RTL/容器 overflow)。另外scaleX === 1过于严格,某些缩放/精度下可能得到0.999/1.001从而绕过该分支。建议改法(示例)
- if (scaleX === 1) { - nextOffsetX = Math.floor(nextOffsetX); - offsetX4Right = Math.floor(offsetX4Right); - } + const isScaleXOne = Math.abs(scaleX - 1) < 1e-3; + if (isScaleXOne) { + // 向 0 截断:正数用 floor,负数用 ceil,避免负数被额外“放大” + nextOffsetX = nextOffsetX > 0 ? Math.floor(nextOffsetX) : Math.ceil(nextOffsetX); + offsetX4Right = + offsetX4Right > 0 ? Math.floor(offsetX4Right) : Math.ceil(offsetX4Right); + }建议至少回归验证:RTL、
nextOffsetX < 0的 placement、不同浏览器缩放(例如 90%/110%)、以及容器overflow: auto下是否仍会出现横向滚动条/hover 抖动。
这个bug还挺有意思的,想要触发它得满足两个条件:
Tooltip 有个贴边检测的特性,会动态计算Tooltip自身与可视区域的距离,来决定自身的渲染位置是否要做偏移或者翻转。
而这个bug的根因也来自于这个特性。
简单讲讲bug触发链路:
问题就出在第三步的Math.round上,当偏移量的小数部分>=0.5时,向上取整,得到的结果偏大,造成了Tooltip最终的渲染位置超出容器宽度,出现滚动条,于是布局抖动,不断重复mouseenter -> mouseleave的循环,产生图中疯狂抽风的问题
而Math.round是为了解决这个issue引入的,但其实也没完全解决。精度问题本身组件库层面就无法完全解掉,用户的屏幕尺寸、浏览器缩放比例都是不可控的,我通过触控板放大页面依旧能看到箭头与弹层的缝隙

总之我的方案是将round改为floor,向下取整,来避免这个情况。以下是修复后的表现:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.