-
Notifications
You must be signed in to change notification settings - Fork 263
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
fix(toast): load dependent CSS #2776
Conversation
Walkthrough此次更改在 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #2776 +/- ##
=======================================
Coverage 84.07% 84.07%
=======================================
Files 217 217
Lines 17830 17830
Branches 2609 2609
=======================================
Hits 14991 14991
Misses 2834 2834
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
95bd96c
to
3d67cfb
Compare
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (28)
src/utils/to-array.ts (1)
1-7
: 实现正确,建议添加文档和测试函数实现逻辑正确,类型定义恰当。建议添加以下改进:
- 添加 JSDoc 文档说明函数用途和参数类型
- 考虑处理更多边缘情况(如 NaN, Symbol 等)
建议添加如下文档:
+/** + * 将任意值转换为数组 + * @param value - 要转换的值 + * @returns 转换后的数组 + * @example + * toArray(1) // [1] + * toArray([1, 2]) // [1, 2] + * toArray(null) // [] + */ export function toArray<T>(value?: T | T[] | null): T[] {🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 3-4: src/utils/to-array.ts#L3-L4
Added lines #L3 - L4 were not covered by testssrc/utils/merge.ts (2)
1-29
: 缺少函数文档说明请添加 JSDoc 文档,说明:
- 函数的用途
- 参数说明
- 返回值说明
- 使用示例
示例文档:
/** * 深度合并多个对象或数组 * @param objects - 要合并的对象或数组列表 * @returns 合并后的对象或数组 * @example * merge({ a: 1 }, { b: 2 }) // { a: 1, b: 2 } * merge([1], [2]) // [1, 2] */🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 10-10: src/utils/merge.ts#L10
Added line #L10 was not covered by tests
[warning] 14-14: src/utils/merge.ts#L14
Added line #L14 was not covered by tests
[warning] 17-18: src/utils/merge.ts#L17-L18
Added lines #L17 - L18 were not covered by tests
26-26
: 建议优化空值处理当前使用
!!obj
过滤空值可能不够严谨,建议明确定义哪些值需要被过滤。-objects.filter((obj) => !!obj).forEach((obj) => mergeHelper(obj)) +objects.filter((obj) => obj != null && typeof obj === 'object').forEach((obj) => mergeHelper(obj))src/packages/form/types.ts (1)
24-24
: 表单字段值操作方法的类型优化
- 新增的
setFieldValue
方法使用泛型T
提供了更好的类型安全性setFieldsValue
方法参数类型从any
改为Store
提升了类型约束建议:
- 考虑为
setFieldValue
的泛型T
添加默认值的约束条件- 在文档中补充这两个方法的使用区别说明
- setFieldValue: <T = any>(name: NamePath, value: T) => void + setFieldValue: <T extends StoreValue = StoreValue>(name: NamePath, value: T) => voidAlso applies to: 26-26
src/packages/toast/toast.scss (3)
Line range hint
21-25
: 建议优化overlay相关样式变量的命名和组织
toast-overlay-default
类中的变量定义建议:
- 考虑将overlay相关的变量统一移至overlay.scss中
- 变量命名可以更具体,比如
--nutui-toast-overlay-bg-color
&-overlay-default { - --nutui-overlay-bg-color: rgba(0, 0, 0, 0); - --nutui-overlay-zIndex: 1300; + --nutui-toast-overlay-bg-color: rgba(0, 0, 0, 0); + --nutui-toast-overlay-zIndex: 1300; }
Line range hint
40-57
: 建议增强响应式设计支持toast组件的内部样式可以进一步优化:
- 考虑添加媒体查询以适应不同屏幕尺寸
- 最小宽度(30%)和最大宽度(95.7%)的值建议使用CSS变量定义,便于主题定制
&-inner { display: inline-block; position: absolute; left: 50%; top: $toast-inner-top; - min-width: 30%; - max-width: 95.7%; + min-width: var(--nutui-toast-min-width, 30%); + max-width: var(--nutui-toast-max-width, 95.7%); font-size: $toast-text-font-size; ... }
Line range hint
146-159
: 优化动画性能过渡动画相关建议:
- 考虑使用transform替代opacity来优化动画性能
- 可以添加will-change属性提升动画性能
- 建议使用CSS变量控制动画时长
.toast-fade-enter-active { - transition: opacity 0.3s; + transition: opacity var(--nutui-toast-transition-duration, 0.3s); + will-change: opacity; } .toast-fade-leave-active { - transition: opacity 0.3s; + transition: opacity var(--nutui-toast-transition-duration, 0.3s); + will-change: opacity; }src/packages/form/form.tsx (1)
50-50
: 建议优化 Context 相关实现虽然当前实现可以正常工作,但建议考虑以下优化:
- 将 Context 值对象提取为 useMemo 缓存的变量,避免不必要的重渲染
- 考虑添加 Context 值类型定义,增强类型安全性
建议按如下方式重构:
+ const contextValue = React.useMemo( + () => ({ formInstance, labelPosition, disabled, validateTrigger }), + [formInstance, labelPosition, disabled, validateTrigger] + ) return ( <form ...> <Cell.Group divider={divider}> - <Context.Provider value={{ formInstance, labelPosition, disabled, validateTrigger }}> + <Context.Provider value={contextValue}> {children} </Context.Provider>Also applies to: 53-53, 105-109
src/packages/form/form.taro.tsx (2)
51-54
: 建议改进属性解构的排序建议将相关的属性组织在一起,比如将验证相关的属性(
validateTrigger
、onFinish
、onFinishFailed
)放在一起,布局相关的属性(labelPosition
、starPosition
)放在一起。const { className, style, footer, children, initialValues, divider, disabled, - onFinish, - onFinishFailed, - validateTrigger, labelPosition, starPosition, + validateTrigger, + onFinish, + onFinishFailed, form, } = {
2-2
: 建议补充 Taro 组件的版本要求建议在文件顶部添加注释,说明该组件依赖的 @tarojs/components 最低版本要求,以便其他开发者了解兼容性要求。
+// @requires @tarojs/components >= 3.x import { Form as TForm } from '@tarojs/components'
src/packages/formitem/formitem.scss (1)
10-16
: 建议优化Flex布局属性flex相关属性的设置合理,但建议考虑以下优化:
- flex: 0 0 auto; + flex: 0 0 $form-item-label-width;这样可以确保标签宽度更加可控,避免内容过长时的布局问题。
src/packages/form/doc.md (2)
81-82
: 属性文档完善,建议补充使用示例新增的
validateTrigger
和disabled
属性文档描述清晰。建议在文档中为这两个属性添加具体的使用示例,以帮助开发者更好地理解其应用场景。建议在示例代码部分添加如下内容:
+ ### 自定义校验触发时机 + + ```tsx + <Form validateTrigger={['onChange', 'onBlur']}> + <Form.Item name="username" rules={[{ required: true }]}> + <Input placeholder="失焦和改变时都触发校验" /> + </Form.Item> + </Form> + ``` + + ### 禁用表单 + + ```tsx + <Form disabled> + <Form.Item name="username"> + <Input placeholder="整个表单被禁用" /> + </Form.Item> + </Form> + ```
129-130
: 方法描述准确,建议补充注意事项
setFieldsValue
和setFieldValue
的方法描述清晰。不过对于setFieldsValue
的说明,建议补充一些常见的使用场景和注意事项。建议在文档中添加以下补充说明:
+ > 注意: + > 1. `setFieldsValue` 会直接修改传入的对象,如需保持原对象不变,请使用 `structuredClone` 或 `JSON.parse(JSON.stringify())` 进行深拷贝 + > 2. `setFieldValue` 相比 `setFieldsValue` 更适合单个字段的更新场景,且性能更好src/packages/form/doc.zh-TW.md (2)
82-83
: 属性文档更新完善新增的
validateTrigger
和disabled
属性文档描述清晰,与组件功能相符。建议补充以下内容:
validateTrigger
可以添加示例值,如['onChange', 'onBlur']
disabled
可以说明对表单内所有表单项的影响
103-103
: 确保属性描述一致性Form.Item 的
validateTrigger
属性类型描述中的反引号使用不一致:-| validateTrigger | 統一設定字段觸發驗證的時機 | `string \| string[]` | `onChange` | +| validateTrigger | 統一設定字段觸發驗證的時機 | `string` \| `string[]` | `onChange` |src/packages/form/doc.taro.md (1)
129-130
: 方法文档更新合理
setFieldsValue
方法补充了重要的对象修改提示,有助于开发者避免潜在问题- 新增的
setFieldValue
方法文档简洁明了建议补充一个使用示例,以便开发者更好地理解这两个方法的区别。
src/packages/form/useform.taro.ts (3)
Line range hint
100-120
: 字段值更新逻辑建议优化当前实现有以下改进空间:
- 建议添加对
newStore
的类型验证- 考虑优化实体更新循环,可以使用 Set 或 Map 提高性能
- 建议添加错误处理机制
setFieldsValue = (newStore: any) => { + if (!newStore || typeof newStore !== 'object') { + console.warn('setFieldsValue: newStore should be an object'); + return; + } const nextStore = merge(this.store, newStore) this.updateStore(nextStore) // ... rest of the code }
122-126
: 新增单字段更新方法
setFieldValue
方法提供了更便捷的单字段更新方式,建议添加泛型约束以增强类型安全性:- setFieldValue = <T>(name: NamePath, value: T) => { + setFieldValue = <T extends unknown>(name: NamePath, value: T) => {
138-142
: 错误处理改进建议虽然添加了名称属性验证,但建议:
- 使用更详细的警告信息,包含字段上下文
- 考虑添加开发环境的详细堆栈信息
- console.warn('Form field missing name property') + console.warn(`Form field missing name property. This field will be ignored in form validation and submission.`, + process.env.NODE_ENV === 'development' ? new Error().stack : '')src/packages/form/doc.en-US.md (2)
83-84
: 建议补充 validateTrigger 属性的可选值说明validateTrigger 属性的文档可以更详细一些,建议:
- 列举具体支持哪些触发时机(如:onChange、onBlur 等)
- 添加数组形式的使用示例
-| validateTrigger | uniformly set the timing for fields to trigger validation | `string` \| `string[]` | `onChange` | +| validateTrigger | uniformly set the timing for fields to trigger validation. Supported values: 'onChange', 'onBlur'. Example: ['onChange', 'onBlur'] will trigger validation on both change and blur events | `string` \| `string[]` | `onChange` |
130-131
: 建议为新增的方法添加使用示例为了帮助开发者更好地理解这两个方法的区别和使用场景,建议:
- 为 setFieldsValue 添加对象拷贝的示例代码
- 为 setFieldValue 添加基础使用示例
建议在方法描述后添加示例:
### 示例 ```tsx // setFieldsValue 示例 const values = { name: 'nutui', age: 18 }; form.setFieldsValue({ ...values }); // 使用对象拷贝避免修改原对象 // setFieldValue 示例 form.setFieldValue('name', 'nutui'); form.setFieldValue(['user', 'name'], 'nutui'); // 支持嵌套路径</blockquote></details> <details> <summary>src/packages/form/useform.ts (2)</summary><blockquote> `122-126`: **为 `setFieldValue` 方法添加测试覆盖** 新添加的 `setFieldValue` 方法目前缺少测试用例。为确保方法的稳定性和正确性,建议添加相应的单元测试。 需要我协助编写测试代码或创建新的 GitHub Issue 吗? <details> <summary>🧰 Tools</summary> <details> <summary>🪛 GitHub Check: codecov/patch</summary> [warning] 123-126: src/packages/form/useform.ts#L123-L126 Added lines #L123 - L126 were not covered by tests </details> </details> --- `140-142`: **为缺少 `name` 属性的字段添加测试** 在 `validateEntities` 方法中,当字段缺少 `name` 属性时,会记录警告信息。然而,该场景未被测试覆盖。为了提高代码的健壮性,建议添加相应的测试用例。 是否需要我帮助编写测试用例或开一个新的 GitHub Issue 来跟踪此任务? <details> <summary>🧰 Tools</summary> <details> <summary>🪛 GitHub Check: codecov/patch</summary> [warning] 140-142: src/packages/form/useform.ts#L140-L142 Added lines #L140 - L142 were not covered by tests </details> </details> </blockquote></details> <details> <summary>src/packages/formitem/formitem.tsx (3)</summary><blockquote> `114-119`: **建议为新增的警告逻辑添加单元测试** 新增在开发环境下对于使用 `defaultValue` 的警告逻辑,目前缺少测试覆盖。为了确保代码的可靠性和可维护性,建议为此添加相应的单元测试。 <details> <summary>🧰 Tools</summary> <details> <summary>🪛 GitHub Check: codecov/patch</summary> [warning] 114-119: src/packages/formitem/formitem.tsx#L114-L119 Added lines #L114 - L119 were not covered by tests </details> </details> --- `195-200`: **建议为 `getClassNameWithDirection` 函数增加测试覆盖** `getClassNameWithDirection` 函数在 `this.context.labelPosition` 为假值的情况下缺少测试覆盖。建议添加单元测试,以覆盖所有逻辑分支,提高代码的稳定性和可靠性。 <details> <summary>🧰 Tools</summary> <details> <summary>🪛 GitHub Check: codecov/patch</summary> [warning] 199-199: src/packages/formitem/formitem.tsx#L199 Added line #L199 was not covered by tests </details> </details> --- `279-283`: **建议为 `noStyle` 分支添加测试** 在 `render` 方法中,当 `this.props.noStyle` 为 `true` 时,直接返回 `returnChildNode` 的逻辑目前没有被测试覆盖。建议添加对应的测试用例,确保该渲染分支的正确性。 <details> <summary>🧰 Tools</summary> <details> <summary>🪛 GitHub Check: codecov/patch</summary> [warning] 281-281: src/packages/formitem/formitem.tsx#L281 Added line #L281 was not covered by tests </details> </details> </blockquote></details> <details> <summary>src/packages/formitem/formitem.taro.tsx (2)</summary><blockquote> `110-120`: **优化默认值的设置方式** 在第115-120行,检测到子组件的`defaultValue`属性并在非生产环境下发出警告。建议统一通过`initialValue`来设置初始值,避免在运行时进行属性检查,以提升性能和一致性。 --- `242-243`: **使用可选链`?.`简化代码** 根据静态分析工具Biome的提示,第242-243行可以使用可选链`?.`来优化代码,防止因`null`或`undefined`导致的错误。 建议修改如下: ```diff -<View - className={`nut-cell-title ${this.getClassNameWithDirection('nut-form-item-label')}`} +<View + className={`nut-cell-title ${this.getClassNameWithDirection?.('nut-form-item-label')}`}
🧰 Tools
🪛 Biome
[error] 242-243: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🛑 Comments failed to post (4)
src/utils/merge.ts (1)
1-2: 🛠️ Refactor suggestion
建议增强类型安全性
建议使用更具体的类型定义来替代
any
,以提高代码的类型安全性和可维护性。-export function merge(...objects: any[]) { - const result: any = Array.isArray(objects[0]) ? [] : {} +export function merge<T extends object | any[]>(...objects: T[]): T { + const result = Array.isArray(objects[0]) ? [] : {} as T📝 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.export function merge<T extends object | any[]>(...objects: T[]): T { const result = Array.isArray(objects[0]) ? [] : {} as T
src/packages/formitem/formitem.scss (1)
110-144: 🛠️ Refactor suggestion
建议统一间距变量
新增的定位类中使用了硬编码的间距值(如24px, 12px),建议:
.nut-form-item-label-right { - padding-right: 24px; + padding-right: $form-item-label-padding; } .nut-form-item-label-left { - padding-left: 12px; + padding-left: $form-item-label-padding-left; }将间距值抽取为SCSS变量,以保持样式的一致性和可维护性。
Committable suggestion skipped: line range outside the PR's diff.
src/packages/formitem/formitem.taro.tsx (2)
189-189:
⚠️ Potential issue避免直接修改
context
中的errors
对象在第189行,直接对
this.context.formInstance.errors
进行赋值可能导致context
的不可预期变化。建议通过调用formInstance
提供的状态更新方法来修改errors
,以维护状态管理的完整性和可靠性。
196-201: 🛠️ Refactor suggestion
getClassNameWithDirection
方法的返回值可能为undefined
在第196-201行,如果
className
或this.context.labelPosition
为undefined
,方法可能返回undefined
。建议在返回值中确保className
始终为字符串,以防止在使用时发生错误。
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit