Skip to content
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.

Sign up for GitHub

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(range): refactor & lint fixed #2637

Merged
merged 1 commit into from
Oct 11, 2024
Merged

fix(range): refactor & lint fixed #2637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions src/packages/range/range.taro.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export const Range: FunctionComponent<
setMarksList(list)
}
}
}, [marks])
}, [marks, max, min])

const scope = () => {
return max - min
Expand Down Expand Up @@ -379,13 +379,12 @@ export const Range: FunctionComponent<
<div className="nut-range-bar" style={barStyle()}>
{range ? (
[0, 1].map((item, index) => {
const cls = `${index === 0 ? 'nut-range-button-wrapper-left' : ''}
${index === 1 ? 'nut-range-button-wrapper-right' : ''}`
return (
<div
key={index}
className={`${
index === 0 ? 'nut-range-button-wrapper-left' : ''
}
${index === 1 ? 'nut-range-button-wrapper-right' : ''}`}
className={cls}
onTouchStart={(e: any) => {
if (typeof index === 'number') {
// 实时更新当前拖动的按钮索引
Expand Down
159 changes: 81 additions & 78 deletions src/packages/range/range.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
useEffect,
useState,
useRef,
useCallback,
ReactNode,
} from 'react'
import type { TouchEvent } from 'react'
Expand Down Expand Up @@ -59,9 +58,6 @@
button,
vertical,
marks,
onChange,
onStart,
onEnd,
minDescription,
maxDescription,
currentDescription,
Expand All @@ -70,6 +66,9 @@
step,
value,
defaultValue,
onChange,
onStart,
onEnd,
} = { ...defaultProps, ...props }

const classPrefix = 'nut-range'
Expand All @@ -78,7 +77,6 @@
const touch = useTouch()
const root = useRef<HTMLDivElement>(null)
const [marksList, setMarksList] = useState<number[]>([])

const [startValue, setStartValue] = useState<any>(0)

const handleChange = (value: RangeValue) => {
Expand All @@ -98,7 +96,6 @@
useEffect(() => {
if (marks) {
if (Array.isArray(marks)) {
// 增加变量
const list = marks
.sort((a, b) => a.value - b.value)
.filter((point) => point.value >= min && point.value <= max)
Expand All @@ -116,11 +113,7 @@
setMarksList(list)
}
}
}, [marks])

const scope = () => {
return max - min
}
}, [marks, max, min])

const classes = classNames(classPrefix, {
[`${classPrefix}-disabled`]: disabled,
Expand All @@ -135,30 +128,31 @@
className
)

const markClassName = useCallback(
(mark: any) => {
const classPrefix = 'nut-range-mark'
let lowerBound = min
let upperBound = max
if (range && Array.isArray(current)) {
lowerBound = current[0]
upperBound = current[1]
} else {
upperBound = current as number
}
const isActive = mark <= upperBound && mark >= lowerBound
return [
`${classPrefix}-text`,
`${isActive ? `${classPrefix}-text-active` : ''}`,
].join(' ')
},
[range, current, min, max]
)
const markClassName = (mark: any) => {
const classPrefix = 'nut-range-mark'
let lowerBound = min
let upperBound = max
if (range && Array.isArray(current)) {
lowerBound = current[0]
upperBound = current[1]

Check warning on line 137 in src/packages/range/range.tsx

View check run for this annotation

Codecov / codecov/patch

src/packages/range/range.tsx#L136-L137 

Added lines #L136 - L137 were not covered by tests
} else {
upperBound = current as number
}
const isActive = mark <= upperBound && mark >= lowerBound
return [
`${classPrefix}-text`,
`${isActive ? `${classPrefix}-text-active` : ''}`,
].join(' ')
}
Comment on lines +131 to +146
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

为函数参数提供具体的类型定义

函数 markClassName 中的参数 mark 类型为 any，建议使用更具体的类型以提高类型安全性和代码可读性。

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 136-137: src/packages/range/range.tsx#L136-L137
Added lines #L136 - L137 were not covered by tests


const isRange = (val: any) => {
return !!range && Array.isArray(val)
}

const scope = () => {
return max - min
}

Comment on lines +152 to +155
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

确保 scope 函数的返回值不为零以避免除零错误

如果 max 等于 min，scope 函数将返回 0，可能导致计算时的除零错误。建议在函数中添加检查或验证 max 大于 min。

const calcMainAxis = () => {
const modelVal = current as any
if (isRange(modelVal)) {
Expand All @@ -169,10 +163,9 @@

const calcOffset = () => {
const modelVal = current as any
if (isRange(modelVal)) {
return `${((modelVal[0] - min) * 100) / scope()}%`
}
return `0%`
return isRange(modelVal)
? `${((modelVal[0] - min) * 100) / scope()}%`
: `0%`
}

const barStyle = () => {
Expand Down Expand Up @@ -276,7 +269,6 @@
} else {
setStartValue(format(current as number))
}

setDragStatus('start')
}

Expand Down Expand Up @@ -345,12 +337,49 @@
)
}

return (
<div className={containerClasses}>
{minDescription !== null && (
<div className="min">{minDescription || min}</div>
)}
<div ref={root} className={classes} onClick={(e) => click(e)}>
const renderRangeButton = () => {
return [0, 1].map((item, index) => {
const cls = `${index === 0 ? 'nut-range-button-wrapper-left' : ''}
${index === 1 ? 'nut-range-button-wrapper-right' : ''}`
return (
<div
key={index}
className={cls}
onTouchStart={(e) => {
if (typeof index === 'number') {
setButtonIndex(index)
}
onTouchStart(e)
}}

Check warning on line 353 in src/packages/range/range.tsx

View check run for this annotation

Codecov / codecov/patch

src/packages/range/range.tsx#L349-L353 

Added lines #L349 - L353 were not covered by tests
Comment on lines +349 to +353
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

添加单元测试以覆盖新的事件处理逻辑

行 349-353 的交互事件处理代码未被测试覆盖，建议添加相应的单元测试以提高代码的可靠性。

需要我帮助编写这些单元测试或为此创建一个新的 GitHub Issue 吗？

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 349-353: src/packages/range/range.tsx#L349-L353
Added lines #L349 - L353 were not covered by tests

onTouchMove={(e) => onTouchMove(e)}
onTouchEnd={onTouchEnd}
onTouchCancel={onTouchEnd}
onClick={(e) => e.stopPropagation()}
>
{renderButton(index)}
</div>
)
})
}

Comment on lines +340 to +364
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

移除冗余的类型检查

index 始终为 number 类型，if (typeof index === 'number') 的检查是多余的，可以移除以简化代码。

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 349-353: src/packages/range/range.tsx#L349-L353
Added lines #L349 - L353 were not covered by tests

const renderSingleButton = () => {
return (
<div
className="nut-range-button-wrapper"
onTouchStart={(e) => onTouchStart(e)}
onTouchMove={(e) => onTouchMove(e)}
onTouchEnd={onTouchEnd}
onTouchCancel={onTouchEnd}
onClick={(e) => e.stopPropagation()}
>
{renderButton()}
</div>
)
}
Comment on lines +365 to +378
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

简化事件处理函数的绑定

可以直接将事件处理函数绑定，而无需创建新的匿名函数。例如，将 onTouchStart={(e) => onTouchStart(e)} 简化为 onTouchStart={onTouchStart}。


const renderMark = () => {
return (
<>
{marksList.length > 0 && (
<div className="nut-range-mark">
{marksList.map((mark: any) => {
Expand All @@ -371,45 +400,19 @@
})}
</div>
)}
</>
)
}

return (
<div className={containerClasses}>
{minDescription !== null && (
<div className="min">{minDescription || min}</div>
)}
<div ref={root} className={classes} onClick={(e) => click(e)}>
{renderMark()}
<div className="nut-range-bar" style={barStyle()}>
{range ? (
[0, 1].map((item, index) => {
return (
<div
key={index}
className={`${
index === 0 ? 'nut-range-button-wrapper-left' : ''
}
${index === 1 ? 'nut-range-button-wrapper-right' : ''}`}
onTouchStart={(e) => {
if (typeof index === 'number') {
// 实时更新当前拖动的按钮索引
setButtonIndex(index)
}
onTouchStart(e)
}}
onTouchMove={(e) => onTouchMove(e)}
onTouchEnd={onTouchEnd}
onTouchCancel={onTouchEnd}
onClick={(e) => e.stopPropagation()}
>
{renderButton(index)}
</div>
)
})
) : (
<div
className="nut-range-button-wrapper"
onTouchStart={(e) => onTouchStart(e)}
onTouchMove={(e) => onTouchMove(e)}
onTouchEnd={onTouchEnd}
onTouchCancel={onTouchEnd}
onClick={(e) => e.stopPropagation()}
>
{renderButton()}
</div>
)}
{range ? renderRangeButton() : renderSingleButton()}
</div>
</div>
{maxDescription !== null && (
Expand Down
Loading