Document phone form field #255
Conversation
WalkthroughAdds a Tel (phone) field documentation block to Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 -->
backend/forms.md
Outdated
| ```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.
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.
backend/forms.md
Outdated
| type: password | ||
| ``` | ||
|
|
||
| ### Tel |
There was a problem hiding this comment.
Needs to be moved to the T section and have a line added to the list of all the fields higher up on the page.
|
I made the changes. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/forms.md`:
- Line 712: Remove the duplicate "### Radio List" header found in
backend/forms.md (the orphaned header at line 712) so the document doesn't
contain two "Radio List" sections; locate the second "### Radio List" heading
that has no content and delete that single header line so the following "Text"
section directly follows the original Radio List content (the Radio List
documented earlier at lines ~618-646).
|
|
||
| See [Defining field options](#defining-field-options) for the different methods to specify the options. | ||
|
|
||
| ### Radio List |
There was a problem hiding this comment.
Remove duplicate "Radio List" header.
Line 712 contains a duplicate "### Radio List" header that should be removed. The Radio List section is already documented at lines 618-646, and this orphaned header has no content and immediately precedes the Text section.
📝 Proposed fix
-### Radio List
### TextBased on static analysis hints.
📝 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.
| ### Radio List | |
| ### Text |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
712-712: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
In `@backend/forms.md` at line 712, Remove the duplicate "### Radio List" header
found in backend/forms.md (the orphaned header at line 712) so the document
doesn't contain two "Radio List" sections; locate the second "### Radio List"
heading that has no content and delete that single header line so the following
"Text" section directly follows the original Radio List content (the Radio List
documented earlier at lines ~618-646).
related: wintercms/winter#1440
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.