-
Notifications
You must be signed in to change notification settings - Fork 267
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
refactor: fixed overlay zIndex is not effective, and extract types of dialog, overlay, fixednav, popup, toast #2954
Conversation
演练这个拉取请求包含了多个组件的重大更新,主要涉及类型定义、导入路径、默认属性和组件配置的重构。更改跨越了多个包,如 变更
可能相关的 PR
建议的审阅者
诗歌
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: 9
🔭 Outside diff range comments (4)
src/packages/fixednav/fixednav.tsx (1)
Line range hint
78-91
: 改进列表项的类型定义list 项使用 any 类型,应该定义具体的接口类型。
+interface FixedNavItem { + id?: string | number + icon: string | React.ReactElement + text: string + num?: number +} -{list.map((item: any, index) => { +{list.map((item: FixedNavItem, index) => {src/packages/toast/Notification.tsx (1)
Line range hint
42-50
: 建议优化定时器清理逻辑当前的定时器清理逻辑可能存在内存泄漏风险,建议修改:
clearCloseTimer() { if (this.closeTimer) { clearTimeout(this.closeTimer) - this.closeTimer = -1 + this.closeTimer = undefined } }src/packages/dialog/dialog.tsx (1)
Line range hint
74-86
: 建议优化异步事件处理器的错误处理当前的异步事件处理器在错误处理方面可以改进:
const handleOk = async (e: MouseEvent<HTMLButtonElement>) => { e.stopPropagation() setLoading(true) try { await onConfirm?.(e) setLoading(false) onClose?.() - } catch { + } catch (error) { + console.error('Dialog confirmation failed:', error) setLoading(false) + // 可以添加错误处理逻辑,比如显示错误提示 } }src/packages/shortpassword/shortpassword.tsx (1)
Line range hint
89-94
: 建议优化 useEffect 的依赖项当前的 useEffect 依赖项可能导致不必要的重渲染:
useEffect(() => { if (visible && autoFocus) { onFocus && onFocus() } - }, [visible]) + }, [visible, autoFocus, onFocus])
♻️ Duplicate comments (1)
src/packages/toast/demos/taro/demo3.tsx (1)
19-24
:⚠️ Potential issue状态更新逻辑需要改进
与 demo4.tsx 存在相同的问题,直接修改状态对象可能导致组件渲染问题。
- const changeState = Object.assign(state, { - icon, - content, - duration, - title, - }) - setState(changeState) + setState({ + ...state, + icon, + content, + duration, + title, + })
🧹 Nitpick comments (26)
src/packages/overlay/types.ts (1)
3-12
: 建议为接口添加 JSDoc 文档注释接口定义结构清晰,但建议添加以下内容:
- 接口的整体说明
- 每个属性的用途说明
- 默认值说明(特别是 zIndex)
建议添加如下文档注释:
+/** + * Overlay 组件的 Props 接口 + */ export interface OverlayProps extends BasicComponent { + /** 遮罩层的 z-index 值 */ zIndex: number + /** 动画时长,单位毫秒 */ duration: number // ... 为其他属性添加类似的注释 }src/packages/overlay/demos/h5/demo2.tsx (1)
22-22
: 建议创建统一的 z-index 管理系统直接使用魔法数字 2000 作为 z-index 值不利于维护。建议:
- 创建统一的 z-index 层级管理
- 使用常量或配置文件定义这些值
- 添加注释说明不同组件的层级关系
示例实现:
// zindex.ts export const Z_INDEX = { OVERLAY: 2000, TOAST: 2100, DIALOG: 2200, // ... 其他组件 } as constsrc/packages/toast/types.ts (1)
3-11
: 建议使用枚举类型替代字符串联合类型对于固定值集合,使用枚举可以提供更好的类型安全和代码提示。
-export type ToastPosition = 'top' | 'bottom' | 'center' -export type ToastSize = 'small' | 'base' | 'large' +export enum ToastPosition { + Top = 'top', + Bottom = 'bottom', + Center = 'center', +} +export enum ToastSize { + Small = 'small', + Base = 'base', + Large = 'large', +}src/packages/overlay/demos/taro/demo1.tsx (1)
22-22
: 建议统一 H5 和 Taro 平台的 zIndex 值目前观察到:
- H5 demo 使用 zIndex={2020}
- Taro demo 使用 zIndex={3000}
建议统一这两个平台的默认值,以保持一致性。
src/packages/toast/demos/taro/demo4.tsx (1)
6-10
: 状态管理需要优化在状态初始化中,建议将所有必需的字段都明确定义默认值,以提高代码的可维护性。
const [state, setState] = useState({ icon: '', - content: 'toast', + content: '', duration: 2, title: '', })src/packages/dialog/content.tsx (1)
5-13
: 建议补充属性注释
defaultContentProps
作为默认配置对象,建议添加 JSDoc 注释说明每个属性的用途,以提高代码可维护性。src/packages/dialog/content.taro.tsx (1)
28-29
: 优化属性解构顺序建议将
onClick
与其他事件处理相关的属性放在一起,以提高代码的可读性和维护性。src/packages/dialog/dialogwrap.tsx (1)
8-17
: 建议优化默认配置的类型安全性
defaultDialogWrapProps
合并了多个默认配置,但缺少对合并后属性的类型检查。建议添加类型断言确保类型安全:
- export const defaultDialogWrapProps: DialogWrapProps = { + export const defaultDialogWrapProps = { ...defaultOverlayProps, ...defaultContentProps, overlay: true, overlayStyle: {}, overlayClassName: 'nut-dialog-overlay', onCancel: () => {}, onClose: () => {}, onOverlayClick: () => true, } as const satisfies DialogWrapPropssrc/packages/toast/demos/taro/demo1.tsx (2)
6-14
: 类型定义可以进一步优化
icon
的类型定义可以更精确,建议使用联合类型限制可能的值。建议修改为:
- icon: string | React.ReactNode + icon: 'text' | 'success' | 'fail' | 'warn' | 'loading' | React.ReactNode
19-24
: 建议添加参数类型注解
openToast
函数的参数类型应该与 state 中的类型保持一致。建议添加完整的类型注解:
const openToast = ( - icon: string | React.ReactNode, + icon: State['icon'], - title?: string, + title?: State['title'], - duration?: number, + duration?: State['duration'], - content?: string + content?: State['content'] ) => {src/packages/overlay/overlay.tsx (1)
54-57
: 建议优化样式对象的创建方式当前的样式对象在每次渲染时都会重新创建,可以考虑使用 useMemo 优化性能。
+const styles = useMemo(() => ({ + ...style, + zIndex, +}), [style, zIndex]) -const styles = { - ...style, - zIndex, -}src/packages/dialog/types.ts (2)
30-30
: 改进联合类型中的 void 使用在联合类型中使用 void 可能会导致混淆,建议使用 undefined 替代。
-onOverlayClick: (e: MouseEvent) => boolean | void +onOverlayClick: (e: MouseEvent) => boolean | undefined -onConfirm?: (e?: MouseEvent<HTMLButtonElement>) => PromiseLike<any> | void +onConfirm?: (e?: MouseEvent<HTMLButtonElement>) => PromiseLike<any> | undefinedAlso applies to: 45-45
🧰 Tools
🪛 Biome (1.9.4)
[error] 30-30: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
68-68
: 完善 destroyList 的类型定义destroyList 的类型定义可以更具体,使用函数字面量类型。
-export const destroyList: Array<() => void> = [] +export const destroyList: Array<{ (): void }> = []src/packages/overlay/overlay.taro.tsx (2)
59-70
: 优化 renderOverlay 函数的性能renderOverlay 函数在每次渲染时都会重新创建,建议使用 useCallback 优化性能。
-const renderOverlay = () => ( +const renderOverlay = useCallback(() => ( <View ref={nodeRef} className={classes} style={styles} {...(rest as any)} catchMove={lockScroll} onClick={handleClick} > {children} </View> - ) +), [nodeRef, classes, styles, rest, lockScroll, handleClick, children])
65-65
: 补充 catchMove 属性的文档说明catchMove 属性缺少注释说明其用途和影响。
+// catchMove 用于阻止页面滚动穿透,在弹出层显示时防止底层页面滚动 catchMove={lockScroll}
src/packages/fixednav/fixednav.tsx (1)
71-73
: 优化事件处理器的性能onClick 事件处理器在每次渲染时都会重新创建,建议使用 useCallback 优化。
+const handleOverlayClick = useCallback(() => onChange(false), [onChange]); -onClick={() => onChange(false)} +onClick={handleOverlayClick}src/packages/fixednav/fixednav.taro.tsx (2)
10-24
: 建议将默认属性值移至配置文件建议将
zIndex
的默认值200
移至统一的配置文件中管理,以便于后续维护和统一调整。- zIndex: 200, + zIndex: config.zIndex.fixednav,
73-73
: 建议增加事件处理器的类型安全性直接调用事件处理函数可能存在类型安全问题,建议添加适当的类型定义:
- onClick={() => onChange(false)} + onClick={(e: React.MouseEvent) => onChange(false)} - onClick={(event) => onSelect(item, event as any)} + onClick={(event: React.MouseEvent) => onSelect(item, event)}Also applies to: 83-83
src/packages/toast/Notification.tsx (1)
Line range hint
21-30
: 建议优化构造函数中的事件绑定当前的事件绑定方式可能导致不必要的重新绑定,建议使用箭头函数:
- constructor(props: ToastProps) { - super(props) - this.close = this.close.bind(this) - this.startCloseTimer = this.startCloseTimer.bind(this) - this.clearCloseTimer = this.clearCloseTimer.bind(this) - this.clickCover = this.clickCover.bind(this) - this.state = { - show: true, - } - } + state = { + show: true, + } + + close = () => { + // ... existing code + } + + startCloseTimer = () => { + // ... existing code + } + + clearCloseTimer = () => { + // ... existing code + } + + clickCover = () => { + // ... existing code + }src/packages/dialog/dialog.tsx (1)
Line range hint
16-32
: 建议优化默认属性配置建议将
zIndex
的默认值移至配置文件,并考虑使用常量对象管理所有默认值:+ const DIALOG_DEFAULTS = { + Z_INDEX: 1200, + FOOTER_DIRECTION: 'horizontal', + CLOSE_ICON_POSITION: 'bottom' + } as const; const defaultProps: DialogBasicProps = { ...defaultDialogWrapProps, confirmText: '', cancelText: '', overlay: true, closeOnOverlayClick: true, hideConfirmButton: false, hideCancelButton: false, disableConfirmButton: false, - footerDirection: 'horizontal', + footerDirection: DIALOG_DEFAULTS.FOOTER_DIRECTION, lockScroll: true, - closeIconPosition: 'bottom', + closeIconPosition: DIALOG_DEFAULTS.CLOSE_ICON_POSITION, closeIcon: false, - zIndex: 1200, + zIndex: DIALOG_DEFAULTS.Z_INDEX, beforeCancel: () => true, beforeClose: () => true, }src/packages/shortpassword/shortpassword.tsx (1)
Line range hint
95-102
: 建议优化 format 函数的调用时机当前的 format 函数调用可能导致重复计算:
useEffect(() => { + const formattedValue = format(value) - const val = format(value) - if (val.length >= comLen) { + if (formattedValue.length >= comLen) { - onComplete && onComplete(val) + onComplete?.(formattedValue) } - setInputValue(format(value)) + setInputValue(formattedValue) }, [value])src/packages/toast/toast.taro.tsx (1)
127-141
: 改进图标渲染逻辑新增的renderIcon函数使图标渲染逻辑更加清晰和可维护。建议添加类型注释以提高代码的可读性。
- const renderIcon = () => { + const renderIcon = (): React.ReactNode => {src/packages/popup/popup.tsx (1)
88-89
: 改进zIndex处理方式使用直接的zIndex属性替代CSS变量是个好的改进,但建议添加注释说明迁移时间表。
- // 逐步废弃掉,可直接使用 zIndex 替换 + // TODO: '--nutui-overlay-zIndex' 将在下一个主版本中移除,请使用 zIndex 属性代替src/packages/popup/popup.taro.tsx (1)
88-89
: 统一Web和Taro版本的zIndex处理zIndex处理方式的改进与Web版本保持一致,建议同样添加详细的迁移说明。
- // 逐步废弃掉,可直接使用 zIndex 替换 + // TODO: '--nutui-overlay-zIndex' 将在下一个主版本中移除,请使用 zIndex 属性代替src/packages/dialog/dialog.taro.tsx (1)
187-189
: 建议使用可选链优化代码!当前的条件判断可以使用可选链来简化,使代码更加简洁。
- const closed = onOverlayClick && onOverlayClick(e) - closed && onClose && onClose() - closed && onCancel && onCancel() + const closed = onOverlayClick?.(e) + if (closed) { + onClose?.() + onCancel?.() + }🧰 Tools
🪛 Biome (1.9.4)
[error] 187-187: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/toast/toast.tsx (1)
43-43
: 建议使用可选链优化代码在
close
函数中,您可以将opts.onClose && opts.onClose()
改为opts.onClose?.()
,使代码更简洁。应用以下修改:
- opts.onClose && opts.onClose() + opts.onClose?.()🧰 Tools
🪛 Biome (1.9.4)
[error] 43-43: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (52)
src/config.json
(2 hunks)src/packages/cascader/cascader.taro.tsx
(1 hunks)src/packages/cascader/cascader.tsx
(1 hunks)src/packages/dialog/config.ts
(0 hunks)src/packages/dialog/confirm.tsx
(1 hunks)src/packages/dialog/content.taro.tsx
(2 hunks)src/packages/dialog/content.tsx
(4 hunks)src/packages/dialog/dialog.scss
(0 hunks)src/packages/dialog/dialog.taro.tsx
(6 hunks)src/packages/dialog/dialog.tsx
(4 hunks)src/packages/dialog/dialogwrap.tsx
(4 hunks)src/packages/dialog/index.taro.ts
(1 hunks)src/packages/dialog/index.ts
(1 hunks)src/packages/dialog/types.ts
(1 hunks)src/packages/fixednav/demos/h5/demo1.tsx
(1 hunks)src/packages/fixednav/demos/taro/demo1.tsx
(1 hunks)src/packages/fixednav/fixednav.taro.tsx
(6 hunks)src/packages/fixednav/fixednav.tsx
(5 hunks)src/packages/fixednav/index.taro.ts
(1 hunks)src/packages/fixednav/index.ts
(1 hunks)src/packages/fixednav/types.ts
(2 hunks)src/packages/numberkeyboard/numberkeyboard.taro.tsx
(1 hunks)src/packages/numberkeyboard/numberkeyboard.tsx
(1 hunks)src/packages/overlay/demos/h5/demo1.tsx
(1 hunks)src/packages/overlay/demos/h5/demo2.tsx
(1 hunks)src/packages/overlay/demos/taro/demo1.tsx
(1 hunks)src/packages/overlay/overlay.taro.tsx
(4 hunks)src/packages/overlay/overlay.tsx
(2 hunks)src/packages/overlay/types.ts
(1 hunks)src/packages/picker/picker.taro.tsx
(1 hunks)src/packages/picker/picker.tsx
(1 hunks)src/packages/popover/popover.taro.tsx
(1 hunks)src/packages/popover/popover.tsx
(1 hunks)src/packages/popup/index.taro.ts
(1 hunks)src/packages/popup/index.ts
(1 hunks)src/packages/popup/popup.taro.tsx
(4 hunks)src/packages/popup/popup.tsx
(3 hunks)src/packages/popup/types.ts
(1 hunks)src/packages/shortpassword/shortpassword.taro.tsx
(1 hunks)src/packages/shortpassword/shortpassword.tsx
(1 hunks)src/packages/toast/Notification.tsx
(2 hunks)src/packages/toast/demos/taro/demo1.tsx
(2 hunks)src/packages/toast/demos/taro/demo2.tsx
(0 hunks)src/packages/toast/demos/taro/demo3.tsx
(1 hunks)src/packages/toast/demos/taro/demo4.tsx
(1 hunks)src/packages/toast/demos/taro/demo5.tsx
(1 hunks)src/packages/toast/index.taro.ts
(1 hunks)src/packages/toast/index.ts
(1 hunks)src/packages/toast/toast.scss
(0 hunks)src/packages/toast/toast.taro.tsx
(6 hunks)src/packages/toast/toast.tsx
(3 hunks)src/packages/toast/types.ts
(1 hunks)
💤 Files with no reviewable changes (4)
- src/packages/dialog/dialog.scss
- src/packages/toast/toast.scss
- src/packages/toast/demos/taro/demo2.tsx
- src/packages/dialog/config.ts
✅ Files skipped from review due to trivial changes (8)
- src/packages/popup/index.ts
- src/packages/picker/picker.tsx
- src/packages/popover/popover.tsx
- src/packages/cascader/cascader.tsx
- src/packages/popover/popover.taro.tsx
- src/packages/dialog/confirm.tsx
- src/packages/picker/picker.taro.tsx
- src/packages/numberkeyboard/numberkeyboard.taro.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/popup/types.ts
[error] 22-22: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 23-23: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/types.ts
[error] 30-30: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 45-45: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/dialog.taro.tsx
[error] 187-187: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/toast/toast.tsx
[error] 43-43: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/overlay/overlay.tsx
[error] 61-61: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 GitHub Check: build
src/packages/overlay/overlay.tsx
[failure] 61-61:
Argument of type 'MouseEvent<Element, MouseEvent>' is not assignable to parameter of type 'TouchEvent'.
🔇 Additional comments (24)
src/packages/toast/index.ts (1)
5-8
: 类型导出重组看起来不错!将类型定义集中到专门的
types
文件中是一个很好的实践,这样可以提高代码的可维护性和可重用性。src/packages/fixednav/index.ts (1)
3-8
: 类型导出结构优化合理!将所有相关的类型定义(
FixedNavType
、FixedNavPosition
、FixedNavItem
、FixedNavProps
)统一从./types
导出,使得类型系统更加清晰和易于维护。src/packages/fixednav/index.taro.ts (1)
3-8
: Taro版本的类型导出同步更新,保持一致性!将类型定义统一从
./types
导出,与非Taro版本保持一致,这样的同步更新有助于维护跨平台的类型系统一致性。src/packages/dialog/index.ts (1)
Line range hint
3-9
: 类型定义迁移需要验证完整性!将类型从
./config
迁移到./types
是个好的改进。但是注意到DialogProps
的导出被移除了,需要确认这个改动不会影响现有的代码。请运行以下脚本来验证
DialogProps
的使用情况:✅ Verification successful
类型
DialogProps
的移除是安全的!通过代码库扫描验证,没有发现任何地方使用
DialogProps
类型。这表明移除该类型导出是安全的,不会影响现有代码。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:检查是否还有代码在使用已移除的 DialogProps 类型 # 搜索所有可能使用 DialogProps 的地方 rg "DialogProps" --type ts --type tsxLength of output: 65
Script:
#!/bin/bash # 检查 .ts 和 .tsx 文件中的 DialogProps 使用情况 echo "=== Searching in .ts files ===" rg "DialogProps" --type ts echo -e "\n=== Searching in source files using ast-grep ===" ast-grep --pattern 'import { $$$DialogProps$$$ } from' ast-grep --pattern 'type $_ = DialogProps'Length of output: 312
src/packages/toast/index.taro.ts (1)
3-10
: 类型导出重组结构合理!类型导出的重组提高了代码的组织性,将相关类型集中在
types.ts
文件中管理,符合类型抽取的重构目标。src/packages/overlay/demos/h5/demo1.tsx (1)
22-22
: 从样式变量迁移到属性配置的改进!将 z-index 从 CSS 变量迁移到组件属性是一个很好的改进,这样可以:
- 提供更直接的属性控制
- 避免 CSS 变量可能带来的特异性问题
- 使配置更加清晰明了
src/packages/toast/demos/taro/demo5.tsx (1)
19-19
: 类型更改符合重构方向将
type
属性更改为icon
的修改符合组件 API 的统一性要求。src/packages/dialog/dialogwrap.tsx (1)
46-56
: zIndex 修复实现正确正确地将
zIndex
属性传递给Overlay
组件,解决了固定遮罩层 zIndex 无效的问题。src/packages/overlay/overlay.tsx (1)
47-49
: 验证 useEffect 的简化是否会影响边缘情况useEffect 的逻辑被简化,可能会跳过一些重要的边缘情况检查。
src/packages/fixednav/fixednav.tsx (1)
20-20
: 验证 zIndex 值的合理性需要确保 zIndex 值在组件层级中的合理性,当前值为 200。
✅ Verification successful
zIndex 值验证完成,当前设置合理
固定导航栏组件使用 zIndex: 200 的设置是合理的,符合组件层级设计规范:
- 位于基础UI组件之上
- 位于模态框、弹出层等覆盖性组件之下
- 与其他导航类组件保持协调
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 搜索其他组件中的 zIndex 值,确保层级合理 rg "zIndex.*[0-9]+" --type ts --type tsxLength of output: 70
Script:
#!/bin/bash # Search for zIndex values in all relevant files rg "z[Ii]ndex.*?[0-9]+" -g "*.{ts,tsx,js,jsx}" # Also search for CSS z-index rg "z-index.*?[0-9]+" -g "*.{css,less,scss,sass,style*}"Length of output: 8249
src/packages/shortpassword/shortpassword.taro.tsx (1)
11-11
: 导入语句优化导入路径的更改使类型导入更加直接和清晰。
src/packages/toast/toast.taro.tsx (3)
6-7
: 引入默认覆盖层属性从overlay模块引入默认属性,提高了代码的可维护性。
20-37
: 优化默认属性配置默认属性现在包含了明确的zIndex值,并且使用了更简洁的事件处理函数。
Line range hint
155-189
: 优化覆盖层实现Overlay组件的实现现在更好地处理了zIndex属性。
src/packages/dialog/dialog.taro.tsx (4)
8-9
: 导入语句重构得当!将类型定义移至专门的 types 文件并引入子组件的默认属性是个很好的做法,这样可以提高代码的可维护性和复用性。
Also applies to: 20-20
22-46
: 默认属性设置合理!通过组合 defaultOverlayProps 和 defaultContentProps 来设置默认属性,同时添加了 overlayStyle、overlayClassName 和 zIndex 等配置项,使组件更加灵活可配置。
93-95
: 事件处理逻辑简化得当!将 useCustomEvent 中的逻辑简化为直接设置状态,减少了不必要的条件分支,使代码更加清晰。
200-210
: 遮罩层配置增强合理!为 Overlay 组件添加了 zIndex、style 和 className 属性,增强了遮罩层的可定制性,使组件更加灵活。
src/packages/cascader/cascader.taro.tsx (1)
13-13
: 导入路径调整合理!将 Popup 组件的导入路径从 popup.taro 改为 index.taro,符合项目的模块化结构。
src/packages/fixednav/demos/h5/demo1.tsx (1)
36-37
: 示例代码优化合理!
- 改进了控制台日志的可读性
- 选择后自动关闭导航栏,提升了用户体验
- 添加 zIndex 属性展示了组件的配置能力
这些改动使示例更加完整和实用。
Also applies to: 40-49
src/packages/numberkeyboard/numberkeyboard.tsx (1)
4-4
: 导入语句合并恰当!将 Popup 组件和 PopupProps 类型的导入合并为一行,提高了代码的简洁性。
src/config.json (1)
Line range hint
317-326
: 版本升级和文档状态变更需要确认组件版本从 2.0.0 升级到 3.0.0,同时文档状态从
dd: false
变更为dd: true
。请确认:
- 版本升级是否符合语义化版本规范
- 文档是否已完善并准备就绪
运行以下脚本以验证版本升级的合理性:
✅ Verification successful
版本升级和文档状态变更合理
根据变更历史,组件进行了以下重大改动:
- 类型系统重构 (refactor: 类型文件统一为 types,增加 fixednavitem 类型 #1789)
- CSS 命名规范和变量修订 (style(fixednav): 修订 样式名称和 CSS 变量 #1702)
- onSelect 参数顺序调整 (fix(fixednav): 修改 onSelect 参数顺序 #1035)
这些变更符合语义化版本的主版本升级要求,将文档状态设置为就绪(dd: true)也是合适的。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:检查 FixedNav 组件的破坏性变更 # 检查是否存在破坏性 API 变更 ast-grep --pattern 'interface FixedNavProps { $$$ }' # 检查更新日志中是否记录了相关变更 rg -i "fixednav|breaking changes" CHANGELOG.md || echo "Warning: No changelog entry found"Length of output: 520
Script:
#!/bin/bash # Check when these changes were merged gh pr view 1035 --json mergedAt,title || echo "PR #1035 not found" gh pr view 1702 --json mergedAt,title || echo "PR #1702 not found" gh pr view 1789 --json mergedAt,title || echo "PR #1789 not found"Length of output: 365
src/packages/popup/index.taro.ts (1)
3-3
: 类型导出路径更新正确将
PopupProps
的导出路径修改为从'./types'
导出,有助于类型定义的集中管理。src/packages/dialog/index.taro.ts (1)
3-3
: 更新类型导出路径以提高模块化将
DialogBasicProps
从'./types'
导出,有助于加强类型定义的模块化和维护性。
src/packages/toast/types.ts
Outdated
export interface ToastProps extends OverlayProps { | ||
id: string | ||
position: ToastPosition | ||
title: string | ||
size: string | number | ||
icon: ToastIcon | ||
content: React.ReactNode | ||
contentClassName: string | ||
contentStyle: React.CSSProperties | ||
wordBreak: ToastWordBreak | ||
onClose: () => void | ||
} |
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.
🛠️ Refactor suggestion
建议优化 Props 接口的类型定义
接口定义可以进行以下改进:
- 部分属性应该是可选的
- size 属性类型不够精确
- 缺少属性的默认值说明
建议修改如下:
export interface ToastProps extends OverlayProps {
id: string
- position: ToastPosition
+ position?: ToastPosition
title: string
- size: string | number
+ size?: ToastSize
- icon: ToastIcon
+ icon?: ToastIcon
content: React.ReactNode
- contentClassName: string
+ contentClassName?: string
- contentStyle: React.CSSProperties
+ contentStyle?: React.CSSProperties
- wordBreak: ToastWordBreak
+ wordBreak?: ToastWordBreak
onClose: () => void
}
Committable suggestion skipped: line range outside the PR's diff.
src/packages/fixednav/types.ts
Outdated
onChange: (item: any) => void | ||
onSelect: ( | ||
item: any, | ||
event: React.MouseEvent<HTMLDivElement, MouseEvent> | ||
) => void |
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.
🛠️ Refactor suggestion
建议改进类型安全性
onChange
和 onSelect
回调函数使用了 any
类型,这可能导致类型安全问题。建议:
- 将
any
替换为具体的类型定义 - 使用
FixedNavItem
类型替代any
- onChange: (item: any) => void
+ onChange: (item: FixedNavItem) => void
- onSelect: (
- item: any,
- event: React.MouseEvent<HTMLDivElement, MouseEvent>
- ) => void
+ onSelect: (
+ item: FixedNavItem,
+ event: React.MouseEvent<HTMLDivElement, MouseEvent>
+ ) => void
Committable suggestion skipped: line range outside the PR's diff.
src/packages/popup/types.ts
Outdated
onOverlayClick: (e: TouchEvent) => boolean | void | ||
onCloseIconClick: (e: TouchEvent) => boolean | void |
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.
🛠️ Refactor suggestion
需要改进事件处理器的返回类型定义
根据静态分析工具的提示,当前实现存在以下问题:
- 在联合类型中使用
void
可能造成混淆 - 事件处理器的返回类型可以更精确
建议修改为:
- onOverlayClick: (e: TouchEvent) => boolean | void
- onCloseIconClick: (e: TouchEvent) => boolean | void
+ onOverlayClick: (e: TouchEvent) => boolean | undefined
+ onCloseIconClick: (e: TouchEvent) => boolean | undefined
📝 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.
onOverlayClick: (e: TouchEvent) => boolean | void | |
onCloseIconClick: (e: TouchEvent) => boolean | void | |
onOverlayClick: (e: TouchEvent) => boolean | undefined | |
onCloseIconClick: (e: TouchEvent) => boolean | undefined |
🧰 Tools
🪛 Biome (1.9.4)
[error] 22-22: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 23-23: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/popup/types.ts
Outdated
position: string | ||
transition: string | ||
overlayStyle: React.CSSProperties | ||
overlayClassName: string | ||
closeable: boolean | ||
closeIconPosition: string | ||
closeIcon: ReactNode | ||
left?: ReactNode | ||
title?: ReactNode | ||
description?: ReactNode | ||
destroyOnClose: boolean | ||
portal: Teleport | ||
overlay: boolean | ||
round: boolean |
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.
🛠️ Refactor suggestion
建议优化属性定义
一些建议:
- 部分属性应该是可选的
- 需要为 position 和 closeIconPosition 定义具体的字面量类型
- TouchEvent 应该使用 React.TouchEvent
建议修改为:
export interface PopupProps extends OverlayProps {
- position: string
+ position: 'top' | 'bottom' | 'left' | 'right' | 'center'
- transition: string
+ transition?: string
- overlayStyle: React.CSSProperties
+ overlayStyle?: React.CSSProperties
- overlayClassName: string
+ overlayClassName?: string
- closeable: boolean
+ closeable?: boolean
- closeIconPosition: string
+ closeIconPosition?: 'top-left' | 'top-right' | 'bottom-left' | 'bottom-right'
- closeIcon: ReactNode
+ closeIcon?: ReactNode
// ... other properties
}
📝 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.
position: string | |
transition: string | |
overlayStyle: React.CSSProperties | |
overlayClassName: string | |
closeable: boolean | |
closeIconPosition: string | |
closeIcon: ReactNode | |
left?: ReactNode | |
title?: ReactNode | |
description?: ReactNode | |
destroyOnClose: boolean | |
portal: Teleport | |
overlay: boolean | |
round: boolean | |
position: 'top' | 'bottom' | 'left' | 'right' | 'center' | |
transition?: string | |
overlayStyle?: React.CSSProperties | |
overlayClassName?: string | |
closeable?: boolean | |
closeIconPosition?: 'top-left' | 'top-right' | 'bottom-left' | 'bottom-right' | |
closeIcon?: ReactNode | |
left?: ReactNode | |
title?: ReactNode | |
description?: ReactNode | |
destroyOnClose: boolean | |
portal: Teleport | |
overlay: boolean | |
round: boolean |
const changeState = Object.assign(state, { | ||
type, | ||
icon, | ||
content, | ||
duration, | ||
icon, | ||
title, | ||
}) |
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.
避免直接修改状态对象
使用 Object.assign
直接修改状态对象可能导致意外的副作用。建议使用展开运算符创建新的状态对象。
- const changeState = Object.assign(state, {
- icon,
- content,
- duration,
- title,
- })
- setState(changeState)
+ setState({
+ ...state,
+ icon,
+ content,
+ duration,
+ title,
+ })
Committable suggestion skipped: line range outside the PR's diff.
const selected = (item: any, event: any) => { | ||
console.log(item, event) | ||
} |
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.
🛠️ Refactor suggestion
需要改进类型安全性和日志处理
- 不建议使用
any
类型,应该保持具体的事件类型定义 - 生产环境代码中不应该保留 console.log 语句
- const selected = (item: any, event: any) => {
- console.log(item, event)
+ const selected = (
+ item: {
+ id: number
+ text: string
+ icon: string
+ num?: number
+ },
+ event: React.MouseEvent<Element, MouseEvent>
+ ) => {
+ // 处理选中逻辑
Committable suggestion skipped: line range outside the PR's diff.
src/packages/dialog/content.tsx
Outdated
footer && ( | ||
<div | ||
className={classNames(`${classPrefix}-footer`, { | ||
[footerDirection as any]: footerDirection, |
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.
🛠️ Refactor suggestion
类型转换需要改进
使用 as any
进行类型转换是不安全的做法,可能会在运行时引发问题。
建议定义一个明确的类型:
- [footerDirection as any]: footerDirection,
+ [footerDirection as 'horizontal' | 'vertical']: footerDirection,
📝 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.
[footerDirection as any]: footerDirection, | |
[footerDirection as 'horizontal' | 'vertical']: footerDirection, |
const handleClick = (e: MouseEvent) => { | ||
if (closeOnOverlayClick) { | ||
onClick && onClick(e) | ||
} | ||
} |
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.
修复事件处理器类型不匹配问题
handleClick 函数的事件类型与 onClick 回调的类型不匹配。onClick 期望接收 TouchEvent,但实际传入的是 MouseEvent。
建议修改为:
-const handleClick = (e: MouseEvent) => {
+const handleClick = (e: TouchEvent) => {
if (closeOnOverlayClick) {
onClick && onClick(e)
}
}
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 61-61: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 GitHub Check: build
[failure] 61-61:
Argument of type 'MouseEvent<Element, MouseEvent>' is not assignable to parameter of type 'TouchEvent'.
src/packages/dialog/types.ts
Outdated
footer: ReactNode | ||
close: ReactNode | ||
footerDirection: string | ||
onClick: (event: any) => void |
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.
🛠️ Refactor suggestion
优化 onClick 事件处理器类型定义
onClick 的类型定义过于宽松,使用 any 可能导致类型安全问题。
-onClick: (event: any) => void
+onClick: (event: MouseEvent<HTMLElement>) => void
Committable suggestion skipped: line range outside the PR's diff.
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: 1
🧹 Nitpick comments (11)
src/packages/overlay/__tests__/overlay.spec.tsx (1)
16-16
: 快照更新正确反映了新的 zIndex 实现方式内联快照已正确更新,显示了直接的 z-index 样式属性,这与新的实现方式保持一致。
建议:考虑添加更多的测试用例来验证不同 zIndex 值的边界情况。
src/packages/popup/popup.tsx (1)
Line range hint
18-36
: 默认属性的类型定义需要改进默认属性的定义方式合理,但建议进行以下改进:
- 事件处理函数的返回值类型应该明确定义为
boolean
- 建议为所有可选属性提供明确的默认值,以提高代码的可预测性
const defaultProps: PopupProps = { ...ComponentDefaults, position: 'center', transition: '', - onOverlayClick: () => true, - onCloseIconClick: () => true, + onOverlayClick: (): boolean => true, + onCloseIconClick: (): boolean => true, ...defaultOverlayProps, }src/packages/fixednav/fixednav.tsx (4)
9-23
: 优化了默认属性的定义和类型通过使用
defaultOverlayProps
提高了组件之间的一致性,并为所有必要的属性提供了合理的默认值。不过建议考虑以下改进:
- 为
list
提供一个空数组作为默认值是好的做法,但建议使用readonly
确保类型安全onChange
和onSelect
的空函数处理可以考虑使用noop
工具函数- list: [], + list: [] as readonly any[], - onChange: () => {}, - onSelect: () => {}, + onChange: noop, + onSelect: noop,
71-76
: 改进了遮罩层的实现遮罩层现在可以正确处理 zIndex,解决了原始问题。事件处理也更加直接和清晰。
建议添加类型断言以提高代码的类型安全性:
- onClick={() => onChange(false)} + onClick={() => onChange(false as const)}
83-85
: 简化了事件处理逻辑事件处理逻辑变得更加直接和清晰,但建议添加类型定义以提高代码的可维护性:
- onClick={(event) => onSelect(item, event)} + onClick={(event: React.MouseEvent<HTMLDivElement>) => onSelect(item, event)}
Line range hint
101-113
: 渲染逻辑清晰且易维护按钮的渲染逻辑和状态切换处理得当:
- 正确使用条件渲染
- 适当处理了国际化文本
- 保持了代码的可读性
建议添加类型断言以提高代码的类型安全性:
- <div className={`${classPrefix}-btn`} onClick={() => onChange(!visible)}> + <div className={`${classPrefix}-btn`} onClick={() => onChange(!visible as boolean)}>src/packages/dialog/content.tsx (3)
5-12
: 建议改进默认值的处理方式对于可选属性,使用空字符串作为默认值可能不是最佳选择。建议考虑使用
undefined
来表示可选值的默认状态。建议如下修改:
export const defaultContentProps: ContentProps = { visible: false, - title: '', - header: '', - footer: '', - close: '', - footerDirection: '', + title: undefined, + header: undefined, + footer: undefined, + close: undefined, + footerDirection: undefined, onClick: () => {}, }
Line range hint
16-30
: 建议加强类型安全性当前的类型定义虽然可以工作,但可以通过以下方式改进:
- 考虑为
onClick
事件处理器添加具体的事件类型- 为合并后的 props 定义明确的类型接口
建议如下修改:
+ type ContentEventHandler = (e: React.MouseEvent<HTMLDivElement>) => void; + + interface MergedContentProps extends Omit<HTMLAttributes<HTMLDivElement>, 'title' | 'content'> { + visible?: boolean; + title?: React.ReactNode; + header?: React.ReactNode; + footer?: React.ReactNode; + close?: React.ReactNode; + footerDirection?: string; + onClick?: ContentEventHandler; + } - const { ...props } = { ...defaultContentProps, ...props } + const mergedProps: MergedContentProps = { ...defaultContentProps, ...props }
58-59
: 建议优化性能对于类名的计算和样式的应用,建议使用
useMemo
来避免不必要的重新计算。建议如下修改:
+ const containerClassName = useMemo(() => + classNames(`${classPrefix}-outer`, className), + [className] + ); return ( <div - className={classNames(`${classPrefix}-outer`, className)} + className={containerClassName} style={style} onClick={(e) => handleClick(e)} >src/packages/popup/popup.taro.tsx (2)
13-13
: 类型定义重构和导入优化将类型定义移至单独的文件中是一个很好的重构,这样可以提高代码的可维护性和复用性。建议在
types.ts
文件中添加详细的类型文档说明。建议在
types.ts
中添加以下文档:/** * Popup 组件的属性定义 * @interface PopupProps * @property {string} position - 弹出位置,可选值:'center' | 'top' | 'bottom' | 'left' | 'right' * ... */Also applies to: 17-17
Line range hint
215-223
: 优化 zIndex 处理逻辑直接传递 zIndex 属性给 Overlay 组件是个好的改进,但建议考虑以下几点:
- 多个弹窗同时存在时的 zIndex 管理
- 性能优化:避免不必要的重渲染
建议添加 zIndex 管理工具类:
// zIndexManager.ts class ZIndexManager { private static baseIndex = 1000; private static stack: number[] = []; static getNextIndex(): number { const index = this.baseIndex + this.stack.length; this.stack.push(index); return index; } static releaseIndex(index: number) { this.stack = this.stack.filter(i => i !== index); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
src/packages/address/__test__/__snapshots__/address.spec.tsx.snap
is excluded by!**/*.snap
src/packages/cascader/__tests__/__snapshots__/cascader.spec.tsx.snap
is excluded by!**/*.snap
src/packages/numberkeyboard/_test_/__snapshots__/index.spec.tsx.snap
is excluded by!**/*.snap
src/packages/overlay/__tests__/__snapshots__/overlay.spec.tsx.snap
is excluded by!**/*.snap
src/packages/timeselect/__test__/__snapshots__/timeselect.spec.tsx.snap
is excluded by!**/*.snap
📒 Files selected for processing (7)
src/packages/dialog/content.tsx
(4 hunks)src/packages/dialog/dialog.tsx
(4 hunks)src/packages/fixednav/fixednav.tsx
(5 hunks)src/packages/fixednav/types.ts
(2 hunks)src/packages/overlay/__tests__/overlay.spec.tsx
(1 hunks)src/packages/popup/popup.taro.tsx
(4 hunks)src/packages/popup/popup.tsx
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/packages/fixednav/types.ts
- src/packages/dialog/dialog.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (7)
src/packages/overlay/__tests__/overlay.spec.tsx (1)
9-9
: 使用 zIndex 属性替换样式属性是个很好的改进!这个改动使得 z-index 的设置更加直接和明确,避免了使用 CSS 变量可能带来的问题。这与修复 overlay zIndex 无效的 PR 目标完全一致。
src/packages/popup/popup.tsx (2)
12-12
: 导入语句重构得当!导入语句的重构和类型的分离使代码结构更加清晰。
Also applies to: 16-16
Line range hint
211-219
: Overlay组件的zIndex处理得当zIndex的处理方式合理,确保了弹出层的正确层叠顺序。
src/packages/fixednav/fixednav.tsx (1)
Line range hint
25-49
: 属性结构定义清晰合理组件属性的类型定义和解构处理符合最佳实践:
- 正确使用
Omit
处理 HTML 原生属性- 清晰地分离了组件特有属性和原生属性
- 保持了类型安全性
src/packages/dialog/content.tsx (2)
1-3
: 导入声明看起来不错!导入声明简洁明了,使用了适当的类型。
39-49
: 类型转换需要改进使用
as any
进行类型转换是不安全的做法,可能会在运行时引发问题。src/packages/popup/popup.taro.tsx (1)
Line range hint
19-37
: 验证默认属性的完整性和正确性默认属性的类型定义更加明确,但需要确保与 overlay 组件的默认属性正确集成。
请运行以下脚本验证属性集成的完整性:
✅ Verification successful
默认属性集成验证完成,配置正确
经过验证,Popup 组件的默认属性已正确集成了 Overlay 组件的所有必要属性,包括:
- 通过
...defaultOverlayProps
继承了所有 overlay 基础属性- 正确设置了类型注解
- 保持了类型安全
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查 overlay 组件的所有属性是否都在 PopupProps 中得到正确处理 ast-grep --pattern 'interface OverlayProps { $$$ }' # 检查 defaultOverlayProps 的所有属性 ast-grep --pattern 'const defaultOverlayProps = { $$$ }'Length of output: 116
Script:
#!/bin/bash # Find overlay-related files fd overlay # Search for overlay interface definitions with more flexible pattern ast-grep --pattern 'interface $_Props { $$$ }' # Search for overlay-related constants rg "const.*[Oo]verlay.*=" -A 5Length of output: 12155
Script:
#!/bin/bash # Get the OverlayProps interface definition rg "interface OverlayProps" -A 10 src/packages/overlay/Length of output: 610
src/packages/popup/popup.tsx
Outdated
const handleOverlayClick = (e: any) => { | ||
e.stopPropagation() | ||
if (closeOnOverlayClick && onOverlayClick(e)) { | ||
close() | ||
} | ||
} |
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.
事件处理函数的类型安全性问题
当前的实现使用 any
类型作为事件参数,这降低了类型安全性。建议使用更具体的事件类型。
- const handleOverlayClick = (e: any) => {
+ const handleOverlayClick = (e: React.MouseEvent) => {
e.stopPropagation()
if (closeOnOverlayClick && onOverlayClick(e)) {
close()
}
}
- const handleCloseIconClick = (e: any) => {
+ const handleCloseIconClick = (e: React.MouseEvent) => {
onCloseIconClick(e) && close()
}
Also applies to: 128-131
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #2954 +/- ##
=============================================
+ Coverage 85.59% 85.74% +0.14%
=============================================
Files 277 277
Lines 18158 18110 -48
Branches 2760 2745 -15
=============================================
- Hits 15543 15528 -15
+ Misses 2610 2577 -33
Partials 5 5 ☔ 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
🧹 Nitpick comments (7)
src/packages/toast/types.ts (1)
4-11
: 建议添加类型文档并优化 ToastIcon 类型定义
- 建议为每个类型添加 JSDoc 文档注释,说明用途和示例
- ToastIcon 类型混合了字符串字面量和 React.ReactNode,建议拆分为两个类型以提高类型安全性
建议修改如下:
+/** Toast 图标的预设值类型 */ +export type ToastIconPreset = 'success' | 'fail' | 'loading' | 'warn' +/** Toast 图标类型,可以是预设值或自定义 React 节点 */ -export type ToastIcon = - | 'success' - | 'fail' - | 'loading' - | 'warn' - | React.ReactNode +export type ToastIcon = ToastIconPreset | React.ReactNode +/** Toast 文本换行方式 */ export type ToastWordBreak = 'normal' | 'break-all' | 'break-word' +/** Toast 尺寸 */ export type ToastSize = 'small' | 'base' | 'large'src/packages/fixednav/fixednav.tsx (1)
84-84
: 建议增加类型安全性建议为 item 参数添加具体的类型定义,避免使用 any 类型。这样可以在编译时捕获潜在的类型错误。
-{list.map((item: any, index) => { +interface NavItem { + id?: string | number; + icon: string | React.ReactElement; + text: string; + num?: number; +} +{list.map((item: NavItem, index) => {src/packages/fixednav/fixednav.taro.tsx (1)
83-83
: 建议改进事件类型处理当前使用
as any
进行类型转换不够严谨。建议使用 Taro 提供的事件类型定义。-onClick={(event) => onSelect(item, event as any)} +import { ITouchEvent } from '@tarojs/components'; +onClick={(event: ITouchEvent) => onSelect(item, event)}src/packages/popup/popup.taro.tsx (1)
Line range hint
18-35
: 建议为默认处理函数添加注释说明默认属性的类型定义更加明确了,这很好。不过建议:
- 为
onOverlayClick
和onCloseIconClick
的默认实现添加注释,说明返回true
的含义- 考虑将这些回调函数的返回值类型显式声明为
boolean
const defaultProps: PopupProps = { ...defaultOverlayProps, // ... other props ... + // 返回 true 表示允许关闭弹窗 onOverlayClick: () => true, + // 返回 true 表示允许关闭弹窗 onCloseIconClick: () => true, }src/packages/popup/popup.tsx (3)
32-33
: 事件处理器默认返回值的改进将
onOverlayClick
和onCloseIconClick
的默认实现简化为返回true
是一个合理的改进,这样更清晰地表达了默认行为。但建议添加注释说明这个返回值的含义和用途。- onOverlayClick: () => true, - onCloseIconClick: () => true, + // 返回 true 表示允许关闭弹窗 + onOverlayClick: () => true, + // 返回 true 表示允许关闭弹窗 + onCloseIconClick: () => true,
Line range hint
241-241
: 避免不必要的类型转换使用
as Teleport
进行类型转换可能会掩盖潜在的类型问题。建议重构代码以避免这种显式类型转换。- return <>{renderToContainer(portal as Teleport, renderNode())}</> + return <>{renderToContainer(portal, renderNode())}</>同时在
renderToContainer
的类型定义中直接使用可选类型:const renderToContainer = (getContainer: Teleport | undefined, node: ReactElement)
Line range hint
1-241
: 组件结构优化建议当前组件承担了多个职责(弹窗、标题渲染、传送门等),建议将其拆分为更小的组件以提高可维护性:
- 将标题渲染逻辑抽取为独立组件
- 将传送门逻辑抽取为独立的 hook
- 考虑使用 Context 来管理 zIndex
这样可以:
- 提高代码的可测试性
- 降低单个组件的复杂度
- 提高代码的复用性
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/packages/fixednav/fixednav.taro.tsx
(6 hunks)src/packages/fixednav/fixednav.tsx
(5 hunks)src/packages/fixednav/types.ts
(2 hunks)src/packages/overlay/overlay.taro.tsx
(4 hunks)src/packages/overlay/types.ts
(1 hunks)src/packages/popup/popup.taro.tsx
(4 hunks)src/packages/popup/popup.tsx
(4 hunks)src/packages/popup/types.ts
(1 hunks)src/packages/toast/types.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/packages/overlay/types.ts
- src/packages/fixednav/types.ts
- src/packages/popup/types.ts
- src/packages/overlay/overlay.taro.tsx
🔇 Additional comments (13)
src/packages/toast/types.ts (2)
1-3
: 导入语句和位置类型定义看起来不错!导入语句正确引用了 OverlayProps,ToastPosition 类型定义清晰明确。
13-24
: 建议将非必需属性标记为可选与之前的评审意见一致,建议将非必需的属性标记为可选类型。
建议修改如下:
export interface ToastProps extends OverlayProps { id: string - position: ToastPosition + position?: ToastPosition title: string - size: ToastSize + size?: ToastSize - icon: ToastIcon + icon?: ToastIcon content: React.ReactNode - contentClassName: string + contentClassName?: string - contentStyle: React.CSSProperties + contentStyle?: React.CSSProperties - wordBreak: ToastWordBreak + wordBreak?: ToastWordBreak onClose: () => void }src/packages/fixednav/fixednav.tsx (3)
9-23
: 默认属性设置合理!使用 defaultOverlayProps 统一了覆盖层的默认属性,并设置了合理的 zIndex 默认值。
43-49
: 属性解构完整且合理!新增的 zIndex 属性解构,以及与默认值的合并处理都很规范。
72-75
: Overlay 组件配置优化得当!zIndex 属性正确传递给 Overlay 组件,事件处理逻辑简化合理。
src/packages/fixednav/fixednav.taro.tsx (3)
1-24
: 导入和默认配置保持一致性!与 Web 版本保持了一致的配置,同时正确使用了 Taro 特定的组件。
71-74
: Overlay 实现与 Web 版本保持一致!zIndex 属性处理方式统一,正确使用了 Taro 的 View 组件。
Line range hint
62-114
: View 组件使用规范!正确使用了 Taro 的 View 组件替代 div,样式和类名处理得当。
src/packages/popup/popup.taro.tsx (2)
13-16
: 代码结构优化值得肯定!将类型定义移至单独的类型文件中是一个很好的实践,这样可以:
- 提高代码的可维护性
- 减少循环依赖的风险
- 使类型定义更容易重用
Line range hint
213-221
: 请验证 zIndex 在各种场景下的表现zIndex 的直接传递解决了原来的问题,但建议验证以下场景:
- 多个弹窗同时展示时的层级关系
- 不同位置(position)的弹窗层级
- 自定义 zIndex 是否正确覆盖默认值
✅ Verification successful
zIndex 实现验证完成,确认无问题
经过源码分析,当前 zIndex 实现完全满足需求:
- Overlay 组件默认 zIndex 为 1000,确保基础层级关系
- 支持自定义 zIndex 值,已通过单元测试验证
- Taro 和 Web 版本实现一致,都支持正确的层级控制
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查是否存在其他使用 zIndex 的地方 rg -l "zIndex|z-index" --type tsx --type css # 检查是否有其他组件也在处理弹窗层级 ast-grep --pattern 'style={$_zIndex$_}'Length of output: 116
Script:
#!/bin/bash # 搜索所有 zIndex 和 z-index 的使用 rg -t ts -t tsx -t css "zIndex|z-index" -A 3 # 查找 Overlay 组件的实现 fd -e ts -e tsx -e css Overlay # 查找组件中的样式相关属性 ast-grep --pattern 'interface $_Props { $$$ style?: $_ $$$ }'Length of output: 177
Script:
#!/bin/bash # 搜索所有包含 zIndex 的文件 rg "zIndex" -l # 搜索所有包含 z-index 的文件 rg "z-index" -l # 查找并显示 Overlay 相关文件内容 find . -type f -name "*verlay*.ts*" -o -name "*verlay*.js*" | xargs catLength of output: 12535
src/packages/popup/popup.tsx (3)
12-17
: 类型定义分离和导入优化将类型定义移至单独的文件是一个很好的重构,这样可以提高代码的可维护性和复用性。同时,为
defaultProps
添加显式类型注解也提高了类型安全性。
119-128
: 事件处理函数的类型安全性问题当前的实现使用
React.MouseEvent
类型作为事件参数,这是一个改进。但建议进一步完善类型定义,指定具体的事件目标类型。- const handleOverlayClick = (e: React.MouseEvent) => { + const handleOverlayClick = (e: React.MouseEvent<HTMLDivElement>) => { e.stopPropagation() if (closeOnOverlayClick && onOverlayClick(e)) { close() } } - const handleCloseIconClick = (e: React.MouseEvent) => { + const handleCloseIconClick = (e: React.MouseEvent<HTMLDivElement>) => { onCloseIconClick(e) && close() }
Line range hint
209-217
: 优化 Overlay 组件的 zIndex 处理直接传递
zIndex
属性到 Overlay 组件是一个很好的改进,这解决了原有的 zIndex 无效问题。但建议添加类型验证以确保index
的值始终为正数。建议添加以下验证:
if (process.env.NODE_ENV !== 'production') { if (typeof index !== 'number' || index < 0) { console.warn('[NutUI Popup] zIndex should be a positive number'); } }
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)
src/packages/dialog/types.ts (2)
30-30
: 改进联合类型定义在联合类型中使用 void 可能会导致混淆。建议使用 undefined 来提高类型安全性。
-onOverlayClick: (e: MouseEvent<HTMLElement>) => boolean | void +onOverlayClick: (e: MouseEvent<HTMLElement>) => boolean | undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 30-30: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
45-45
: 优化 Promise 相关类型定义在联合类型中使用 void 可能会导致混淆。建议使用更明确的返回类型。
-onConfirm?: (e?: MouseEvent<HTMLButtonElement>) => PromiseLike<any> | void +onConfirm?: (e?: MouseEvent<HTMLButtonElement>) => PromiseLike<any> | undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 45-45: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/dialog.taro.tsx (1)
187-189
: 优化事件处理逻辑可以使用可选链操作符来简化代码并提高可读性。
-const closed = onOverlayClick && onOverlayClick(e) -closed && onClose && onClose() -closed && onCancel && onCancel() +const closed = onOverlayClick?.(e) +closed && onClose?.() +closed && onCancel?.()🧰 Tools
🪛 Biome (1.9.4)
[error] 187-187: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/packages/dialog/content.taro.tsx
(3 hunks)src/packages/dialog/content.tsx
(4 hunks)src/packages/dialog/dialog.taro.tsx
(6 hunks)src/packages/dialog/dialogwrap.tsx
(4 hunks)src/packages/dialog/types.ts
(1 hunks)src/packages/fixednav/demos/taro/demo1.tsx
(2 hunks)src/packages/overlay/overlay.taro.tsx
(4 hunks)src/packages/overlay/types.ts
(1 hunks)src/packages/toast/demos/taro/demo4.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/packages/overlay/types.ts
- src/packages/dialog/content.taro.tsx
- src/packages/toast/demos/taro/demo4.tsx
- src/packages/overlay/overlay.taro.tsx
- src/packages/fixednav/demos/taro/demo1.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/dialog/types.ts
[error] 30-30: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 45-45: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/dialog.taro.tsx
[error] 187-187: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (3)
src/packages/dialog/content.tsx (1)
43-43
: 🛠️ Refactor suggestion需要改进类型转换的安全性
使用对象字面量索引时的类型转换可能导致运行时错误。建议使用更明确的类型定义。
-[footerDirection]: footerDirection, +[footerDirection as DialogFooterDirection]: footerDirection,Likely invalid or redundant comment.
src/packages/dialog/dialogwrap.tsx (1)
48-50
: 优化 Overlay 组件的 zIndex 处理zIndex 属性现在正确地传递给 Overlay 组件,这解决了原始问题。实现看起来很好。
src/packages/dialog/dialog.taro.tsx (1)
40-40
: 验证 zIndex 值的合理性默认的 zIndex 值设置为 1200,请确认这个值在整个应用的层级体系中是合适的。
✅ Verification successful
zIndex 值设置合理
当前 Dialog 组件的 zIndex 值 1200 在整个应用的层级体系中是合理的:
- 高于基础遮罩层(1000)和弹出层(1100)
- 低于 Toast 提示(1300)
这样的层级设置确保了组件间的正确显示顺序。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 搜索其他组件中的 zIndex 值以确保没有冲突 rg -n "zIndex.*=.*\d+" --type tsxLength of output: 63
Script:
#!/bin/bash # Search for both zIndex and z-index in all relevant files echo "=== Searching for zIndex in TypeScript/JavaScript files ===" rg -n "zIndex.*[:=].*\d+" echo -e "\n=== Searching for z-index in CSS/SCSS/Less/Style files ===" rg -n "z-index.*[:=].*\d+" echo -e "\n=== Searching for zIndex in style objects within TS/JS files ===" rg -n "style.*\{[^}]*zIndex.*[:=].*\d+[^}]*\}"Length of output: 29427
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: 1
🧹 Nitpick comments (4)
src/packages/popup/types.ts (2)
4-4
: 建议为 Teleport 类型添加文档注释为了提高代码的可维护性,建议添加 JSDoc 注释来说明 Teleport 类型的用途和使用场景。
+/** + * 定义弹出层渲染容器的类型 + * @type {HTMLElement} - 直接指定 DOM 元素作为容器 + * @type {() => HTMLElement} - 返回 DOM 元素的函数 + * @type {null} - 默认渲染到 body + */ export type Teleport = HTMLElement | (() => HTMLElement) | null
11-34
: 建议将非必需属性标记为可选以下属性在实际使用中可能不是必需的,建议标记为可选属性:
export interface PopupProps extends Omit<OverlayProps, 'onClick'> { position: Position - transition: string + transition?: string - overlayStyle: React.CSSProperties + overlayStyle?: React.CSSProperties - overlayClassName: string + overlayClassName?: string - closeable: boolean + closeable?: boolean - closeIconPosition: CloseIconPosition + closeIconPosition?: CloseIconPosition - closeIcon: ReactNode + closeIcon?: ReactNode // ... other properties }src/packages/overlay/overlay.taro.tsx (1)
49-51
: 建议添加 zIndex 管理的说明注释为了便于维护和理解组件的层级管理,建议添加注释说明 zIndex 的计算逻辑和使用规则。
const styles = { ...style, + // zIndex 优先级: + // 1. 用户传入的 zIndex 属性 + // 2. 默认值 1000 zIndex, }src/packages/popup/popup.tsx (1)
119-131
: 建议优化事件处理函数的类型安全性当前实现中的事件处理函数类型已经改进,但仍可以进一步优化:
- 使用泛型约束事件类型
- 统一事件参数命名
- const handleOverlayClick = ( - e: React.MouseEvent<HTMLDivElement, MouseEvent> - ) => { + const handleOverlayClick = <T extends React.MouseEvent<HTMLDivElement>>( + event: T + ) => { - e.stopPropagation() + event.stopPropagation() if (closeOnOverlayClick && onOverlayClick(e)) { close() } } - const handleCloseIconClick = ( - e: React.MouseEvent<HTMLDivElement, MouseEvent> - ) => { + const handleCloseIconClick = <T extends React.MouseEvent<HTMLDivElement>>( + event: T + ) => { - onCloseIconClick(e) && close() + onCloseIconClick(event) && close() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/packages/overlay/overlay.taro.tsx
(3 hunks)src/packages/popup/popup.taro.tsx
(7 hunks)src/packages/popup/popup.tsx
(5 hunks)src/packages/popup/types.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/popup/popup.taro.tsx
[error] 209-213: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/overlay/overlay.taro.tsx
[error] 55-55: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/popup/popup.taro.tsx
Outdated
onClick={(e) => | ||
handleCloseIconClick( | ||
e as unknown as React.MouseEvent<HTMLDivElement, MouseEvent> | ||
) | ||
} |
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.
🛠️ Refactor suggestion
建议减少类型断言的使用
当前实现中存在多处 as unknown as
类型断言,这可能导致类型安全问题。建议:
- 创建统一的事件类型定义
- 使用类型守卫替代类型断言
+// 在 types.ts 中添加
+export type TaroEvent = {
+ stopPropagation: () => void
+ target: unknown
+ currentTarget: unknown
+}
+
+// 创建类型守卫
+function isTaroEvent(event: unknown): event is TaroEvent {
+ return event && typeof (event as TaroEvent).stopPropagation === 'function'
+}
-onClick={(e) =>
- onClick &&
- onClick(
- e as unknown as React.MouseEvent<HTMLDivElement, MouseEvent>
- )
+onClick={(e) => {
+ if (isTaroEvent(e) && onClick) {
+ onClick(e)
+ }
+}}
Also applies to: 208-213
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 (1)
src/packages/popup/index.taro.ts (1)
10-13
: 建议改进事件处理器的返回类型在联合类型中使用
void
可能会造成混淆,建议使用undefined
替代。- onOverlayClick: (e: ITouchEvent) => boolean | void - onCloseIconClick: (e: ITouchEvent) => boolean | void + onOverlayClick: (e: ITouchEvent) => boolean | undefined + onCloseIconClick: (e: ITouchEvent) => boolean | undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 11-11: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 12-12: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/packages/overlay/index.taro.ts
(1 hunks)src/packages/popup/index.taro.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/overlay/index.taro.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/popup/index.taro.ts
[error] 11-11: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 12-12: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (2)
src/packages/popup/index.taro.ts (2)
1-5
: 导入和导出声明看起来不错!类型的组织结构清晰,命名规范合理,很好地区分了Web和Taro特定的类型定义。
6-9
: 类型结构设计合理!使用 Omit 来移除 Web 特定的事件处理器,然后添加 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.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/packages/dialog/index.taro.ts (1)
22-24
: 建议优化联合类型定义在
DialogBasicProps
类型中使用void
作为联合类型可能会导致混淆。建议使用undefined
替代。- onOverlayClick: (event: ITouchEvent) => boolean | void + onOverlayClick: (event: ITouchEvent) => boolean | undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/content.taro.tsx (1)
51-53
: 建议优化事件处理逻辑使用可选链操作符可以使代码更简洁安全。
- const handleClick = (e: ITouchEvent) => { - onClick && onClick(e) - } + const handleClick = (e: ITouchEvent) => { + onClick?.(e) + }🧰 Tools
🪛 Biome (1.9.4)
[error] 52-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/dialog/dialog.taro.tsx (1)
184-189
: 建议优化事件处理逻辑当前的事件处理逻辑可以通过使用可选链和更简洁的条件处理来改进。
const onHandleClickOverlay = (e: ITouchEvent) => { if (closeOnOverlayClick && visible && e.target === e.currentTarget) { - const closed = onOverlayClick && onOverlayClick(e) - closed && onClose && onClose() - closed && onCancel && onCancel() + const closed = onOverlayClick?.(e) + if (closed) { + onClose?.() + onCancel?.() + } } }🧰 Tools
🪛 Biome (1.9.4)
[error] 186-186: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/toast/toast.taro.tsx (2)
127-141
: 建议优化图标渲染逻辑当前的图标渲染逻辑已经比之前清晰,但还可以进一步优化:
- 建议将图标映射对象提取为常量
- 考虑添加类型检查以提高代码健壮性
建议重构为:
+const ICON_MAP = { + success: <Success />, + fail: <Failure />, + warn: <Tips />, + loading: <Loading className="nut-icon-loading" />, +} as const; const renderIcon = () => { let iconNode = icon if (typeof icon === 'string') { - iconNode = { - success: <Success />, - fail: <Failure />, - warn: <Tips />, - loading: <Loading className="nut-icon-loading" />, - }[icon] + iconNode = ICON_MAP[icon as keyof typeof ICON_MAP] || null } return iconNode && <View className={`${classPrefix}-icon-wrapper`}>{iconNode}</View> }
Line range hint
67-126
: 优化组件结构和生命周期管理当前的组件结构基本合理,但建议以下改进:
- 考虑将定时器逻辑抽取为自定义 Hook
- 优化事件处理函数的组织方式
- 添加必要的清理逻辑
建议创建自定义 Hook:
function useAutoClose(visible: boolean, duration: number, onClose: () => void) { const timer = useRef<number>(-1); useEffect(() => { if (visible && duration > 0) { timer.current = window.setTimeout(onClose, duration * 1000); } return () => { if (timer.current > -1) { clearTimeout(timer.current); timer.current = -1; } }; }, [visible, duration, onClose]); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/packages/dialog/content.taro.tsx
(3 hunks)src/packages/dialog/dialog.taro.tsx
(8 hunks)src/packages/dialog/index.taro.ts
(1 hunks)src/packages/dialog/index.ts
(1 hunks)src/packages/dialog/types.ts
(3 hunks)src/packages/toast/toast.taro.tsx
(6 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/dialog/content.taro.tsx
[error] 52-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/dialog/types.ts
[error] 31-31: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/index.taro.ts
[error] 23-23: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/dialog.taro.tsx
[error] 186-186: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 GitHub Check: build
src/packages/dialog/dialog.taro.tsx
[failure] 21-21:
Type '{ title: string; content: string; header: string; footer: string; confirmText: string; cancelText: string; overlay: true; overlayStyle: {}; overlayClassName: string; hideConfirmButton: false; hideCancelButton: false; ... 21 more ...; afterClose: () => void; }' is not assignable to type 'DialogBasicProps'.
[failure] 139-139:
Type '(e: ITouchEvent) => void' is not assignable to type '(e: MouseEvent<HTMLButtonElement, MouseEvent>) => void'.
[failure] 153-153:
Type '(e: ITouchEvent) => Promise' is not assignable to type '(e: MouseEvent<HTMLButtonElement, MouseEvent>) => void'.
src/packages/toast/toast.taro.tsx
[failure] 20-20:
Type '{ id: string; duration: number; position: "center"; title: string; size: "base"; icon: null; content: string; contentClassName: string; contentStyle: {}; closeOnOverlayClick: false; lockScroll: false; ... 9 more ...; onClick: (event: ITouchEvent) => void; }' is not assignable to type 'ToastProps'.
🪛 GitHub Actions: CI
src/packages/dialog/dialog.taro.tsx
[error] 21-21: Type mismatch error: Object type is not assignable to type 'DialogBasicProps'. Check the property types in the dialog component.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (7)
src/packages/dialog/index.ts (1)
6-8
: 类型导出重组看起来不错!新增的类型导出使得类型定义更加集中和清晰。
src/packages/dialog/index.taro.ts (1)
18-20
: 类型定义转换合理!将 Web 事件类型适配为 Taro 事件类型的做法很好。
src/packages/dialog/content.taro.tsx (1)
6-13
: 默认属性定义清晰!默认属性的集中定义有助于代码维护和复用。
src/packages/dialog/types.ts (2)
31-31
: 建议修改联合类型定义与之前相同的建议,使用
undefined
替代void
。- onOverlayClick: (e: React.MouseEvent<Element, MouseEvent>) => boolean | void + onOverlayClick: (e: React.MouseEvent<Element, MouseEvent>) => boolean | undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 31-31: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
12-21
: 类型层次结构设计合理!类型定义的层次结构清晰,有助于代码的维护和扩展。
src/packages/dialog/dialog.taro.tsx (1)
199-209
: zIndex 实现正确!Overlay 组件的 zIndex 属性实现正确,这解决了原始问题。
src/packages/toast/toast.taro.tsx (1)
Line range hint
155-189
: 验证 zIndex 在各场景下的有效性Overlay 组件的 zIndex 集成看起来没有问题,但建议:
- 验证多个 Toast 同时显示时的层级关系
- 确保与其他弹出组件(如 Dialog、Popup)的 zIndex 协调
✅ Verification successful
Toast 组件的 zIndex 层级设置合理
经过验证,Toast 组件的 zIndex 值为 1300,这个设置是合理的:
- 高于 Dialog (1200) 和 Popup (1100),确保 Toast 消息始终显示在最上层
- 与其他覆盖型组件形成了清晰的层级关系
- 符合用户体验最佳实践,确保提示信息不会被其他弹层遮挡
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查其他组件的 zIndex 默认值 rg -l "zIndex.*1300" src/packages/Length of output: 99
Script:
#!/bin/bash # Search for zIndex in overlay-related components and configuration files echo "=== Searching for zIndex in components ===" rg "zIndex.*\d+" src/packages/ -A 2 echo -e "\n=== Looking for zIndex related constants ===" rg -i "const.*zindex|zIndex" src/packages/Length of output: 35477
const defaultProps: DialogBasicProps = { | ||
...defaultOverlayProps, | ||
...defaultContentProps, | ||
title: '', | ||
content: '', | ||
header: '', | ||
footer: '', | ||
confirmText: '', | ||
cancelText: '', | ||
overlay: true, | ||
closeOnOverlayClick: true, | ||
overlayStyle: {}, | ||
overlayClassName: 'nut-dialog-overlay', | ||
hideConfirmButton: false, | ||
hideCancelButton: false, | ||
disableConfirmButton: false, | ||
footerDirection: 'horizontal', | ||
lockScroll: true, | ||
closeIconPosition: 'bottom', | ||
closeIcon: false, | ||
zIndex: 1200, | ||
beforeCancel: () => true, | ||
beforeClose: () => true, | ||
onCancel: () => {}, | ||
onClose: () => {}, | ||
onOverlayClick: () => true, | ||
} as DialogProps | ||
} |
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.
需要修复类型不匹配问题
defaultProps
的类型定义与 DialogBasicProps
不匹配,这会导致编译错误。
建议检查并确保所有必需的属性都已定义,特别是 afterClose
属性。
🧰 Tools
🪛 GitHub Check: build
[failure] 21-21:
Type '{ title: string; content: string; header: string; footer: string; confirmText: string; cancelText: string; overlay: true; overlayStyle: {}; overlayClassName: string; hideConfirmButton: false; hideCancelButton: false; ... 21 more ...; afterClose: () => void; }' is not assignable to type 'DialogBasicProps'.
🪛 GitHub Actions: CI
[error] 21-21: Type mismatch error: Object type is not assignable to type 'DialogBasicProps'. Check the property types in the dialog component.
const defaultProps: ToastProps = { | ||
...defaultOverlayProps, | ||
id: '', | ||
duration: 2, // 时长,duration为0则一直展示 | ||
position: 'center', | ||
title: '', | ||
size: 'base', // 设置字体大小,默认base,可选large\small\base | ||
icon: null, | ||
content: '', | ||
type: 'text', | ||
contentClassName: '', | ||
contentStyle: {}, | ||
closeOnOverlayClick: false, | ||
lockScroll: false, | ||
contentClassName: '', // 内容自定义样式名 | ||
visible: false, | ||
wordBreak: 'break-all', | ||
onClose: () => {}, // 未实现 | ||
} as unknown as ToastProps | ||
|
||
const classPrefix = 'nut-toast' | ||
zIndex: 1300, | ||
onClose: () => {}, | ||
} |
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.
类型定义需要修复
defaultProps 的类型定义与 ToastProps 接口不匹配。建议检查以下几点:
- 确保所有必需的属性都已定义
- 验证属性类型是否正确
- 考虑使用 Partial 类型
建议按如下方式修复:
-const defaultProps: ToastProps = {
+const defaultProps: Partial<ToastProps> = {
...defaultOverlayProps,
// ... other props
}
📝 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 defaultProps: ToastProps = { | |
...defaultOverlayProps, | |
id: '', | |
duration: 2, // 时长,duration为0则一直展示 | |
position: 'center', | |
title: '', | |
size: 'base', // 设置字体大小,默认base,可选large\small\base | |
icon: null, | |
content: '', | |
type: 'text', | |
contentClassName: '', | |
contentStyle: {}, | |
closeOnOverlayClick: false, | |
lockScroll: false, | |
contentClassName: '', // 内容自定义样式名 | |
visible: false, | |
wordBreak: 'break-all', | |
onClose: () => {}, // 未实现 | |
} as unknown as ToastProps | |
const classPrefix = 'nut-toast' | |
zIndex: 1300, | |
onClose: () => {}, | |
} | |
const defaultProps: Partial<ToastProps> = { | |
...defaultOverlayProps, | |
id: '', | |
duration: 2, // 时长,duration为0则一直展示 | |
position: 'center', | |
title: '', | |
size: 'base', // 设置字体大小,默认base,可选large\small\base | |
icon: null, | |
content: '', | |
contentClassName: '', | |
contentStyle: {}, | |
closeOnOverlayClick: false, | |
lockScroll: false, | |
visible: false, | |
wordBreak: 'break-all', | |
zIndex: 1300, | |
onClose: () => {}, | |
} |
🧰 Tools
🪛 GitHub Check: build
[failure] 20-20:
Type '{ id: string; duration: number; position: "center"; title: string; size: "base"; icon: null; content: string; contentClassName: string; contentStyle: {}; closeOnOverlayClick: false; lockScroll: false; ... 9 more ...; onClick: (event: ITouchEvent) => void; }' is not assignable to type 'ToastProps'.
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 (1)
src/packages/dialog/index.taro.ts (1)
22-28
: 建议优化联合类型中的 void 使用为了提高类型定义的清晰度,建议将联合类型中的
void
替换为undefined
。建议按如下方式修改:
- onOverlayClick: (event: ITouchEvent) => boolean | void + onOverlayClick: (event: ITouchEvent) => boolean | undefined - onConfirm?: (e?: ITouchEvent) => PromiseLike<any> | void + onConfirm?: (e?: ITouchEvent) => PromiseLike<any> | undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 26-26: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 27-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/dialog/index.taro.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/dialog/index.taro.ts
[error] 26-26: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 27-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (3)
src/packages/dialog/index.taro.ts (3)
1-7
: 导入声明看起来很清晰!类型别名的使用有助于避免命名冲突,导入结构合理。
9-16
: 类型导出结构合理!将相关的对话框类型集中导出,提高了代码的可维护性。
18-20
: ContentProps 类型定义准确!正确地将 Web 版本的点击事件类型适配为 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.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/packages/dialog/content.tsx (1)
42-44
: 🛠️ Refactor suggestion需要改进类型安全性
使用对象字面量动态属性时需要确保类型安全。建议明确定义
footerDirection
的可能值。- [footerDirection]: footerDirection, + [footerDirection as 'horizontal' | 'vertical']: footerDirection,
🧹 Nitpick comments (2)
src/packages/dialog/index.taro.ts (1)
22-29
: 建议优化联合类型中的 void 使用为提高类型定义的清晰度,建议将联合类型中的
void
替换为undefined
。建议应用以下修改:
- onOverlayClick: (event: ITouchEvent) => boolean | void + onOverlayClick: (event: ITouchEvent) => boolean | undefined - onConfirm?: (e?: ITouchEvent) => PromiseLike<any> | void + onConfirm?: (e?: ITouchEvent) => PromiseLike<any> | undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 27-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 28-28: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/dialogwrap.tsx (1)
Line range hint
1-70
: 建议:增强类型安全性为提高代码的可维护性和类型安全性,建议:
- 考虑使用
const assertions
来定义默认属性:export const defaultDialogWrapProps = { // ... } as const;
- 为组件添加泛型支持,使其更灵活:
export const DialogWrap = <T extends DialogWrapProps>() => { // ... }
- 考虑将 Overlay 和 Content 组件的属性类型分别定义,以便更好地管理依赖关系。
🧰 Tools
🪛 GitHub Check: build
[failure] 9-9:
Type '{ overlay: true; overlayStyle: {}; overlayClassName: string; onCancel: () => void; onClose: () => void; onOverlayClick: () => true; visible: boolean; title: React.ReactNode; header: React.ReactNode; ... 13 more ...; afterClose: () => void; }' is not assignable to type 'DialogWrapProps'.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/packages/dialog/content.tsx
(3 hunks)src/packages/dialog/dialogwrap.tsx
(4 hunks)src/packages/dialog/index.taro.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/dialog/index.taro.ts
[error] 27-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 28-28: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/content.tsx
[error] 53-53: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 GitHub Check: build
src/packages/dialog/dialogwrap.tsx
[failure] 9-9:
Type '{ overlay: true; overlayStyle: {}; overlayClassName: string; onCancel: () => void; onClose: () => void; onOverlayClick: () => true; visible: boolean; title: React.ReactNode; header: React.ReactNode; ... 13 more ...; afterClose: () => void; }' is not assignable to type 'DialogWrapProps'.
[failure] 55-55:
Type '(e: React.MouseEvent<Element, MouseEvent>) => void' is not assignable to type '((event: MouseEvent<Element, MouseEvent>) => void) & MouseEventHandler'.
[failure] 66-66:
Type '{ visible: boolean; duration: number; onClick: (event: MouseEvent<Element, MouseEvent>) => void; afterShow: () => void; afterClose: () => void; className?: string | undefined; ... 264 more ...; onTransitionEndCapture?: TransitionEventHandler<...> | undefined; }' is not assignable to type 'Partial'.
src/packages/dialog/content.tsx
[failure] 53-53:
Argument of type 'MouseEvent<Element, MouseEvent>' is not assignable to parameter of type 'MouseEvent<Element, MouseEvent<Element, MouseEvent>>'.
🪛 GitHub Actions: CI
src/packages/dialog/content.tsx
[error] 53-53: Type mismatch error: Argument of type 'MouseEvent<Element, MouseEvent>' is not assignable to parameter of type 'MouseEvent<Element, MouseEvent<Element, MouseEvent>>'
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (8)
src/packages/dialog/index.taro.ts (3)
1-7
: 导入声明结构清晰!类型别名的使用提高了代码的可维护性,同时与类型提取的目标保持一致。
9-16
: 类型导出组织合理!将相关的对话框类型集中导出,使得类型定义更加集中和易于维护。
18-20
: ContentProps 类型定义准确!通过 Omit 和交叉类型的组合,正确地为 Taro 环境重新定义了 onClick 事件类型。
src/packages/dialog/dialogwrap.tsx (2)
3-7
: 类型导入重构得当!将类型定义移至单独的类型文件是一个很好的实践,有助于提高代码的可维护性和重用性。
31-36
: zIndex 属性添加正确!正确地从 props 中解构出
zIndex
,这解决了原有的 overlay zIndex 无效的问题。src/packages/dialog/content.tsx (3)
5-12
: 代码结构清晰,类型定义完整!将默认属性集中管理是一个很好的实践,提高了代码的可维护性。
Line range hint
16-30
: 类型定义优化得当!使用
Omit
排除重复的 HTML 属性是个很好的做法,避免了类型冲突。同时使用展开运算符合并默认值的方式也很规范。🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 GitHub Check: build
[failure] 53-53:
Argument of type 'MouseEvent<Element, MouseEvent>' is not assignable to parameter of type 'MouseEvent<Element, MouseEvent<Element, MouseEvent>>'.🪛 GitHub Actions: CI
[error] 53-53: Type mismatch error: Argument of type 'MouseEvent<Element, MouseEvent>' is not assignable to parameter of type 'MouseEvent<Element, MouseEvent<Element, MouseEvent>>'
Line range hint
58-72
: 组件结构合理,渲染逻辑清晰!使用
classNames
处理类名组合,通过visible
控制显示状态,代码结构清晰易懂。
export const defaultDialogWrapProps: DialogWrapProps = { | ||
...defaultOverlayProps, | ||
...defaultContentProps, | ||
overlay: true, | ||
overlayStyle: {}, | ||
overlayClassName: '', | ||
overlayClassName: 'nut-dialog-overlay', | ||
onCancel: () => {}, | ||
onClose: () => {}, | ||
onOverlayClick: (e: MouseEvent) => true, | ||
...defaultOverlayProps, | ||
} as DialogWrapProps | ||
onOverlayClick: () => true, | ||
} |
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.
需要修复类型错误
defaultDialogWrapProps
的类型定义与 DialogWrapProps
接口不匹配。这可能会导致运行时出现意外行为。
建议检查 DialogWrapProps
类型定义,确保包含所有必需的属性:
- 验证
types.ts
中的DialogWrapProps
定义是否完整 - 确保
defaultOverlayProps
和defaultContentProps
的所有属性都在DialogWrapProps
中声明
🧰 Tools
🪛 GitHub Check: build
[failure] 9-9:
Type '{ overlay: true; overlayStyle: {}; overlayClassName: string; onCancel: () => void; onClose: () => void; onOverlayClick: () => true; visible: boolean; title: React.ReactNode; header: React.ReactNode; ... 13 more ...; afterClose: () => void; }' is not assignable to type 'DialogWrapProps'.
{overlay && ( | ||
<Overlay | ||
zIndex={zIndex} | ||
style={overlayStyle} | ||
className={classNames('nut-dialog-overlay', overlayClassName)} | ||
className={overlayClassName} | ||
visible={visible} | ||
closeOnOverlayClick={closeOnOverlayClick} | ||
lockScroll={lockScroll} | ||
onClick={onHandleClickOverlay} | ||
/> | ||
) : null} | ||
)} |
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.
需要修复事件处理器类型
Overlay 组件的 onClick
处理器类型不匹配。
建议修改事件处理器的类型定义:
onClick: (e: MouseEvent<HTMLDivElement>) => void
另外,zIndex
传递给 Overlay 组件是个很好的改进。
🧰 Tools
🪛 GitHub Check: build
[failure] 55-55:
Type '(e: React.MouseEvent<Element, MouseEvent>) => void' is not assignable to type '((event: MouseEvent<Element, MouseEvent>) => void) & MouseEventHandler'.
@@ -73,7 +63,7 @@ | |||
unmountOnExit | |||
appear | |||
> | |||
<Content {...props} visible={visible} /> | |||
<Content {...restProps} visible={visible} /> |
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.
需要修复 Content 组件的属性类型
传递给 Content 组件的属性类型与 ContentProps
不匹配。
建议:
- 检查
ContentProps
类型定义是否完整 - 确保
restProps
中的属性类型与ContentProps
兼容 - 考虑使用
Pick
或Omit
来精确控制传递的属性
🧰 Tools
🪛 GitHub Check: build
[failure] 66-66:
Type '{ visible: boolean; duration: number; onClick: (event: MouseEvent<Element, MouseEvent>) => void; afterShow: () => void; afterClose: () => void; className?: string | undefined; ... 264 more ...; onTransitionEndCapture?: TransitionEventHandler<...> | undefined; }' is not assignable to type 'Partial'.
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
♻️ Duplicate comments (1)
src/packages/toast/types.ts (1)
13-24
: 🛠️ Refactor suggestion建议优化 Props 接口的类型定义
接口定义可以进行以下改进:
- 部分属性应该是可选的
- 建议添加属性的文档注释
建议修改如下:
export interface ToastProps extends Omit<OverlayProps, 'onClick'> { id: string - position: ToastPosition + position?: ToastPosition title: string size: ToastSize - icon: ToastIcon + icon?: ToastIcon content: React.ReactNode - contentClassName: string + contentClassName?: string - contentStyle: React.CSSProperties + contentStyle?: React.CSSProperties - wordBreak: ToastWordBreak + wordBreak?: ToastWordBreak onClose: () => void }
🧹 Nitpick comments (5)
src/packages/toast/types.ts (1)
4-11
: 建议添加类型文档注释为了提高代码的可维护性和可读性,建议为每个类型添加 JSDoc 注释,说明其用途和具体值的含义。特别是
ToastSize
类型,应该说明每个尺寸的具体大小。建议添加如下注释:
+/** Toast 图标类型,可以是预设图标或自定义 React 节点 */ export type ToastIcon = | 'success' | 'fail' | 'loading' | 'warn' | React.ReactNode +/** Toast 文本换行方式 */ export type ToastWordBreak = 'normal' | 'break-all' | 'break-word' +/** + * Toast 尺寸 + * - small: 小号提示 + * - base: 标准提示 + * - large: 大号提示 + */ export type ToastSize = 'small' | 'base' | 'large'src/packages/dialog/index.taro.ts (1)
27-27
: 建议改进返回类型定义在联合类型中使用
void
可能会导致混淆,建议使用undefined
。- onOverlayClick: (event: ITouchEvent) => boolean | void + onOverlayClick: (event: ITouchEvent) => boolean | undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 27-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/types.ts (2)
31-31
: 建议改进 onOverlayClick 的返回类型与 Taro 版本保持一致,建议在联合类型中使用
undefined
替代void
。- onOverlayClick: (e: MouseEvent) => boolean | void + onOverlayClick: (e: MouseEvent) => boolean | undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 31-31: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
25-26
: 避免重复的类型定义
DialogWrapProps
中的visible
属性在ContentProps
中已经定义,这里重复定义是多余的。export interface DialogWrapProps extends OverlayProps, Omit<ContentProps, 'onClick'> { - visible: boolean overlay: boolean overlayStyle: CSSProperties
Also applies to: 27-28
src/packages/dialog/dialog.taro.tsx (1)
188-192
: 建议优化事件处理逻辑使用可选链操作符可以使代码更简洁安全。
- const closed = onOverlayClick && onOverlayClick(e) - closed && onClose && onClose() - closed && onCancel && onCancel() + const closed = onOverlayClick?.(e) + if (closed) { + onClose?.() + onCancel?.() + }🧰 Tools
🪛 Biome (1.9.4)
[error] 190-190: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/packages/dialog/dialog.taro.tsx
(8 hunks)src/packages/dialog/dialogwrap.tsx
(4 hunks)src/packages/dialog/index.taro.ts
(1 hunks)src/packages/dialog/types.ts
(3 hunks)src/packages/toast/types.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/dialog/dialogwrap.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/dialog/dialog.taro.tsx
[error] 190-190: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/dialog/types.ts
[error] 31-31: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/index.taro.ts
[error] 27-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (5)
src/packages/toast/types.ts (1)
1-3
: 导入语句和位置类型定义看起来不错!导入语句清晰,ToastPosition 类型定义简洁明了。
src/packages/dialog/index.taro.ts (1)
18-20
: 类型定义的改进很好将 Web 和 Taro 平台的类型定义分离,并且正确处理了事件类型,这样的改进提高了代码的类型安全性和可维护性。
Also applies to: 22-28
src/packages/dialog/types.ts (1)
12-21
: 类型结构组织合理将
DialogFooterDirection
和ContentProps
的定义提取到类型文件中,提高了代码的可重用性和可维护性。src/packages/dialog/dialog.taro.tsx (2)
31-32
: 修复了 overlay zIndex 问题通过在默认属性中正确设置
overlayStyle
、overlayClassName
和zIndex
,解决了 overlay zIndex 无效的问题。Also applies to: 39-39
203-213
: Overlay 组件配置完善正确传递了
zIndex
、overlayStyle
和overlayClassName
属性到 Overlay 组件,使配置更加灵活。
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: 1
♻️ Duplicate comments (2)
src/packages/popup/types.ts (2)
12-26
: 🛠️ Refactor suggestion建议优化属性可选性
一些属性在实际使用中应该是可选的,建议进行以下修改:
export interface PopupProps extends OverlayProps { position: Position - transition: string + transition?: string - overlayStyle: React.CSSProperties + overlayStyle?: React.CSSProperties - overlayClassName: string + overlayClassName?: string - closeable: boolean + closeable?: boolean - closeIconPosition: CloseIconPosition + closeIconPosition?: CloseIconPosition - closeIcon: ReactNode + closeIcon?: ReactNode // ... 其他属性 }
27-30
: 🛠️ Refactor suggestion优化事件处理器类型定义
建议进行以下改进:
- 使用
undefined
替代void
以提高类型清晰度- 为事件处理器添加完整的泛型类型
- onOverlayClick: (e: React.MouseEvent) => boolean | void - onCloseIconClick: (e: React.MouseEvent) => boolean | void + onOverlayClick: (e: React.MouseEvent<HTMLElement>) => boolean | undefined + onCloseIconClick: (e: React.MouseEvent<HTMLElement>) => boolean | undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 29-29: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 30-30: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🧹 Nitpick comments (8)
src/packages/fixednav/fixednav.tsx (3)
9-23
: 建议提高 zIndex 的默认值当前 zIndex 默认值为 200,这个值对于全局性的浮层组件来说可能偏低,可能会被其他浮层元素覆盖。建议参考常见的组件库设计,将默认值调整到 1000-2000 范围内。
- zIndex: 200, + zIndex: 1000,
73-74
: 优化 Overlay 的点击处理建议为 onClick 事件处理添加防抖,避免快速点击导致的问题。同时建议添加错误处理。
+ import { useDebounceFn } from '@/packages/hooks' // 在组件内部 + const debouncedOnChange = useDebounceFn( + (value: boolean) => { + try { + onChange(value) + } catch (error) { + console.error('Overlay onChange error:', error) + } + }, + { wait: 300 } + ) // 在 Overlay 组件中 - onClick={() => onChange(false)} + onClick={() => debouncedOnChange(false)}
101-101
: 优化状态切换处理建议为状态切换添加过渡动画,提升用户体验。同时建议添加错误处理。
+ import { CSSTransition } from 'react-transition-group' // 在组件内部 + const handleVisibleChange = (value: boolean) => { + try { + onChange(value) + } catch (error) { + console.error('Visible change error:', error) + } + } - <div className={`${classPrefix}-btn`} onClick={() => onChange(!visible)}> + <div className={`${classPrefix}-btn`} onClick={() => handleVisibleChange(!visible)}>src/packages/toast/toast.tsx (2)
7-24
: 建议为 zIndex 添加类型验证建议为 zIndex 属性添加类型验证以确保其值的合法性。同时建议将默认值 1300 提取为常量或配置项,便于统一管理和维护。
+ const DEFAULT_TOAST_Z_INDEX = 1300; let defaultProps: ToastProps = { ...defaultOverlayProps, id: '', position: 'center', title: '', size: 'base', icon: null, content: '', contentClassName: '', contentStyle: {}, wordBreak: 'break-all', duration: 2, closeOnOverlayClick: false, lockScroll: false, - zIndex: 1300, + zIndex: DEFAULT_TOAST_Z_INDEX, onClose: () => {}, }
Line range hint
42-52
: 优化错误处理和可选链使用建议优化以下几点:
- 使用可选链操作符来简化代码
- 添加错误处理机制
function notice(opts: any) { function close() { if (messageInstance) { messageInstance.destroy() messageInstance = null - opts.onClose && opts.onClose() + opts?.onClose?.() } } const opts2 = { ...defaultProps, ...opts, onClose: close } + try { getInstance(opts2, (notification: any) => { messageInstance = notification }) + } catch (error) { + console.error('[Toast] Failed to create instance:', error) + } }🧰 Tools
🪛 Biome (1.9.4)
[error] 48-48: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/toast/Notification.tsx (3)
6-16
: 建议完善类型定义建议扩展 NotificationState 接口,增加更多状态属性以提高类型安全性:
export interface NotificationState { show: boolean + loading?: boolean + error?: Error | null + animationState?: 'enter' | 'leave' | 'none' }
Line range hint
33-43
: 优化关闭逻辑的错误处理建议添加 try-catch 块来处理可能的 DOM 操作错误,并添加日志记录:
close() { this.setState({ show: false, }) this.clearCloseTimer() + try { if (this.props.id) { const element = document.getElementById(this.props.id) element?.parentNode?.removeChild(element) } this.props?.onClose?.() + } catch (error) { + console.error('[Toast] Error while closing:', error) + } }🧰 Tools
🪛 Biome (1.9.4)
[error] 40-40: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 42-42: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 GitHub Check: codecov/patch
[warning] 42-42: src/packages/toast/Notification.tsx#L42
Added line #L42 was not covered by tests
112-144
: 优化渲染性能建议进行以下优化:
- 使用 useMemo 或 React.memo 优化复杂的 className 计算
- 考虑使用 CSS-in-JS 或 CSS Modules 来减少类名冲突
+ const getContentClasses = React.useMemo(() => { + return classNames( + `${classPrefix}-inner`, + `${classPrefix}-${position}`, + contentClassName, + `${classPrefix}-inner-${size}`, + `${classPrefix}-inner-${wordBreak}`, + { + [`${classPrefix}-inner-descrption`]: content, + } + ) + }, [position, contentClassName, size, wordBreak, content]) return ( <Overlay visible={show} style={style} zIndex={zIndex} className={`${classPrefix}-overlay-default ${className}`} onClick={() => this.clickCover()} closeOnOverlayClick={closeOnOverlayClick} lockScroll={lockScroll} > <div className={`${classPrefix} ${classes}`} id={`toast-${id}`}> <div - className={classNames(...)} + className={getContentClasses} style={contentStyle} > {this.renderIcon()} {title && <div className={`${classPrefix}-title`}>{title}</div>} {content && <span className={`${classPrefix}-text`}>{content}</span>} </div> </div> </Overlay> )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/packages/configprovider/types.ts
(0 hunks)src/packages/fixednav/fixednav.taro.tsx
(6 hunks)src/packages/fixednav/fixednav.tsx
(5 hunks)src/packages/popover/__tests__/popover.spec.tsx
(2 hunks)src/packages/popup/types.ts
(1 hunks)src/packages/toast/Notification.tsx
(3 hunks)src/packages/toast/toast.tsx
(4 hunks)
💤 Files with no reviewable changes (1)
- src/packages/configprovider/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/fixednav/fixednav.taro.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/toast/Notification.tsx
[error] 42-42: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/toast/toast.tsx
[error] 48-48: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/popup/types.ts
[error] 29-29: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 30-30: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🪛 GitHub Check: codecov/patch
src/packages/toast/Notification.tsx
[warning] 42-42: src/packages/toast/Notification.tsx#L42
Added line #L42 was not covered by tests
src/packages/toast/toast.tsx
[warning] 71-72: src/packages/toast/toast.tsx#L71-L72
Added lines #L71 - L72 were not covered by tests
🔇 Additional comments (5)
src/packages/popup/types.ts (1)
1-11
: 类型定义清晰且完整!类型定义采用了字面量类型,限制了可能的值范围,有助于提供更好的类型安全性和开发体验。
src/packages/fixednav/fixednav.tsx (1)
Line range hint
25-49
: 代码结构清晰,类型定义合理组件的类型定义和属性解构遵循了 React 的最佳实践,使用 Partial 和 Omit 来优化类型定义。
src/packages/popover/__tests__/popover.spec.tsx (2)
61-61
: 类名更新符合预期!类名从
nut-popup-default
更新为nut-popup-none
的变更与 PR 中提到的 overlay 组件重构保持一致。
86-86
: 类名更新保持一致性!此处类名更新与之前的测试用例保持一致,确保了整个测试套件的一致性。
建议验证这个改动是否需要更新其他相关组件的测试用例:
✅ Verification successful
类名更新验证完成! ✅
经过验证,
nut-popup-none
类名的使用仅限于 popover 组件的测试文件中,且在该文件中保持了一致性。无需更新其他组件的测试用例。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 搜索其他测试文件中可能使用了 nut-popup-default 类名的地方 rg "nut-popup-default" "src/packages/**/__tests__/*.spec.tsx"Length of output: 138
Script:
#!/bin/bash # Search for both old and new class names in test files echo "Searching in test files:" rg "nut-popup-(default|none)" "src/**/*.spec.tsx" echo -e "\nSearching in component files:" rg "nut-popup-(default|none)" "src/**/*.tsx" --type-not specLength of output: 359
Script:
#!/bin/bash echo "Searching in test files:" fd "\.spec\.tsx$" src/ | xargs rg "nut-popup-(default|none)" echo -e "\nSearching in component files:" fd "\.tsx$" src/ --exclude "*.spec.tsx" | xargs rg "nut-popup-(default|none)"Length of output: 514
src/packages/toast/toast.tsx (1)
71-72
: 添加测试覆盖并考虑线程安全
- 该函数缺少测试覆盖,建议添加相关测试用例
- 由于修改全局状态,可能存在并发安全问题
建议添加以下测试用例:
describe('Toast.config', () => { it('should update default props', () => { const newConfig = { zIndex: 2000 }; Toast.config(newConfig); Toast.show('test'); // 验证 zIndex 是否正确更新 expect(document.querySelector('.nut-toast-overlay')?.style.zIndex).toBe('2000'); }); });🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 71-72: src/packages/toast/toast.tsx#L71-L72
Added lines #L71 - L72 were not covered by tests
@@ -104,7 +81,7 @@ export const FixedNav: FunctionComponent< | |||
return ( | |||
<div | |||
className={`${classPrefix}-list-item`} | |||
onClick={(event) => handleClick(item, event)} | |||
onClick={(event) => onSelect(item, event)} |
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.
🛠️ Refactor suggestion
增强 onSelect 的类型安全性
建议为 item 参数添加明确的类型定义,避免使用 any。同时建议添加错误处理。
- onClick={(event) => onSelect(item, event)}
+ onClick={(event: React.MouseEvent) => {
+ try {
+ onSelect(item as FixedNavItem, event)
+ } catch (error) {
+ console.error('Item select error:', error)
+ }
+ }}
📝 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.
onClick={(event) => onSelect(item, event)} | |
onClick={(event: React.MouseEvent) => { | |
try { | |
onSelect(item as FixedNavItem, event) | |
} catch (error) { | |
console.error('Item select error:', error) | |
} | |
}} |
src/packages/overlay/index.taro.ts
Outdated
import { OverlayProps as OverlayWebProps } from './types' | ||
|
||
export type OverlayProps = Omit<OverlayWebProps, 'onClick'> & { | ||
onClick: (event: ITouchEvent) => void |
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.
非常好,可以采用为开发标准,关于类型的差异化处理方案,期待更通用的类型处理逻辑。
src/packages/dialog/confirm.tsx
Outdated
...newConfig, | ||
title: config.title, // 避免 newConfig 未传递 title 时,icon 出现多个的问题 |
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.
newConfig 中存在 title 时,无法被覆盖,值一直是 config.title
src/packages/dialog/content.tsx
Outdated
} | ||
|
||
const handleClick = (e: any) => { | ||
const handleClick = (e: React.MouseEvent<Element, MouseEvent>) => { |
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.
看下 AI 给的建议
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: 1
♻️ Duplicate comments (1)
src/packages/overlay/overlay.tsx (1)
59-63
:⚠️ Potential issue修复事件处理器类型不匹配问题
handleClick 函数的事件类型与 onClick 回调的类型不匹配。onClick 期望接收 TouchEvent,但实际传入的是 MouseEvent。
建议修改为:
-const handleClick = (e: MouseEvent) => { +const handleClick = (e: TouchEvent) => { if (closeOnOverlayClick) { - onClick && onClick(e) + onClick?.(e) } }🧰 Tools
🪛 Biome (1.9.4)
[error] 61-61: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🧹 Nitpick comments (17)
src/packages/overlay/types.taro.ts (1)
1-6
: 建议添加类型定义的文档注释为了提高代码的可维护性和可读性,建议为
OverlayProps
类型添加 JSDoc 文档注释,说明其用途和与 Web 版本的区别。建议添加如下注释:
+/** + * Taro 环境下的 Overlay 组件属性 + * @extends {Omit<OverlayWebProps, 'onClick'>} 继承自 Web 版本的属性(不包含 onClick) + * @property {(event: ITouchEvent) => void} onClick - Taro 触摸事件处理函数 + */ export type OverlayProps = Omit<OverlayWebProps, 'onClick'> & { onClick: (event: ITouchEvent) => void }src/packages/overlay/overlay.tsx (2)
14-24
: 建议重构默认属性的组织方式建议将回调函数的默认值组织在一起,提高代码的可读性。
建议按照以下方式重构:
export const defaultOverlayProps: OverlayProps = { ...ComponentDefaults, zIndex: 1000, duration: 300, closeOnOverlayClick: true, visible: false, lockScroll: true, - onClick: () => {}, - afterShow: () => {}, - afterClose: () => {}, + // 回调函数 + onClick: () => {}, + afterShow: () => {}, + afterClose: () => {}, }
72-73
: 建议添加过渡动画完成的状态判断在调用 afterShow 和 afterClose 回调时,建议添加状态检查以避免可能的内存泄漏。
- onEntered={afterShow} - onExited={afterClose} + onEntered={() => innerVisible && afterShow?.()} + onExited={() => !innerVisible && afterClose?.()}src/packages/popup/types.taro.ts (1)
9-11
: 事件处理器返回类型需要统一建议统一使用
undefined
替代void
作为事件处理器的返回类型,这样可以避免在联合类型中使用void
带来的混淆。- onClick: (event: ITouchEvent) => void - onOverlayClick: (e: ITouchEvent) => boolean | void - onCloseIconClick: (e: ITouchEvent) => boolean | void + onClick: (event: ITouchEvent) => undefined + onOverlayClick: (e: ITouchEvent) => boolean | undefined + onCloseIconClick: (e: ITouchEvent) => boolean | undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 11-11: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/types.taro.ts (1)
25-26
: 保持与 Popup 组件一致的事件处理器返回类型为了保持代码库的一致性,建议采用与 Popup 组件相同的事件处理器返回类型定义。
- onClick: (event: ITouchEvent) => void - onOverlayClick: (event: ITouchEvent) => boolean | void + onClick: (event: ITouchEvent) => undefined + onOverlayClick: (event: ITouchEvent) => boolean | undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 26-26: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/content.tsx (1)
52-54
: 使用可选链操作符优化事件处理建议使用可选链操作符来简化事件处理逻辑。
const handleClick = (e: React.MouseEvent<HTMLDivElement>) => { - onClick && onClick(e) + onClick?.(e) }🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/dialog/content.taro.tsx (2)
6-13
: 优化默认属性的类型安全性建议为
defaultContentProps
中的事件处理器添加明确的类型定义。export const defaultContentProps: ContentProps = { visible: false, title: '', header: '', footer: '', close: '', footerDirection: 'horizontal', - onClick: () => {}, + onClick: (event: ITouchEvent) => {}, }
51-53
: 优化事件处理函数的实现建议使用可选链操作符来简化事件处理逻辑。
- const handleClick = (e: ITouchEvent) => { - onClick && onClick(e) - } + const handleClick = (e: ITouchEvent) => onClick?.(e)🧰 Tools
🪛 Biome (1.9.4)
[error] 52-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/dialog/types.ts (2)
31-31
: 优化联合类型定义在联合类型中使用
void
可能导致混淆,建议使用undefined
。- onOverlayClick: (e: MouseEvent) => boolean | void + onOverlayClick: (e: MouseEvent) => boolean | undefined - onConfirm?: (e?: MouseEvent<HTMLButtonElement>) => PromiseLike<any> | void + onConfirm?: (e?: MouseEvent<HTMLButtonElement>) => PromiseLike<any> | undefinedAlso applies to: 45-45
🧰 Tools
🪛 Biome (1.9.4)
[error] 31-31: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
45-45
: 改进 Promise 返回类型的定义使用
any
作为 Promise 的泛型类型不够严谨,建议定义更具体的类型。- onConfirm?: (e?: MouseEvent<HTMLButtonElement>) => PromiseLike<any> | undefined + onConfirm?: (e?: MouseEvent<HTMLButtonElement>) => PromiseLike<void> | undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 45-45: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/confirm.tsx (1)
89-95
: 优化 update 方法的类型安全性建议添加类型检查和错误处理。
- const update = (newConfig: DialogConfirmNativeProps) => { + const update = (newConfig: DialogConfirmNativeProps) => { + if (!newConfig || typeof newConfig !== 'object') { + console.error('Invalid config object provided to update method') + return + } dialogConfig = { ...dialogConfig, ...newConfig, } dialogConfig = normalizeConfig(dialogConfig) renderFunction(dialogConfig) }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 89-89: src/packages/dialog/confirm.tsx#L89
Added line #L89 was not covered by testssrc/packages/fixednav/fixednav.tsx (1)
21-22
: 改进默认事件处理器的类型定义建议为默认的事件处理器添加类型注解。
- onChange: (value: boolean) => {}, - onSelect: () => {}, + onChange: (value: boolean): void => {}, + onSelect: (item: FixedNavItem, event: React.MouseEvent): void => {},src/packages/fixednav/fixednav.taro.tsx (1)
10-24
: 建议统一组件的 zIndex 层级管理当前
zIndex
被硬编码为 200。建议:
- 将所有组件的 zIndex 统一在一个配置文件中管理
- 确保不同组件的 zIndex 遵循合理的层级关系
需要我帮您创建一个统一的 zIndex 管理方案吗?
src/packages/popup/popup.tsx (2)
119-124
: 优化事件处理逻辑建议使用可选链操作符来简化事件处理逻辑:
- const handleOverlayClick = (e: React.MouseEvent<Element, MouseEvent>) => { + const handleOverlayClick = (e: React.MouseEvent<Element, MouseEvent>) => { e.stopPropagation() - if (closeOnOverlayClick && onOverlayClick(e)) { + if (closeOnOverlayClick && onOverlayClick?.(e)) { close() } }
Line range hint
209-222
: 建议优化 Overlay 组件的属性传递当前的实现方式直接传递了多个独立的属性,建议使用对象解构来简化属性传递:
- <Overlay - zIndex={index} - style={overlayStyles} - className={overlayClassName} - visible={innerVisible} - closeOnOverlayClick={closeOnOverlayClick} - lockScroll={lockScroll} - duration={duration} - onClick={handleOverlayClick} - /> + <Overlay + {...{ + zIndex: index, + style: overlayStyles, + className: overlayClassName, + visible: innerVisible, + closeOnOverlayClick, + lockScroll, + duration, + onClick: handleOverlayClick + }} + />src/packages/popup/popup.taro.tsx (1)
Line range hint
188-193
: 优化 overlay 点击事件处理当前的实现可以通过使用可选链和逻辑运算符来简化:
- const onHandleClickOverlay = (e: ITouchEvent) => { - if (closeOnOverlayClick && visible && e.target === e.currentTarget) { - const closed = onOverlayClick && onOverlayClick(e) - closed && onClose && onClose() - closed && onCancel && onCancel() - } - } + const onHandleClickOverlay = (e: ITouchEvent) => { + if (closeOnOverlayClick && visible && e.target === e.currentTarget) { + const closed = onOverlayClick?.(e) + if (closed) { + onClose?.() + onCancel?.() + } + } + }src/packages/dialog/dialog.taro.tsx (1)
21-45
: 优化默认属性的类型定义建议将默认属性的类型定义拆分为更小的接口,以提高可维护性:
interface DialogOverlayProps { overlay: boolean overlayStyle: Record<string, any> overlayClassName: string zIndex: number } interface DialogContentProps { title: string content: string header: string footer: string } interface DialogButtonProps { confirmText: string cancelText: string hideConfirmButton: boolean hideCancelButton: boolean }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
scripts/build-taro.mjs
(4 hunks)src/packages/dialog/confirm.tsx
(2 hunks)src/packages/dialog/content.taro.tsx
(3 hunks)src/packages/dialog/content.tsx
(3 hunks)src/packages/dialog/demos/h5/demo1.tsx
(1 hunks)src/packages/dialog/dialog.taro.tsx
(8 hunks)src/packages/dialog/dialog.tsx
(5 hunks)src/packages/dialog/index.taro.ts
(1 hunks)src/packages/dialog/types.taro.ts
(1 hunks)src/packages/dialog/types.ts
(1 hunks)src/packages/fixednav/fixednav.taro.tsx
(6 hunks)src/packages/fixednav/fixednav.tsx
(5 hunks)src/packages/fixednav/index.taro.ts
(1 hunks)src/packages/fixednav/types.taro.ts
(1 hunks)src/packages/overlay/index.taro.ts
(1 hunks)src/packages/overlay/overlay.taro.tsx
(4 hunks)src/packages/overlay/overlay.tsx
(2 hunks)src/packages/overlay/types.taro.ts
(1 hunks)src/packages/popup/index.taro.ts
(1 hunks)src/packages/popup/popup.taro.tsx
(4 hunks)src/packages/popup/popup.tsx
(5 hunks)src/packages/popup/types.taro.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/packages/fixednav/types.taro.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/packages/popup/index.taro.ts
- src/packages/dialog/index.taro.ts
- src/packages/fixednav/index.taro.ts
- src/packages/overlay/index.taro.ts
- src/packages/overlay/overlay.taro.tsx
- src/packages/dialog/dialog.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/popup/types.taro.ts
[error] 10-10: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 11-11: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/types.taro.ts
[error] 26-26: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/content.taro.tsx
[error] 52-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/dialog/content.tsx
[error] 53-53: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/dialog/types.ts
[error] 31-31: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 45-45: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/dialog/dialog.taro.tsx
[error] 190-190: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/overlay/overlay.tsx
[error] 61-61: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 GitHub Check: codecov/patch
src/packages/dialog/confirm.tsx
[warning] 7-7: src/packages/dialog/confirm.tsx#L7
Added line #L7 was not covered by tests
[warning] 13-15: src/packages/dialog/confirm.tsx#L13-L15
Added lines #L13 - L15 were not covered by tests
[warning] 20-21: src/packages/dialog/confirm.tsx#L20-L21
Added lines #L20 - L21 were not covered by tests
[warning] 26-26: src/packages/dialog/confirm.tsx#L26
Added line #L26 was not covered by tests
[warning] 31-31: src/packages/dialog/confirm.tsx#L31
Added line #L31 was not covered by tests
[warning] 89-89: src/packages/dialog/confirm.tsx#L89
Added line #L89 was not covered by tests
🔇 Additional comments (8)
src/packages/overlay/overlay.tsx (1)
56-56
: 验证 zIndex 属性的有效性需要确保 zIndex 属性在所有场景下都能正确应用。
✅ Verification successful
zIndex 属性实现验证完成
经过验证,zIndex 属性实现完全符合要求:
- 默认值 1000 合理且不与其他组件冲突
- 支持自定义配置,满足不同场景需求
- 已有相应测试用例保证功能正确性
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查是否有其他组件可能会覆盖 overlay 的 z-index rg -g '*.{tsx,less,css,scss}' 'z-index' src/packages/Length of output: 3694
Script:
#!/bin/bash # Check overlay component implementation and types ast-grep --pattern 'interface $_Props { $$$ zIndex?: $_ $$$ }' # Check overlay implementation ast-grep --pattern 'const Overlay = ($_) => { $$$ }' # Find scss variable definition rg -g '*.scss' '\$overlay-zIndex' src/packages/Length of output: 239
Script:
#!/bin/bash # Check the full overlay component implementation cat src/packages/overlay/overlay.tsx # Find scss variable definitions fd -g '*variables*.scss' src/packages/ -x cat {}Length of output: 2004
scripts/build-taro.mjs (1)
22-24
: 构建脚本改进很好新增的
isTypesTaro
辅助函数和对.taro
扩展名的处理逻辑改进了构建过程,使其更加健壮。Also applies to: 52-53
src/packages/dialog/content.tsx (2)
5-12
: 验证默认值与 Taro 版本的一致性根据之前的评论,需要确保默认值与 Taro 版本保持一致。请检查以下默认值:
header: ''
footer: ''
42-44
: 改进类型转换当前的类型转换可能不够安全,建议明确指定
footerDirection
的类型。- [footerDirection]: footerDirection, + [footerDirection as 'horizontal' | 'vertical']: footerDirection,src/packages/dialog/demos/h5/demo1.tsx (1)
72-72
: 代码示例更新正确新增的 title 属性更新示例有助于用户理解动态更新对话框的功能。
src/packages/dialog/confirm.tsx (1)
7-9
: 需要增加测试覆盖率以下代码行缺少测试覆盖:
- ConfirmDialog 组件
- normalizeConfig 函数
- confirm 函数的参数处理
- dialogConfig 的初始化
- render 函数
- update 函数
建议添加相应的单元测试以确保代码质量和可靠性。
Also applies to: 13-15, 20-21, 26-26, 31-31, 89-89
✅ Verification successful
测试覆盖率确实不足
经验证,当前测试文件
src/packages/dialog/__test__/dialog.spec.tsx
仅包含基础 Dialog 组件的测试用例,确实缺少以下测试覆盖:
- ConfirmDialog 组件的渲染和功能测试
- normalizeConfig 函数的配置处理测试
- confirm 函数的参数处理测试
- dialogConfig 的初始化测试
- render 和 update 函数的行为测试
建议按照原始评审意见补充相关测试用例。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查现有的测试文件 fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts . src/packages/dialog/Length of output: 118
Script:
#!/bin/bash # Check content of dialog test file and search for confirm-related tests echo "=== Dialog Test Content ===" cat src/packages/dialog/__test__/dialog.spec.tsx echo -e "\n=== Searching for Confirm-related Tests ===" rg "confirm" src/packages/dialog/__test__/dialog.spec.tsxLength of output: 4585
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 7-7: src/packages/dialog/confirm.tsx#L7
Added line #L7 was not covered by testssrc/packages/fixednav/fixednav.tsx (1)
81-84
: 🛠️ Refactor suggestion增强类型安全性和错误处理
list.map 中使用 any 类型不够安全,建议添加明确的类型定义和错误处理。
- {list.map((item: any, index) => { + {list.map((item: FixedNavItem, index) => { return ( <div className={`${classPrefix}-list-item`} - onClick={(event) => onSelect(item, event)} + onClick={(event) => { + try { + onSelect(item, event) + } catch (error) { + console.error('选择项目时发生错误:', error) + } + }}Likely invalid or redundant comment.
src/packages/fixednav/fixednav.taro.tsx (1)
7-7
: 注意潜在的循环依赖问题从
./types.taro
导入类型可能会导致循环依赖。建议将共享类型移至一个独立的类型定义文件中。
const handleCancel = ( | ||
e: ITouchEvent | React.MouseEvent<HTMLButtonElement> | ||
) => { |
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.
🛠️ Refactor suggestion
统一事件处理类型
当前代码中混合使用了 ITouchEvent
和 React.MouseEvent
,建议统一使用 Taro 的事件类型:
- e: ITouchEvent | React.MouseEvent<HTMLButtonElement>
+ e: ITouchEvent
这样可以确保在小程序环境下的事件处理一致性。
Also applies to: 110-110, 132-132, 141-143
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)
scripts/build-taro.mjs (3)
22-24
: 建议增强类型检查函数的健壮性建议对输入参数进行验证,并使用更精确的字符串匹配方法。
const isTypesTaro = (name, type) => { + if (!name || !type || typeof name !== 'string' || typeof type !== 'string') { + return false + } - return name.indexOf(type) > -1 + return name.includes(type) }
38-38
: 优化代码可读性和类型安全性
- 条件表达式较复杂,建议拆分以提高可读性
- 建议添加 JSDoc 注释说明参数用途
+/** + * 转换文件路径 + * @param {Object} file - 文件对象 + * @param {Object} api - API对象 + * @param {boolean} replace - 是否替换.taro后缀 + */ const transform = (file, api, replace) => { // ... if (!path.node.source) return if (!alias) { - path.node.source.value = (!replace || replace && !isTypesTaro(path.node.source.value, 'types.taro')) - ? path.node.source.value.replace('.taro', '') - : path.node.source.value + const sourceValue = path.node.source.value + const shouldReplaceTaro = !replace || (replace && !isTypesTaro(sourceValue, 'types.taro')) + path.node.source.value = shouldReplaceTaro ? sourceValue.replace('.taro', '') : sourceValue return }Also applies to: 52-55
180-180
: 完善文件路径处理逻辑和注释
- 建议扩展注释,说明为什么要保留 types.taro 文件
- 文件路径处理逻辑可以更清晰
- // 修改文件名,除 types.taro 不改之外,其他都修改。 + // 处理文件路径: + // 1. 对于 types.taro 类型的文件保持原样 + // 2. 对于其他文件移除 .taro 后缀 + // 这样处理是为了保持类型文件的特殊性,避免与普通组件类型冲突 let to = file.replace('dist/types/src', '') - to = isTypesTaro(to, 'types.taro') ? to : to.replace('.taro', '') + const isTypeTaroFile = isTypesTaro(to, 'types.taro') + to = isTypeTaroFile ? to : to.replace('.taro', '')Also applies to: 183-185
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
基于提供的变更摘要,以下是更新后的发布说明:
新功能
zIndex
属性以增强组件的可配置性性能优化
重构
类型改进