-
Notifications
You must be signed in to change notification settings - Fork 260
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/range 适配 JDRN、鸿蒙端 #2357
Feat/range 适配 JDRN、鸿蒙端 #2357
Conversation
Walkthrough这些改动主要发生在 Changes
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (17)
Files skipped from review as they are similar to previous changes (15)
Additional context usedBiome
Additional comments not posted (1)
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 20
Outside diff range and nitpick comments (5)
src/packages/range/demos/taro/demo12.tsx (1)
7-22
: 根据平台动态调整样式,使用了 rn() 来适配 JDRN 平台。建议在文档中添加对这些特定平台样式调整的说明,以便其他开发者理解和维护。src/packages/range/demos/taro/demo11.tsx (2)
7-27
: 根据平台动态调整样式,使用了 rn() 来适配 JDRN 平台。建议在文档中添加对这些特定平台样式调整的说明,以便其他开发者理解和维护。
29-36
: 使用 harmony() 来适配鸿蒙平台,确保样式的一致性和兼容性。这种平台特定的代码分支处理是必要的,但也增加了代码的复杂性。src/packages/range/demos/taro/demo13.tsx (1)
7-45
: 针对 JDRN 和鸿蒙平台进行了样式调整。这种平台特定的代码分支处理是必要的,但也增加了代码的复杂性。建议在文档中添加对这些特定平台样式调整的说明,以便其他开发者理解和维护。src/packages/range/demo.taro.tsx (1)
15-15
: 注释了 Demo10 的引用。如果这个组件是永久移除的,建议从项目中完全删除相关的文件和资源,以避免未来的混淆和潜在错误。
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: 7
Outside diff range and nitpick comments (1)
packages/nutui-taro-demo-rn/scripts/taro/adapted.js (1)
Line range hint
1-1
: 请避免重新分配全局变量,这可能会覆盖重要功能。- exports = module.exports = [ + module.exports = [
const isSameValue = (newValue: RangeValue, oldValue: RangeValue) => { | ||
return JSON.stringify(newValue) === JSON.stringify(oldValue) | ||
} | ||
|
||
const handleOverlap = (value: number[]) => { | ||
if (value[0] > value[1]) { | ||
return value.slice(0).reverse() | ||
} | ||
return value |
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.
isSameValue
函数通过比较 JSON 字符串来检查两个值是否相同,这种方法虽然简单,但可能会因为 JSON.stringify 的顺序问题而导致意外的结果。建议改用更稳定的深度比较方法。
- return JSON.stringify(newValue) === JSON.stringify(oldValue)
+ return _.isEqual(newValue, oldValue) // 使用 lodash 的 isEqual 方法进行深度比较
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const isSameValue = (newValue: RangeValue, oldValue: RangeValue) => { | |
return JSON.stringify(newValue) === JSON.stringify(oldValue) | |
} | |
const handleOverlap = (value: number[]) => { | |
if (value[0] > value[1]) { | |
return value.slice(0).reverse() | |
} | |
return value | |
const isSameValue = (newValue: RangeValue, oldValue: RangeValue) => { | |
return _.isEqual(newValue, oldValue) // 使用 lodash 的 isEqual 方法进行深度比较 | |
} | |
const handleOverlap = (value: number[]) => { | |
if (value[0] > value[1]) { | |
return value.slice(0).reverse() | |
} | |
return value |
const onTouchStart = useCallback( | ||
(event: any) => { | ||
if (disabled) { | ||
return | ||
} | ||
touch.start(event) | ||
setExactValue(current) | ||
if (isRange(current)) { | ||
setStartValue((current as number[]).map(format)) | ||
} else { | ||
updateValue([left, value], true) | ||
setStartValue(format(current as number)) | ||
} | ||
} else { | ||
updateValue(value, true) | ||
} | ||
} | ||
|
||
const onTouchStart = (event: any) => { | ||
setDragStatus('start') | ||
}, | ||
[current, disabled, format, isRange, touch] | ||
) | ||
|
||
const onTouchMove = useCallback( | ||
async (event: any) => { | ||
// @TODO RN、鸿蒙端垂直滑动时,页面会一同滑动,待解决 | ||
if (disabled || !root.current) { | ||
return | ||
} | ||
if (dragStatus === 'start') { | ||
onStart && onStart() | ||
} | ||
|
||
touch.move(isRn ? event.nativeEvent : event) | ||
// console.log(JSON.stringify(event.touches[0])) | ||
|
||
setDragStatus('draging') | ||
|
||
const rect = await getRectByTaro(root.current) | ||
if (!rect) return | ||
let delta = isHm | ||
? parseFloat(pxTransform(touch.deltaX.current)) | ||
: touch.deltaX.current | ||
let total = rect.width | ||
// console.log(pxTransform(delta), total) | ||
let diff = (delta / total) * scope | ||
diff = rtl ? -diff : diff | ||
if (vertical) { | ||
delta = isHm | ||
? parseFloat(pxTransform(touch.deltaY.current)) | ||
: touch.deltaY.current | ||
total = rect.height | ||
diff = (delta / total) * scope | ||
} | ||
|
||
let newValue | ||
if (isRange(startValue)) { | ||
newValue = (exactValue as number[]).slice() | ||
newValue[buttonIndex] = startValue[buttonIndex] + diff | ||
} else { | ||
newValue = startValue + diff | ||
} | ||
setExactValue(newValue) | ||
updateValue(newValue) | ||
}, | ||
[ | ||
buttonIndex, | ||
disabled, | ||
dragStatus, | ||
exactValue, | ||
isRange, | ||
onStart, | ||
rtl, | ||
scope, | ||
startValue, | ||
touch, | ||
updateValue, | ||
vertical, | ||
] | ||
) |
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.
onTouchMove
函数处理触摸移动事件。代码中有一处待解决的 TODO 注释,关于在 RN 和 鸿蒙端垂直滑动时页面会一同滑动的问题,建议跟踪这个问题,确保它能够得到妥善解决。
- // @TODO RN、鸿蒙端垂直滑动时,页面会一同滑动,待解决
+ // @TODO 解决 RN 和 鸿蒙端在垂直滑动时页面同步滑动的问题
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const onTouchStart = useCallback( | |
(event: any) => { | |
if (disabled) { | |
return | |
} | |
touch.start(event) | |
setExactValue(current) | |
if (isRange(current)) { | |
setStartValue((current as number[]).map(format)) | |
} else { | |
updateValue([left, value], true) | |
setStartValue(format(current as number)) | |
} | |
} else { | |
updateValue(value, true) | |
} | |
} | |
const onTouchStart = (event: any) => { | |
setDragStatus('start') | |
}, | |
[current, disabled, format, isRange, touch] | |
) | |
const onTouchMove = useCallback( | |
async (event: any) => { | |
// @TODO RN、鸿蒙端垂直滑动时,页面会一同滑动,待解决 | |
if (disabled || !root.current) { | |
return | |
} | |
if (dragStatus === 'start') { | |
onStart && onStart() | |
} | |
touch.move(isRn ? event.nativeEvent : event) | |
// console.log(JSON.stringify(event.touches[0])) | |
setDragStatus('draging') | |
const rect = await getRectByTaro(root.current) | |
if (!rect) return | |
let delta = isHm | |
? parseFloat(pxTransform(touch.deltaX.current)) | |
: touch.deltaX.current | |
let total = rect.width | |
// console.log(pxTransform(delta), total) | |
let diff = (delta / total) * scope | |
diff = rtl ? -diff : diff | |
if (vertical) { | |
delta = isHm | |
? parseFloat(pxTransform(touch.deltaY.current)) | |
: touch.deltaY.current | |
total = rect.height | |
diff = (delta / total) * scope | |
} | |
let newValue | |
if (isRange(startValue)) { | |
newValue = (exactValue as number[]).slice() | |
newValue[buttonIndex] = startValue[buttonIndex] + diff | |
} else { | |
newValue = startValue + diff | |
} | |
setExactValue(newValue) | |
updateValue(newValue) | |
}, | |
[ | |
buttonIndex, | |
disabled, | |
dragStatus, | |
exactValue, | |
isRange, | |
onStart, | |
rtl, | |
scope, | |
startValue, | |
touch, | |
updateValue, | |
vertical, | |
] | |
) | |
const onTouchMove = useCallback( | |
async (event: any) => { | |
// @TODO 解决 RN 和 鸿蒙端在垂直滑动时页面同步滑动的问题 | |
if (disabled || !root.current) { | |
return | |
} | |
if (dragStatus === 'start') { | |
onStart && onStart() | |
} | |
touch.move(isRn ? event.nativeEvent : event) | |
// console.log(JSON.stringify(event.touches[0])) | |
setDragStatus('draging') | |
const rect = await getRectByTaro(root.current) | |
if (!rect) return | |
let delta = isHm | |
? parseFloat(pxTransform(touch.deltaX.current)) | |
: touch.deltaX.current | |
let total = rect.width | |
// console.log(pxTransform(delta), total) | |
let diff = (delta / total) * scope | |
diff = rtl ? -diff : diff | |
if (vertical) { | |
delta = isHm | |
? parseFloat(pxTransform(touch.deltaY.current)) | |
: touch.deltaY.current | |
total = rect.height | |
diff = (delta / total) * scope | |
} | |
let newValue | |
if (isRange(startValue)) { | |
newValue = (exactValue as number[]).slice() | |
newValue[buttonIndex] = startValue[buttonIndex] + diff | |
} else { | |
newValue = startValue + diff | |
} | |
setExactValue(newValue) | |
updateValue(newValue) | |
}, | |
[ | |
buttonIndex, | |
disabled, | |
dragStatus, | |
exactValue, | |
isRange, | |
onStart, | |
rtl, | |
scope, | |
startValue, | |
touch, | |
updateValue, | |
vertical, | |
] | |
) |
Tools
Biome
[error] 334-334: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
import { Range, Cell, Toast } from '@nutui/nutui-react-taro' | ||
import React, { useMemo, useState } from 'react' | ||
import { View } from '@tarojs/components' | ||
import { Range, Cell /* , Toast */ } from '@nutui/nutui-react-taro' |
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.
Tip
Codebase Verification
移除了 Toast
组件的引用,但在项目的其他部分仍然有大量的 Toast
组件使用实例。因此,移除 Toast
组件的引用可能会导致功能问题。
src/packages/table/demo.taro.tsx
src/packages/table/demos/h5/demo9.tsx
src/packages/hoverbutton/demos/h5/demo1.tsx
src/packages/hoverbutton/demos/h5/demo2.tsx
src/packages/toast/Notification.tsx
src/packages/toast/demo.taro.tsx
src/packages/toast/toast.taro.tsx
src/packages/toast/index.taro.ts
src/packages/toast/toast.tsx
src/packages/toast/__test__/toast.spec.tsx
src/packages/toast/demos/taro/demo5.tsx
src/packages/toast/demos/taro/demo4.tsx
src/packages/toast/demos/taro/demo3.tsx
src/packages/toast/demos/taro/demo1.tsx
src/packages/toast/demos/h5/demo5.tsx
src/packages/toast/demos/h5/demo3.tsx
src/packages/toast/demos/h5/demo2.tsx
src/packages/toast/demos/h5/demo1.tsx
src/packages/toast/demos/h5/demo4.tsx
Analysis chain
移除了 Toast
组件的引用,但在项目的其他部分仍然有大量的 Toast
组件使用实例。因此,移除 Toast
组件的引用可能会导致功能问题。
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the Toast component is not used elsewhere in the project where it might still be necessary.
# Test: Search for the Toast usage in the project. Expect: No critical usage that conflicts with its removal.
rg --type typescript $'Toast' --files-with-matches
Length of output: 84
Script:
#!/bin/bash
# Description: Verify that the Toast component is not used elsewhere in the project where it might still be necessary.
# Test: Search for the Toast usage in the project. Expect: No critical usage that conflicts with its removal.
rg --type ts 'Toast'
Length of output: 41463
{/* <Toast | ||
type="text" | ||
visible={show} | ||
content={msg} | ||
onClose={() => { | ||
setShow(false) | ||
}} | ||
/> | ||
</> | ||
/> */} |
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.
注释了 Toast
组件的使用,如果这是临时的,请添加 TODO 或 FIXME 标记。
+ // TODO: Review the necessity of Toast component here and decide whether to reintroduce or permanently remove.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
{/* <Toast | |
type="text" | |
visible={show} | |
content={msg} | |
onClose={() => { | |
setShow(false) | |
}} | |
/> | |
</> | |
/> */} | |
// TODO: Review the necessity of Toast component here and decide whether to reintroduce or permanently remove. | |
{/* <Toast | |
type="text" | |
visible={show} | |
content={msg} | |
onClose={() => { | |
setShow(false) | |
}} | |
/> */} |
key={index} | ||
className={classNames(`${classPrefix}-button-wrapper-${suffix}`, { | ||
[`${verticalClassPrefix}-button-wrapper-${suffix}`]: vertical, | ||
[`${rtlClassPrefix}-button-wrapper-${suffix}`]: rtl, | ||
})} | ||
style={{ | ||
transform, | ||
}} | ||
onTouchStart={(e) => { | ||
if (typeof index === 'number') { | ||
// 实时更新当前拖动的按钮索引 | ||
setButtonIndex(index) | ||
} | ||
onTouchStart(e) | ||
}} | ||
onTouchMove={(e) => onTouchMove(e)} | ||
onTouchEnd={onTouchEnd} | ||
onTouchCancel={onTouchEnd} | ||
onClick={(e) => e.stopPropagation()} | ||
> | ||
{renderButton(index)} |
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.
触发鼠标事件的操作还应该伴有相应的键盘事件以支持仅使用键盘的导航。
+ onKeyUp={(e) => { /* handle key up */ }}
Also applies to: 510-524, 554-562
Tools
Biome
[error] 483-503: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
const [buttonIndex, setButtonIndex] = useState(0) | ||
const [dragStatus, setDragStatus] = useState('start' || 'draging' || '') | ||
const touch = useTouch() | ||
const root = useRef<HTMLDivElement>(null) | ||
const [marksList, setMarksList] = useState<number[]>([]) | ||
|
||
const [startValue, setStartValue] = useState<any>(0) | ||
const scope = useMemo(() => max - min, [max, min]) | ||
|
||
const handleChange = (value: RangeValue) => { | ||
onChange && onChange(value) |
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.
建议使用可选链来简化代码。
- onChange && onChange(value)
+ onChange?.(value)
Also applies to: 317-318
Tools
Biome
[error] 105-105: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
const isSameValue = (newValue: RangeValue, oldValue: RangeValue) => { | ||
return JSON.stringify(newValue) === JSON.stringify(oldValue) | ||
} |
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.
isSameValue
函数通过比较 JSON 字符串来判断值是否相同,这种方法虽然简单,但可能会有性能问题,特别是当值包含大量数据或复杂结构时。建议使用更高效的深度比较方法。
- return JSON.stringify(newValue) === JSON.stringify(oldValue)
+ return _.isEqual(newValue, oldValue) // 使用 lodash 的 isEqual 进行深度比较
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const isSameValue = (newValue: RangeValue, oldValue: RangeValue) => { | |
return JSON.stringify(newValue) === JSON.stringify(oldValue) | |
} | |
const isSameValue = (newValue: RangeValue, oldValue: RangeValue) => { | |
return _.isEqual(newValue, oldValue) // 使用 lodash 的 isEqual 进行深度比较 | |
} |
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.
Demo 最好要兼容老的实现,兼容 小程序 和 h5 的最简写法~~
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.
需要fix 冲突
@@ -26,15 +34,15 @@ const Demo1 = () => { | |||
onEnd={(val) => showToast(`${val}`)} | |||
/> | |||
</Cell> | |||
<Toast | |||
{/* <Toast |
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.
不要影响 h5 和 小程序~
padding: '40px 18px', | ||
} | ||
const cellStyle = useMemo( | ||
() => ({ |
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.
最好保持 h5 和 小程序 的最简写法~ 做兼容处理。
以下的demo类似~~
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: 2
@@ -26,6 +26,7 @@ exports = module.exports = [ | |||
'hoverbutton', | |||
'safearea', | |||
'hoverbuttonitem', | |||
'range', |
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.
添加 'range' 到导出列表是符合 PR 目标的。但是,请注意修复全局变量重新赋值的问题。
- exports = module.exports = [
+ module.exports = [
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
'range', | |
module.exports = [ | |
'range', |
{/* <Toast | ||
type="text" | ||
visible={show} | ||
content={msg} | ||
onClose={() => { | ||
setShow(false) | ||
}} | ||
/> | ||
</> | ||
/> */} |
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.
注释了 'Toast' 组件的使用。建议添加 TODO 或 FIXME 标记,以便将来审查此组件的必要性和是否重新引入或永久移除。
+ // TODO: Review the necessity of Toast component here and decide whether to reintroduce or permanently remove.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
{/* <Toast | |
type="text" | |
visible={show} | |
content={msg} | |
onClose={() => { | |
setShow(false) | |
}} | |
/> | |
</> | |
/> */} | |
{/* <Toast | |
type="text" | |
visible={show} | |
content={msg} | |
onClose={() => { | |
setShow(false) | |
}} | |
/> */} | |
// TODO: Review the necessity of Toast component here and decide whether to reintroduce or permanently remove. |
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新特性
Range
组件的支持。样式
Cell
组件样式调整,使用pxTransform()
进行动态样式设置。<View>
标签进行渲染,并添加了适用于不同平台的样式条件。代码优化
useMemo
优化样式计算,提升性能。Toast
组件的相关代码。修复
getX
和getY
函数,以更准确地返回touch
对象的属性。