-
Notifications
You must be signed in to change notification settings - Fork 155
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: take margin into account #291
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改涉及两个主要文件。首先,在 Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #291 +/- ##
==========================================
+ Coverage 98.48% 98.49% +0.01%
==========================================
Files 18 18
Lines 725 732 +7
Branches 180 181 +1
==========================================
+ Hits 714 721 +7
Misses 11 11 ☔ View full report in Codecov by Sentry. |
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
🧹 Outside diff range and nitpick comments (1)
src/hooks/useHeights.tsx (1)
40-47
: 优化了高度计算逻辑,但建议添加注释代码正确地计算了包含 margin 在内的总高度,这解决了原始问题。不过,建议添加注释说明计算逻辑。
建议添加如下注释:
+ // 计算元素的总高度,包括上下外边距 const { marginTop, marginBottom } = getComputedStyle(htmlElement); const marginTopNum = parseNumber(marginTop); const marginBottomNum = parseNumber(marginBottom); const totalHeight = offsetHeight + marginTopNum + marginBottomNum; + // 只在高度发生变化时更新缓存 if (heightsRef.current.get(key) !== totalHeight) { heightsRef.current.set(key, totalHeight); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
examples/switch.tsx
(1 hunks)src/hooks/useHeights.tsx
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- examples/switch.tsx
🧰 Additional context used
🪛 Biome
src/hooks/useHeights.tsx
[error] 10-10: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
🔇 Additional comments (2)
src/hooks/useHeights.tsx (2)
3-4
: 导入语句重新排序已完成
导入语句的重新排序提高了代码的可读性。
40-47
: 验证边距计算的兼容性
需要确保在不同浏览器中 getComputedStyle
返回的边距值格式一致。
✅ Verification successful
边距计算实现已经具备良好的兼容性处理
代码已经实现了稳健的边距值处理:
- 使用
parseNumber
函数统一处理边距值,该函数能够正确处理数值转换并默认为0 - 这是代码库中唯一使用
getComputedStyle
处理边距的地方,不存在不一致性风险 parseFloat
的使用确保了跨浏览器的数值解析兼容性
当前实现完全满足跨浏览器兼容性要求,无需额外修改。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索代码库中是否有其他地方使用 getComputedStyle 来处理边距,以确保一致性
rg "getComputedStyle.*margin"
Length of output: 31
Script:
#!/bin/bash
# 搜索 parseNumber 函数的实现,以了解边距值的处理方式
ast-grep --pattern 'function parseNumber($_) { $$$ }'
# 搜索所有与 margin 相关的计算逻辑
rg -A 3 "margin.*=.*"
# 搜索其他可能的样式计算方法
rg -A 3 "getComputedStyle"
Length of output: 1122
ref ant-design/ant-design#51485
Summary by CodeRabbit
新功能
parseNumber
,用于将字符串转换为浮点数,以支持高度计算。改进
marginTop
和marginBottom
,以提高布局和渲染的准确性。样式
MyItem
组件中添加了一个注释,指示之前存在的样式属性。