Skip to content

[OLYMPUS-1661]: Incorporate Filament Phone Number input plugin into the common package - #180

Open
amit-canyon wants to merge 7 commits into
mainfrom
OLYMPUS-1661
Open

amit-canyon wants to merge 7 commits into
mainfrom
OLYMPUS-1661

Conversation

@amit-canyon

@amit-canyon amit-canyon commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Ticket(s) or GitHub Issue

Technical Description

Centralizes the Filament phone input plugin and shared configuration in Common for consistent reuse across applications.

  • Adds ysfkaya/filament-phone-input (^4.2) as a Composer dependency instead of copying the plugin source.
  • Configures shared defaults in CommonServiceProvider:
    • US default and initial country.
    • Strict input mode and lenient phone validation.
    • Disabled IP lookup using disableLookup().
    • International formatting for phone infolist entries.
  • Preserves upstream keyboard behaviour without custom JavaScript or additional tabindex attributes.
  • Allows applications to override the shared defaults.
  • Adds tests for default configuration, overrides, and international formatting.

Consuming applications can remove their duplicate configuration after updating Common.

Any deployment steps required?

No

Are any Feature Flags and/or Data Migrations that can eventually be removed Added?

No


Before contributing and submitting this PR, make sure you have Read, agree, and are compliant with the contributing guidelines.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved state synchronization, debounce, configuration, validation, and geolocation defects affect phone-field behavior.

7 open issues
What changed in this PR

This PR brings the upstream Filament phone-input components into canyongbs/common, allowing Canyon GBS applications to use and maintain them without the separate plugin dependency.

Changes:

  • Adds phone form, table, and infolist components under Common’s namespace.
  • Integrates frontend assets, asset registration, and phone-processing dependencies.
  • Adds third-party notices and tests for configuration, format conversion, and asset safeguards.
File Description
THIRD_PARTY_NOTICES.md Records upstream attribution and licenses.
tests/​Filament/​Forms/​Components/​PhoneInput/​PhoneInputTest.php Tests field options and disabling lookup.
tests/​Filament/​Forms/​Components/​PhoneInput/​PhoneInputNumberTypeTest.php Tests number-format mappings.
tests/​Filament/​Forms/​Components/​PhoneInput/​PhoneInputAssetsTest.php Checks accessibility source hooks and notice inclusion.
src/​Filament/​Tables/​Columns/​PhoneInput/​PhoneInputColumn.php Provides the deprecated column alias.
src/​Filament/​Tables/​Columns/​PhoneInput/​PhoneColumn.php Adds formatted table phone values.
src/​Filament/​Infolists/​Components/​PhoneInput/​PhoneEntry.php Adds formatted infolist phone values.
src/​Filament/​Forms/​Components/​PhoneInput/​PhoneInputNumberType.php Defines supported number formats.
src/​Filament/​Forms/​Components/​PhoneInput/​PhoneInput.php Implements field configuration, validation, formatting, and lookup.
src/​CommonServiceProvider.php Registers phone-input assets.
resources/​views/​filament/​forms/​components/​phone-input/​phone-input.blade.php Connects the field to Alpine and Livewire.
resources/​js/​filament/​phone-input.js Implements the country picker and state synchronization.
resources/​css/​phone-input.css Adds phone-input styling and dark-mode overrides.
package.json Adds intl-tel-input.
package-lock.json Locks the frontend dependency.
composer.lock Records phone-library dependencies and package-tools update.
composer.json Declares phone-processing and package-tools requirements.
bin/​build.js Builds phone assets and embeds WebP images as data URLs.

Review effort: Balanced

Comment thread resources/js/filament/phone-input.js Resolved
Comment thread resources/js/filament/phone-input.js Resolved
Comment thread resources/js/filament/phone-input.js Resolved
Comment thread resources/js/filament/phone-input.js Resolved
amit-canyon and others added 2 commits October 29, 2026 16:36
- Deleted PhoneInputNumberType.php, PhoneEntry.php, PhoneColumn.php, PhoneInputColumn.php, and associated tests to streamline the codebase.
- Introduced new accessibility script for phone input country listbox to enhance keyboard navigation.
- Updated JavaScript files to ensure accessibility compliance.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The accessibility observer remains attached to a detached body after SPA navigation, so country lists on subsequent pages never receive tabindex; it must be reconnected or attached to a stable ancestor, and the distributed bundle regenerated.

1 open issue
7 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The added tab stop lets focus enter searchable country lists while accessibility updates remain on the unfocused search input, leaving the focused list without an accessible active option.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Fix focus and aria-activedescendant handling in searchable dropdowns

resources/​js/​filament/​phone-input-a11y.js:40

When a searchable dropdown is rendered outside the country container, such as a fullscreen popup, this added tab stop lets keyboard users move focus from the search input into the list. Arrow keys then change the highlighted country, but aria-activedescendant is updated only on the now-unfocused search input, leaving the focused list without an accessible active option. Keep searchable lists out of the tab sequence, or implement matching listbox focus management, and regenerate the bundled asset.

1 resolved since last review

Review effort: Balanced

@amit-canyon
amit-canyon marked this pull request as ready for review October 29, 2026 16:09
@amit-canyon
amit-canyon requested a review from a team as a code owner October 29, 2026 16:09
@danharrin
danharrin requested review from danharrin and removed request for payal-canyon October 30, 2026 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants