-
-
Notifications
You must be signed in to change notification settings - Fork 175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: ✨ 重置表单项为初始值,并移除校验结果 #719
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough本次变更涉及对 Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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 (
|
✅ Deploy Preview for wot-design-uni ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
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: 3
🧹 Outside diff range and nitpick comments (2)
src/uni_modules/wot-design-uni/components/wd-form/types.ts (1)
83-86
: 类型定义符合需求,建议补充文档说明
resetFields
方法的类型定义清晰且符合重置表单的需求。不过建议在注释中详细说明reset
和resetFields
两个方法的具体区别,以便开发者选择合适的方法使用。建议将注释修改为:
/** - * 重置表单项为初始值,并清空验证提示 + * 重置表单项为初始值,并清空验证提示。 + * 注意:与 reset 方法的区别是,reset 只清空验证提示,而 resetFields 会同时重置值为初始值 */src/pages/form/Index.vue (1)
198-200
: 建议增加空值检查以提高类型安全性当前实现使用了非空断言操作符(!),这可能在运行时导致潜在的空值错误。
建议修改为:
function resetFields1() { - form1.value!.resetFields() + if (form1.value) { + form1.value.resetFields() + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
docs/component/form.md
(1 hunks)src/pages/form/Index.vue
(3 hunks)src/uni_modules/wot-design-uni/components/wd-form/types.ts
(1 hunks)src/uni_modules/wot-design-uni/components/wd-form/wd-form.vue
(2 hunks)
🔇 Additional comments (5)
src/uni_modules/wot-design-uni/components/wd-form/wd-form.vue (2)
26-26
: 导入声明看起来没有问题!
onMounted
的导入符合 Vue 3 组合式 API 的最佳实践。
202-202
: 方法暴露的更新符合预期!
resetFields
方法的暴露正确使用了 FormExpose
类型,并与现有的 validate
和 reset
方法保持一致。
src/pages/form/Index.vue (2)
28-28
: 重置按钮实现符合需求
重置按钮的实现完全符合issue #701中提出的需求,并且保持了与提交按钮一致的样式。
226-228
: 样式实现合理
重置按钮的样式设置合理,与整体表单布局协调。
docs/component/form.md (1)
945-945
: 新增的重置功能文档完善且清晰
resetFields
事件的文档说明符合需求,描述准确清晰。该功能满足了 Issue #701 中提出的重置表单项的需求。
let initialValue: any = undefined | ||
function resetFields() { | ||
let model = props.model | ||
children.forEach((field) => { | ||
model[field.prop] = deepClone(initialValue[field.prop]) | ||
}) | ||
|
||
reset() | ||
} |
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.
需要增强类型安全性和错误处理
当前实现存在以下问题:
- 直接访问和修改
model[field.prop]
缺少类型检查 - 没有对
field.prop
进行空值检查 - 直接修改 props 可能违反 Vue 的单向数据流原则
建议按照以下方式重构:
-let model = props.model
-children.forEach((field) => {
- model[field.prop] = deepClone(initialValue[field.prop])
-})
+const model = props.model
+children.forEach((field) => {
+ if (field.prop && initialValue && initialValue[field.prop] !== undefined) {
+ // 建议通过 emit 事件通知父组件更新,而不是直接修改
+ emit('update:model', {
+ ...model,
+ [field.prop]: deepClone(initialValue[field.prop])
+ })
+ }
+})
Committable suggestion skipped: line range outside the PR's diff.
onMounted(() => { | ||
// 保存初始值 | ||
initialValue = deepClone(props.model) | ||
}) |
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.model
后续发生变化,初始值不会更新 - 组件卸载时没有清理
initialValue
建议改进如下:
+import { ref, watch } from 'vue'
-let initialValue: any = undefined
+const initialValue = ref<any>(undefined)
+watch(() => props.model, (newModel) => {
+ initialValue.value = deepClone(newModel)
+}, { immediate: true, deep: true })
-onMounted(() => {
- // 保存初始值
- initialValue = deepClone(props.model)
-})
+onUnmounted(() => {
+ initialValue.value = undefined
+})
Committable suggestion skipped: line range outside the PR's diff.
@@ -942,6 +942,7 @@ function handleIconClick() { | |||
| -------- | ------------------------------------------------------------------------------ | --------------- | -------- | | |||
| validate | 验证表单,支持传入一个 prop 来验证单个表单项,不传入 prop 时,会验证所有表单项 | `prop?: string` | 0.2.0 | | |||
| reset | 重置校验结果 | - | 0.2.0 | | |||
| resetFields | 重置表单项为初始值,并移除校验结果 | - | $LOWEST_VERSION$ | |
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.
需要替换版本号占位符
$LOWEST_VERSION$
需要替换为实际的版本号。
-| resetFields | 重置表单项为初始值,并移除校验结果 | - | $LOWEST_VERSION$ |
+| resetFields | 重置表单项为初始值,并移除校验结果 | - | 1.4.0 |
📝 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.
| resetFields | 重置表单项为初始值,并移除校验结果 | - | $LOWEST_VERSION$ | | |
| resetFields | 重置表单项为初始值,并移除校验结果 | - | 1.4.0 | |
✅ Closes: #701
🤔 这个 PR 的性质是?(至少选择一个)
🔗 相关 Issue
#701
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新功能
文档
wd-form
组件的文档,提供更清晰的功能和用法说明。