Skip to content

Conversation

@mjauvin
Copy link
Member

@mjauvin mjauvin commented Jan 14, 2026

Related docs PR: wintercms/docs#255

phone:
    type: phone
    label: Phone number
    pattern: "[0-9]{3}-[0-9]{3}-[0-9]{4}"
    placeholder: xxx-xxx-xxxx
    minlength: 12
    maxlength: 20
    size: 20
image

Summary by CodeRabbit

  • New Features
    • Added telephone input field widget supporting preview and edit modes
    • Includes optional datalist with label translation capabilities
    • Supports validation attributes: autocomplete, maxlength, minlength, pattern, and placeholder

✏️ Tip: You can customize this high-level summary in your review settings.

@mjauvin mjauvin added this to the 1.2.10 milestone Jan 14, 2026
@mjauvin mjauvin requested a review from LukeTowers January 14, 2026 21:39
@mjauvin mjauvin self-assigned this Jan 14, 2026
@mjauvin mjauvin added the enhancement PRs that implement a new feature or substantial change label Jan 14, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 14, 2026

Walkthrough

A new PHP partial for rendering a telephone input field with dual modes: preview mode displays a tel: link or empty placeholder, while edit mode renders a tel input with optional datalist support and dynamic attributes including autocomplete, validation patterns, and placeholder text.

Changes

Cohort / File(s) Summary
Telephone Input Partial
modules/backend/widgets/form/partials/_field_tel.php
New file introducing telephone field partial with conditional preview/edit mode rendering, dynamic attribute binding, optional datalist with label translation support, and error handling via type-checking

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add phone form field' directly and clearly describes the main change: introducing a new telephone input field type for forms.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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

🤖 Fix all issues with AI agents
In `@modules/backend/widgets/form/partials/_field_phone.php`:
- Line 1: The file’s top HTML comment incorrectly reads "URL" due to copy-paste;
update the comment in the _field_phone.php partial to say "Phone" (or a more
descriptive "Phone field") so it accurately reflects the field purpose; locate
the comment token at the top of the partial and replace the text "URL" with
"Phone".
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d4bd85f and feda831.

📒 Files selected for processing (1)
  • modules/backend/widgets/form/partials/_field_phone.php
🧰 Additional context used
🧬 Code graph analysis (1)
modules/backend/widgets/form/partials/_field_phone.php (1)
modules/backend/classes/FormField.php (3)
  • options (228-246)
  • size (217-221)
  • getAttributes (431-445)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: windows-latest / PHP 8.3
  • GitHub Check: windows-latest / PHP 8.1
  • GitHub Check: ubuntu-latest / PHP 8.4
  • GitHub Check: ubuntu-latest / PHP 8.1
  • GitHub Check: windows-latest / PHP 8.2
  • GitHub Check: windows-latest / PHP 8.4
  • GitHub Check: ubuntu-latest / PHP 8.3
  • GitHub Check: ubuntu-latest / PHP 8.2
  • GitHub Check: windows-latest / JavaScript
🔇 Additional comments (3)
modules/backend/widgets/form/partials/_field_phone.php (3)

11-23: LGTM!

Preview mode implementation is well-structured with proper security measures (target="_blank" paired with rel="noopener noreferrer") and consistent output escaping via e().


25-39: Implementation looks good with one note on size attribute semantics.

The conditional attribute rendering with type validation is well done. The is_numeric($field->size) check on line 36 correctly filters out the framework's string-based size values (like 'large'), avoiding conflict with the HTML size attribute.

Based on the relevant snippet showing FormField::size() defaults to 'large', the numeric check ensures only explicit numeric values set the HTML attribute. Consider documenting this distinction in the field's YAML documentation to avoid confusion between the framework's size property and the HTML input's size attribute.


40-47: LGTM!

The datalist implementation correctly handles both associative and sequential arrays via the integer key check on line 43, and properly applies translation to labels for i18n support.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@LukeTowers
Copy link
Member

@mjauvin can you do up a docs PR for this field too? ❤️

@mjauvin
Copy link
Member Author

mjauvin commented Jan 14, 2026

@mjauvin can you do up a docs PR for this field too? ❤️

Done!

<?php if ($this->previewMode): ?>
<?php if ($field->value): ?>
<a
href="tel:<?= e($field->value) ?>"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need stripping of all non-integer characters at very least. There's a lot more complicated logic around reliably making a compliant phone number value (see below class that focuses on just normalizing Canadian phone numbers) that we could explore at some point.

@mjauvin thought, what do you think about having the country code portion of the phone number? See https://intl-tel-input.com/, it could perhaps be an extra property to enable the dropdown and / or an option to set which countries are supported; that would make it easier for a phone number lib like below to process the input data accordingly. If we were to go that route I could see an argument for making the field a dedicated FormWidget.

<?php

namespace Cadets\Core\Classes\Helpers;

use Propaganistas\LaravelPhone\PhoneNumber as PhoneNumberLib;
use Winter\Storm\Support\Str;

/**
 * Helper class to parse phone numbers
 *
 * @see https://stackoverflow.com/a/67810744/6652884
 * @see https://github.com/google/libphonenumber/blob/master/FALSEHOODS.md
 */
class PhoneNumber
{
    protected const EXTENSION_SEPARATOR = ';';

    /**
     * @see https://www.allareacodes.com/canadian_area_codes.htm
     */
    protected const CANADIAN_AREA_CODES = [
        '204' => 'Manitoba',
        '226' => 'London, ON',
        '236' => 'Vancouver, BC',
        '249' => 'Sudbury, ON',
        '250' => 'Kelowna, BC',
        '263' => 'Montreal, QC',
        '289' => 'Hamilton, ON',
        '306' => 'Saskatchewan',
        '343' => 'Ottawa, ON',
        '354' => 'Granby, QC',
        '365' => 'Hamilton, ON',
        '367' => 'Quebec, QC',
        '368' => 'Calgary, AB',
        '403' => 'Calgary, AB',
        '416' => 'Toronto, ON',
        '418' => 'Quebec, QC',
        '431' => 'Manitoba',
        '437' => 'Toronto, ON',
        '438' => 'Montreal, QC',
        '450' => 'Granby, QC',
        '468' => 'Sherbrooke, QC',
        '474' => 'Saskatchewan',
        '506' => 'New Brunswick',
        '514' => 'Montreal, QC',
        '519' => 'London, ON',
        '548' => 'London, ON',
        '579' => 'Granby, QC',
        '581' => 'Quebec, QC',
        '584' => 'Manitoba',
        '587' => 'Calgary, AB',
        '604' => 'Vancouver, BC',
        '613' => 'Ottawa, ON',
        '639' => 'Saskatchewan',
        '647' => 'Toronto, ON',
        '672' => 'Vancouver, BC',
        '683' => 'Sudbury, ON',
        '705' => 'Sudbury, ON',
        '709' => 'Newfoundland/Labrador',
        '742' => 'Hamilton, ON',
        '753' => 'Ottawa, ON',
        '778' => 'Vancouver, BC',
        '780' => 'Edmonton, AB',
        '782' => 'Nova Scotia/PE Island',
        '800' => 'Commercial',
        '807' => 'Kenora, ON',
        '819' => 'Sherbrooke, QC',
        '825' => 'Calgary, AB',
        '867' => 'Northern Canada',
        '873' => 'Sherbrooke, QC',
        '902' => 'Nova Scotia/PE Island',
        '905' => 'Hamilton, ON',
    ];

    /**
     * Attempt to parse the provided phone number as a Canadian phone number
     */
    public static function parse(string $number): ?string
    {
        if (Str::contains($number, ['@'])) {
            return null;
        }

        // Normalize by removing non-numeric characters
        $digitsOnly = static::onlyDigits($number);

        // Return null if no digits are present
        if (empty($digitsOnly)) {
            return null;
        }

        // Remove leading '1' if present
        if (substr($digitsOnly, 0, 1) === '1') {
            $digitsOnly = substr($digitsOnly, 1);
        }

        // Check for valid Canadian area code
        $areaCode = substr($digitsOnly, 0, 3);
        if (!array_key_exists($areaCode, self::CANADIAN_AREA_CODES)) {
            return null;
        }

        $number = self::parseExtension($number);

        try {
            $parts = explode(static::EXTENSION_SEPARATOR, $number);
            $number = (new PhoneNumberLib($parts[0], 'CA'))->formatE164();
            $number .= !empty($parts[1]) ? static::EXTENSION_SEPARATOR . $parts[1] : '';
        } catch (\Exception $e) {
            return null;
        }

        return $number;
    }

    /**
     * Split the provided number into the number and extension parts
     */
    public static function parseExtension(string $number): ?string
    {
        // Remove non-numeric characters, except for extension identifiers and +
        $normalizedCase = preg_replace('/[^\d+xextnsionp#]/i', '', $number);

        // Regex to extract extension
        $extRegex = '/(?:extension|ext|x|poste|#|opt)\s*(\d+)/i';
        $extension = '';
        if (preg_match($extRegex, $normalizedCase, $extMatches)) {
            $extension = static::onlyDigits($extMatches[1]);

            // Check for "123-456-7890 or 987-654-3210"
            if (strlen($extension) === 10) {
                $extension = false;
            }

            // Remove extension part from the number
            $normalizedCase = preg_replace($extRegex, '', $normalizedCase);
        }

        $normalizedCase = static::onlyDigits($normalizedCase);

        // Remove leading '1' if present
        if (substr($normalizedCase, 0, 1) === '1') {
            $normalizedCase = substr($normalizedCase, 1);
        }

        // Check for remaining surplus digits and use as extension
        if (strlen($normalizedCase) > 10) {
            $number = substr($normalizedCase, 0, 10);
            $extension = substr($normalizedCase, 10);
            $normalizedCase = $number;
        }

        return $normalizedCase . (!empty($extension) ? static::EXTENSION_SEPARATOR . $extension : '');
    }

    protected static function onlyDigits(string $number): string
    {
        return preg_replace('/[^\d]/', '', $number);
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement PRs that implement a new feature or substantial change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants