-
-
Notifications
You must be signed in to change notification settings - Fork 233
Add phone form field #1440
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?
Add phone form field #1440
Conversation
WalkthroughA 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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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
🤖 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
📒 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 withrel="noopener noreferrer") and consistent output escaping viae().
25-39: Implementation looks good with one note onsizeattribute 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 HTMLsizeattribute.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'ssizeproperty and the HTML input'ssizeattribute.
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.
|
@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) ?>" |
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.
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);
}
}
Related docs PR: wintercms/docs#255
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.