-
-
Notifications
You must be signed in to change notification settings - Fork 77
Document phone form field #255
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
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughAdds Phone field documentation to backend/forms.md, including validation details, datalist-based option rendering, HTML5 attributes support, YAML example, and display behavior notes for edit and preview modes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/forms.md (1)
270-289: Add Phone to the table of contents.The new Phone field type is documented below but is missing from this list of available field types. It should be added between "Password" and "Radio List" to maintain alphabetical order.
📝 Proposed fix
- [Partial](`#partial`) - [Password](`#password`) +- [Phone](`#phone`) - [Radio List](`#radio-list`)
🤖 Fix all issues with AI agents
In `@backend/forms.md`:
- Around line 620-633: The telephone field's pattern "[0-9]{3}[0-9]{3}[0-9]{4}"
conflicts with the placeholder "xxx-xxx-xxxx"; update the telephone field to
make pattern and placeholder consistent by either (A) changing the pattern on
the telephone entry to accept optional hyphens (e.g., allow digits with optional
'-' between groups) so "xxx-xxx-xxxx" is valid, or (B) change the placeholder to
a plain 10-digit example (e.g., "xxxxxxxxxx") to match the current strict
digits-only pattern; edit the `telephone` YAML block (pattern and/or placeholder
attributes) accordingly.
🧹 Nitpick comments (1)
backend/forms.md (1)
616-642: Consider adding server-side validation guidance.For consistency with the Email and URL field documentation, consider adding a brief section on server-side validation. This would help users understand how to validate phone numbers on the model.
📖 Example validation section
Add after line 642:
#### Server-side validation To validate this field on save, define a validation rule in your model: ```php public $rules = [ 'telephone' => 'nullable|regex:/^[0-9]{3}-[0-9]{3}-[0-9]{4}$/', ];Tip: Use
nullableif the field is not required but must still match the pattern when provided.</details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between c2734e20372a4e23c94b75e965b77999fc63c96d and 195c98571fd6128d47c40cc99fea637232ea43ce. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `backend/forms.md` </details> <sub>✏️ Tip: You can disable this entire section by setting `review_details` to `false` in your review settings.</sub> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| ```yaml | ||
| telephone: | ||
| label: Phone number | ||
| type: phone | ||
| pattern: "[0-9]{3}[0-9]{3}[0-9]{4}" | ||
| placeholder: xxx-xxx-xxxx | ||
| maxlength: 20 | ||
| minlength: 12 | ||
| size: 20 | ||
| required: true | ||
| options: | ||
| 514-123-4567: First Test Phone Number | ||
| 800-111-2222: Second Test Phone 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.
Pattern and placeholder mismatch.
The pattern attribute specifies [0-9]{3}[0-9]{3}[0-9]{4} (10 consecutive digits), but the placeholder shows xxx-xxx-xxxx (with hyphens). This mismatch may confuse users, as input with hyphens would fail validation against the pattern.
🔧 Suggested fix options
Option 1: Adjust the pattern to accept hyphens:
- pattern: "[0-9]{3}[0-9]{3}[0-9]{4}"
+ pattern: "[0-9]{3}-[0-9]{3}-[0-9]{4}"Option 2: Update the placeholder to match the pattern (no hyphens):
- placeholder: xxx-xxx-xxxx
+ placeholder: xxxxxxxxxx📝 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.
| ```yaml | |
| telephone: | |
| label: Phone number | |
| type: phone | |
| pattern: "[0-9]{3}[0-9]{3}[0-9]{4}" | |
| placeholder: xxx-xxx-xxxx | |
| maxlength: 20 | |
| minlength: 12 | |
| size: 20 | |
| required: true | |
| options: | |
| 514-123-4567: First Test Phone Number | |
| 800-111-2222: Second Test Phone Number | |
| ``` |
| ```yaml | |
| telephone: | |
| label: Phone number | |
| type: phone | |
| pattern: "[0-9]{3}[0-9]{3}[0-9]{4}" | |
| placeholder: xxx-xxx-xxxx | |
| maxlength: 20 | |
| minlength: 12 | |
| size: 20 | |
| required: true | |
| options: | |
| 514-123-4567: First Test Phone Number | |
| 800-111-2222: Second Test Phone Number | |
| ``` |
🤖 Prompt for AI Agents
In `@backend/forms.md` around lines 620 - 633, The telephone field's pattern
"[0-9]{3}[0-9]{3}[0-9]{4}" conflicts with the placeholder "xxx-xxx-xxxx"; update
the telephone field to make pattern and placeholder consistent by either (A)
changing the pattern on the telephone entry to accept optional hyphens (e.g.,
allow digits with optional '-' between groups) so "xxx-xxx-xxxx" is valid, or
(B) change the placeholder to a plain 10-digit example (e.g., "xxxxxxxxxx") to
match the current strict digits-only pattern; edit the `telephone` YAML block
(pattern and/or placeholder attributes) accordingly.
related: wintercms/winter#1440
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.