feat(Form): add HTML5 validation to programmatic submit#6002
feat(Form): add HTML5 validation to programmatic submit#6002IlyaSemenov wants to merge 1 commit intonuxt:v4from
Conversation
📝 WalkthroughWalkthroughAdds HTML5 validation to programmatic form submission: Form.vue introduces a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/content/docs/2.components/form.md (1)
229-230:⚠️ Potential issue | 🟡 MinorPre-existing typo: missing
|(pipe) operator in type signatures.Lines 229 and 230 are missing the union pipe between
keyof TandRegExp:
- Line 229:
getErrors(path?: keyof T RegExp)→ should bekeyof T | RegExp- Line 230:
setErrors(errors: FormError[], name?: keyof T RegExp)→ should bekeyof T | RegExpNot introduced by this PR, but worth fixing while you're editing this table.
🧹 Nitpick comments (2)
src/runtime/components/Form.vue (1)
105-105: Consider narrowing the ref type to reflect the actual runtime possibilities.
formElis typed asref<HTMLFormElement>()but it can also hold anHTMLDivElementwhen the form is nested. Theinstanceofguard at Line 403 handles this at runtime, but the type is slightly misleading.Suggested type change
-const formEl = ref<HTMLFormElement>() +const formEl = ref<HTMLFormElement | HTMLDivElement>()test/components/Form.spec.ts (1)
645-667: Consider usingafterEachfor spy cleanup to prevent leaks on test failure.If a test throws before reaching
mockRestore(), the spy leaks to subsequent tests. A safer pattern:Suggested pattern
describe('HTML5 validation', () => { + let reportValiditySpy: ReturnType<typeof vi.spyOn> | undefined + + afterEach(() => { + reportValiditySpy?.mockRestore() + }) + it('programmatic submit() triggers HTML5 validation and prevents submission when invalid', async () => { const onSubmit = vi.fn() - const reportValiditySpy = vi.spyOn(HTMLFormElement.prototype, 'reportValidity').mockReturnValue(false) + reportValiditySpy = vi.spyOn(HTMLFormElement.prototype, 'reportValidity').mockReturnValue(false) // ... rest of test without mockRestore at the end
d0cdb27 to
e2d3435
Compare
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/components/Form.spec.ts`:
- Around line 647-650: The test file uses afterEach but it's not imported from
vitest causing a build error; update the vitest import (the import statement
that currently brings in functions like describe/it/etc.) to also include
afterEach so the afterEach(() => { reportValiditySpy?.mockRestore();
reportValiditySpy = undefined }) call resolves, and ensure the symbol name is
spelled exactly as afterEach to match the test hook.
e2d3435 to
448888a
Compare
🔗 Linked issue
Resolves #5139.
❓ Type of change
📚 Description
When calling
form.submit()programmatically, the form now triggers native HTML5 validation (required, type, min, max, etc.) before submission. If validation fails, submission is prevented and browser validation messages are displayed.This is particularly useful when the submit button is outside the form element, such as in modal footers.
📝 Checklist
💡 Notes
This adds an inner template ref within the
UFormcomponent. I could instead usedocument.getElementById(), but then the tests wouldn't work because the fixture mount helper doesn't actually insert the mounted component intodocument.body. I could rework the helper to address that, but the template ref approach seemed cleaner as it introduced less coupling overall.EDIT: As suggested by coderabbit, this also fixes nearby documentation formatting errors (not directly affected by the PR).