-
-
Notifications
You must be signed in to change notification settings - Fork 319
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: activeBar offset not right when start from the opposite direction #890
fix: activeBar offset not right when start from the opposite direction #890
Conversation
Co-authored-by: Dmaziyo <[email protected]> close react-component#889 ref: ant-design/ant-design#51480
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改涉及 Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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 #890 +/- ##
==========================================
+ Coverage 95.43% 95.45% +0.01%
==========================================
Files 64 65 +1
Lines 2719 2727 +8
Branches 762 763 +1
==========================================
+ Hits 2595 2603 +8
Misses 121 121
Partials 3 3 ☔ 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: 0
🧹 Outside diff range and nitpick comments (2)
src/PickerInput/Selector/hooks/usePrevious.ts (2)
1-3
: 建议添加 JSDoc 文档为了提高代码的可维护性和可读性,建议添加 JSDoc 文档来说明这个 hook 的用途、参数和返回值。
import { useRef } from 'react'; +/** + * 自定义 Hook,用于追踪状态的前一个值 + * @template T 状态值的类型 + * @param {T} state 当前状态值 + * @returns {T | undefined} 返回状态的前一个值,首次渲染时返回 undefined + */ function usePrevious<T>(state: T): T | undefined {
3-13
: 实现逻辑正确,建议增加初始渲染的处理Hook 的核心实现逻辑正确,使用
Object.is
进行严格相等比较很恰当。不过建议明确处理初始渲染的情况。function usePrevious<T>(state: T): T | undefined { const prevRef = useRef<T>(); const curRef = useRef<T>(); + // 初始渲染时更新 curRef + if (curRef.current === undefined) { + curRef.current = state; + return undefined; + } + if (Object.is(curRef.current, state)) { prevRef.current = curRef.current; curRef.current = state; } return prevRef.current; }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 8-9: src/PickerInput/Selector/hooks/usePrevious.ts#L8-L9
Added lines #L8 - L9 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/PickerInput/Selector/RangeSelector.tsx
(4 hunks)src/PickerInput/Selector/hooks/usePrevious.ts
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/PickerInput/Selector/hooks/usePrevious.ts
[warning] 8-9: src/PickerInput/Selector/hooks/usePrevious.ts#L8-L9
Added lines #L8 - L9 were not covered by tests
🔇 Additional comments (3)
src/PickerInput/Selector/hooks/usePrevious.ts (1)
8-9
: 需要添加单元测试
这些关键的状态转换逻辑行目前缺少测试覆盖。建议添加以下测试场景:
- 初始渲染时的行为
- 状态更新时的行为
- 连续多次更新的行为
需要我帮您生成相应的单元测试代码吗?可以包含上述测试场景的完整实现。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 8-9: src/PickerInput/Selector/hooks/usePrevious.ts#L8-L9
Added lines #L8 - L9 were not covered by tests
src/PickerInput/Selector/RangeSelector.tsx (2)
4-4
: 导入和钩子使用看起来不错!
新增的 omit
工具函数和 usePrevious
钩子的使用很合理,这些改动为解决活动条样式问题提供了必要的基础支持。
Also applies to: 13-13, 180-180
192-202
: 实现逻辑正确,建议验证边缘情况
样式更新的逻辑改进很好:
- 正确处理了不同方向的偏移计算
- 使用
omit
清理了之前的样式,避免样式冲突 - 完全符合修复活动条偏移问题的目标
建议验证以下场景:
- 在快速切换方向时的行为
- 在不同容器宽度下的表现
- 在 RTL 模式下的正确性
✅ Verification successful
Let me gather more information about the style handling and RTL support.
Let me try one more time with correct file type flags:
样式更新逻辑验证完成,实现安全可靠
根据代码分析结果:
getoffsetUnit
函数在utils/uiUtil.ts
中实现,正确处理了 RTL 和方向切换逻辑- 活动条样式更新仅在
RangeSelector
组件中使用,不影响其他组件 - 使用
usePrevious
hook 和omit
函数确保了方向切换时的样式清理
实现完全满足需求,无需额外验证。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有其他组件使用了类似的偏移逻辑,确保解决方案的通用性
# 搜索相似的样式处理模式
rg -l "position:\s*absolute.*width.*offset" --type tsx
# 检查是否有其他使用 offsetUnit 的地方
ast-grep --pattern 'const $_ = getoffsetUnit($_, $_)'
Length of output: 337
Script:
#!/bin/bash
# Check the implementation of getoffsetUnit and related RTL logic
rg -A 5 "getoffsetUnit" --type ts --type tsx
# Look for RTL related style handling
rg -A 5 "rtl.*placement" --type ts --type tsx
# Check for any similar active bar or selector styling
rg -A 5 "setActiveBarStyle|activeBarStyle" --type ts --type tsx
Length of output: 242
Script:
#!/bin/bash
# Check the implementation of getoffsetUnit and related RTL logic
rg -A 5 "getoffsetUnit"
# Look for RTL related style handling
rg -A 5 "rtl.*placement"
# Check for any similar active bar or selector styling
rg -A 5 "setActiveBarStyle|activeBarStyle"
# Find all TypeScript/React files that might contain similar patterns
fd "\.(tsx?|jsx?)$" -x rg -l "position.*absolute.*offset"
Length of output: 3901
close #889
ref: ant-design/ant-design#51480
Summary by CodeRabbit
新功能
usePrevious
,用于跟踪给定值的先前状态。RangeSelector
组件的样式处理,增强了对活动条样式的控制。文档
usePrevious
钩子的使用说明。