-
-
Notifications
You must be signed in to change notification settings - Fork 777
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
feat: retire deprecated api #1055
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
总览变更说明此次变更包括将 变更
诗歌
序列图由于变更主要涉及代码重构和测试调整,不生成序列图。 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 (
|
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)
tests/Range.test.tsx (2)
171-171
: 统一断言方法以提高可读性将
expect(onAfterChange).not.toBeCalled();
改为expect(onAfterChange).not.toHaveBeenCalled();
,使断言方法更加直观和一致。
556-556
: 统一断言方法以提高可读性将
expect(handleFocus).toBeCalled();
改为expect(handleFocus).toHaveBeenCalled();
,保持了断言方法的统一性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.gitignore
(1 hunks)src/Slider.tsx
(6 hunks)tests/Range.test.tsx
(2 hunks)tests/Slider.test.js
(2 hunks)tests/common.test.js
(0 hunks)
💤 Files with no reviewable changes (1)
- tests/common.test.js
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🔇 Additional comments (9)
src/Slider.tsx (6)
244-245
: 确保处理单值和数组的情况在处理
mergedValue
时,代码正确地将其统一为数组形式,确保了后续逻辑的一致性。
254-256
: 当值未定义时提供默认值当
mergedValue
为undefined
时,代码将returnValues
设置为[mergedMin, mergedMin]
。请确认这符合预期行为,避免产生意外的默认值。
360-360
: 在空的rawValues
情况下添加新值当启用范围且
rawValues
为空时,代码会在cloneNextValues
中添加newValue
。请确保在这种情况下添加的新值不会导致后续逻辑错误。
550-550
: 为轨道添加样式支持在
div
元素上添加了style={styles?.rail}
,这使得轨道样式可以通过外部传入的样式对象进行定制。
556-556
: 为Tracks
组件添加样式支持通过传递
style={styles?.track}
,现在可以自定义Tracks
组件的样式,增强了组件的灵活性。
574-574
: 为Handles
组件添加样式支持增加了
style={styles?.handle}
,允许外部自定义Handles
组件的样式,更好地适应不同的样式需求。tests/Slider.test.js (2)
565-580
: 更新测试以验证onChange
回调修改了测试用例,确保在滑块被点击时,
onChange
回调函数被正确调用,符合组件的预期行为。
662-662
: 验证当value
未定义时的组件行为测试在
value
为undefined
或null
且tooltip
打开时,组件是否正常运行,确保组件的稳定性。tests/Range.test.tsx (1)
175-175
: 移除count
参数后更新测试用例删除了对
count={3}
的使用,直接提供值数组来测试多段滑块的渲染。请确保组件在移除count
参数后仍然正常工作。
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 (3)
docs/examples/range.tsx (1)
228-235
: 样式 API 已统一将独立的样式属性 (
trackStyle
,handleStyle
,railStyle
) 合并为统一的styles
对象是一个好的改进。建议删除注释掉的旧代码。- // trackStyle={[{ backgroundColor: 'red' }, { backgroundColor: 'green' }]} - // handleStyle={[{ backgroundColor: 'yellow' }, { backgroundColor: 'gray' }]} - // railStyle={{ backgroundColor: 'black' }}docs/examples/slider.tsx (2)
235-245
: 样式配置已更新,但可以优化新的
styles
API 实现正确,但样式对象的结构可以更简洁。建议将相同的属性分组。styles={{ - rail: { backgroundColor: 'red', height: 10 }, - track: { backgroundColor: 'blue', height: 10 }, + rail: { + backgroundColor: 'red', + height: 10 + }, + track: { + backgroundColor: 'blue', + height: 10 + }, handle: { borderColor: 'blue', height: 28, width: 28, marginLeft: -14, marginTop: -9, backgroundColor: 'black', }, }}
277-287
: 建议提取重复的样式配置相同的样式配置在多个地方重复出现,建议提取为常量以提高可维护性。
+const sliderStyles = { + track: { backgroundColor: 'blue', height: 10 }, + handle: { + borderColor: 'blue', + height: 28, + width: 28, + marginLeft: -14, + marginTop: -9, + backgroundColor: 'black', + }, + rail: { backgroundColor: 'red', height: 10 }, +}; -styles={{ - track: { backgroundColor: 'blue', height: 10 }, - handle: { - borderColor: 'blue', - height: 28, - width: 28, - marginLeft: -14, - marginTop: -9, - backgroundColor: 'black', - }, - rail: { backgroundColor: 'red', height: 10 }, -}} +styles={sliderStyles}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/examples/range.tsx
(2 hunks)docs/examples/slider.tsx
(4 hunks)
🔇 Additional comments (3)
docs/examples/range.tsx (2)
220-221
: range 属性格式已更新将
range
从布尔值更改为对象配置({ minCount: 3 }
)提供了更灵活的范围控制。这是一个突破性的改变。建议:
- 更新组件文档以反映新的 API 格式
- 考虑添加向后兼容层以支持布尔值用法
196-196
: 事件处理器已更新从
onBeforeChange
更改为onChange
是 API 更新的一部分。请确保所有依赖此组件的代码都已相应更新。docs/examples/slider.tsx (1)
201-201
: 事件处理器更改保持一致与 range.tsx 中的更改一致,这里也将事件处理器从
onBeforeChange
更改为onChange
。
/** @deprecated Please use `styles.handle` instead */ | ||
handleStyle?: React.CSSProperties | React.CSSProperties[]; | ||
/** @deprecated Please use `styles.rail` instead */ | ||
railStyle?: React.CSSProperties; | ||
dotStyle?: React.CSSProperties | ((dotValue: number) => React.CSSProperties); |
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.
看起来这两个 miss 了,是不是已经有平替了?下面那个也是
Summary by CodeRabbit
新功能
Bug 修复
重构
文档
样式
测试
其他