-
Notifications
You must be signed in to change notification settings - Fork 284
feat(switch): 异步切换支持受控loading态 #3122
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -5,20 +5,26 @@ const Demo2 = () => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [checkedAsync, setCheckedAsync] = useState(true) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [value, setValue] = useState(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [showToast, setShowToast] = useState(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const mockRequest = (): Promise<void> => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return new Promise((resolve) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setTimeout(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
resolve() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, 2000) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const onChangeAsync = (value: boolean, event: any) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const onChangeAsync = async (value: boolean) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setValue(value) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setShowToast(true) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setTimeout(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setCheckedAsync(value) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, 2000) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await mockRequest() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setCheckedAsync(value) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<Cell> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<Switch | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
checked={checkedAsync} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onChange={(value, event) => onChangeAsync(value, event)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onChange={(value) => onChangeAsync(value)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 更新事件处理器,匹配新的函数签名 更新了 在异步操作期间(2秒延迟),应添加加载状态指示器以提升用户体验。建议在 + const [loading, setLoading] = useState(false)
const onChangeAsync = (value: boolean) => {
setValue(value)
setShowToast(true)
+ setLoading(true)
setTimeout(() => {
setCheckedAsync(value)
+ setLoading(false)
}, 2000)
}
return (
<>
<Cell>
<Switch
checked={checkedAsync}
+ loading={loading}
onChange={(value) => onChangeAsync(value)}
/>
</Cell> 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</Cell> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<Toast | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import React, { FunctionComponent } from 'react' | ||
import React, { FunctionComponent, useEffect, useState } from 'react' | ||
import classNames from 'classnames' | ||
import { Loading1 } from '@nutui/icons-react' | ||
import { ComponentDefaults } from '@/utils/typings' | ||
import { usePropsValue } from '@/hooks/use-props-value' | ||
import { useRtl } from '@/packages/configprovider' | ||
|
@@ -10,6 +11,7 @@ const defaultProps = { | |
disabled: false, | ||
activeText: '', | ||
inactiveText: '', | ||
loadingIcon: <Loading1 />, | ||
} as WebSwitchProps | ||
export const Switch: FunctionComponent<Partial<WebSwitchProps>> = (props) => { | ||
const { | ||
|
@@ -18,6 +20,7 @@ export const Switch: FunctionComponent<Partial<WebSwitchProps>> = (props) => { | |
disabled, | ||
activeText, | ||
inactiveText, | ||
loadingIcon, | ||
className, | ||
style, | ||
onChange, | ||
|
@@ -35,6 +38,12 @@ export const Switch: FunctionComponent<Partial<WebSwitchProps>> = (props) => { | |
defaultValue: defaultChecked, | ||
}) | ||
|
||
useEffect(() => { | ||
changing && setChanging(false) | ||
}, [value]) | ||
|
||
const [changing, setChanging] = useState(false) | ||
Comment on lines
+41
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chain添加状态重置逻辑和状态变量 这段代码添加了两个关键功能:
这是处理异步状态变化的良好实践,但需确保测试覆盖。 🏁 Script executed: #!/bin/bash
# 检查是否存在测试文件并验证是否覆盖了新增的状态逻辑
echo "查找 Switch 组件的测试文件..."
fd "switch.*test" --type f Length of output: 79 代码状态变化逻辑已正确引入,但缺少相应测试覆盖 当前修改在
这些改动整体逻辑清晰且符合异步状态处理的良好实践。不过,通过执行检测脚本,目前没有找到针对 Switch 组件新增逻辑的测试文件。请确认或新增相应的单元测试,确保覆盖以下方面:
|
||
|
||
const classes = () => { | ||
return classNames([ | ||
classPrefix, | ||
|
@@ -47,18 +56,16 @@ export const Switch: FunctionComponent<Partial<WebSwitchProps>> = (props) => { | |
]) | ||
} | ||
|
||
const onClick = (event: React.MouseEvent<Element, MouseEvent>) => { | ||
if (disabled) return | ||
onChange && onChange(!value, event) | ||
const onClick = () => { | ||
if (disabled || changing) return | ||
if (props.onChange) { | ||
setChanging(true) | ||
props.onChange(!value) | ||
} | ||
setValue(!value) | ||
} | ||
Comment on lines
+60
to
66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chain需要添加测试覆盖新增功能 静态分析工具显示这些关键的新增代码行没有测试覆盖,特别是:
建议增加测试用例确保这些功能在各种条件下正常工作,例如:
这些测试对确保组件的稳定性和可靠性非常重要。 Also applies to: 84-84 🏁 Script executed: #!/bin/bash
# 查找与 Switch 组件相关的测试文件和测试覆盖情况
echo "检查 Switch 组件的测试文件..."
fd "switch.*test" --type f
echo "检查测试中是否已经测试了异步处理逻辑..."
if fd "switch.*test" --type f | xargs rg -l "changing|loading|async"; then
echo "找到了可能包含异步处理逻辑测试的文件"
else
echo "未找到包含异步处理逻辑测试的文件"
fi Length of output: 13008 请完善测试覆盖异步状态和加载图标功能 目前静态分析工具提示,新增的代码逻辑缺少相应的测试验证,建议对以下情况补充测试用例:
此外,这些问题同样适用于文件中其它局部相似的代码(例如:行 84)。请参考现有的 Switch 组件测试文件(如 🧰 Tools🪛 GitHub Check: codecov/patch[warning] 60-64: src/packages/switch/switch.tsx#L60-L64 |
||
return ( | ||
<div | ||
className={classes()} | ||
onClick={(e) => onClick(e)} | ||
style={style} | ||
{...rest} | ||
> | ||
<div className={classes()} onClick={onClick} style={style} {...rest}> | ||
<div | ||
className={classNames([ | ||
[`${classPrefix}-button`], | ||
|
@@ -73,8 +80,14 @@ export const Switch: FunctionComponent<Partial<WebSwitchProps>> = (props) => { | |
}, | ||
])} | ||
> | ||
{!value && !activeText && ( | ||
<div className={`${classPrefix}-close-line`} /> | ||
{changing && loadingIcon ? ( | ||
<>{loadingIcon}</> | ||
) : ( | ||
<> | ||
{!value && !activeText && ( | ||
<div className={`${classPrefix}-close-line`} /> | ||
)} | ||
</> | ||
)} | ||
</div> | ||
{activeText && ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,12 @@ | ||
import { ReactNode } from 'react' | ||
import { BaseProps } from '../../base/props' | ||
|
||
export interface BaseSwitch<EVENT = any> extends BaseProps { | ||
export interface BaseSwitch extends BaseProps { | ||
checked: boolean | ||
defaultChecked: boolean | ||
disabled: boolean | ||
activeText: ReactNode | ||
inactiveText: ReactNode | ||
onChange: (val: boolean, event: EVENT) => void | ||
loadingIcon: ReactNode | ||
onChange: (val: boolean) => void | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
import { BaseSwitch } from '../switch/base' | ||
|
||
export interface WebSwitchProps extends BaseSwitch<React.MouseEvent> {} | ||
export interface WebSwitchProps extends BaseSwitch {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,3 @@ | ||
import { ITouchEvent } from '@tarojs/components' | ||
import { BaseSwitch } from './base' | ||
|
||
type UnionEvent = React.MouseEvent<Element, MouseEvent> | ITouchEvent | ||
export interface TaroSwitchProps extends BaseSwitch<UnionEvent> {} | ||
export interface TaroSwitchProps extends BaseSwitch {} |
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.
loadingIcon测试中存在选择器错误,且缺少必要的断言
在此测试中发现以下问题:
.nut-icon
中的前导点号会导致选择失败,应该使用nut-icon
waitFor
函数没有被await,可能导致测试时序问题建议修改如下:
📝 Committable suggestion