Skip to content

Enhance phone validation with country picker and richer error messages#19079

Open
Skrypt wants to merge 41 commits into
mainfrom
skrypt/#19052
Open

Enhance phone validation with country picker and richer error messages#19079
Skrypt wants to merge 41 commits into
mainfrom
skrypt/#19052

Conversation

@Skrypt

@Skrypt Skrypt commented Mar 28, 2026

Copy link
Copy Markdown
Contributor

Description

  • Replace PhoneValidationResult with Result<PhoneEntry> from OrchardCore.Infrastructure in IPhoneFormatValidator.Validate(), keeping PhoneValidationResult marked [Obsolete] for backward compatibility
  • Mark IsValid() as [Obsolete] with a default interface implementation that delegates to Validate(), so existing implementations remain compatible without changes
  • Add XML API docs with a code example to IPhoneFormatValidator showing how to call Validate() and access E164Number, NationalNumber, RegionCode, and CountryCode; PhoneEntry also has full XML docs
  • Implement rich validation in DefaultPhoneFormatValidator using libphonenumber-csharp, returning all previously available field values via PhoneEntry inside Result<PhoneEntry> with detailed localized error messages (invalid country code, too short, too long, etc.)
  • Add intl-tel-input phone input component with country dropdown, flags, dial codes, as-you-type formatting, and inline validation icons
  • Bundle as ES module via Vite with dark mode support and OrchardCore asset conventions
  • Update UserPhoneNumber.Edit.cshtml and SmsAuthenticator/Index.cshtml to use the new component
  • Normalize phone numbers to E.164 format on save in UserPhoneNumberDisplayDriver and SmsAuthenticatorController
  • Remove DefaultPhoneRegion from LoginSettings entirely; use Thread.CurrentThread.CurrentUICulture as the server-side region fallback when no region code is submitted
  • Phone editor country picker starts empty by default (no auto-IP detection, no server-side pre-selection from thread culture); users can optionally pick a region, and the server uses the thread culture region as a fallback
  • Add .node-version file (24.14.1) to fix CI copilot job failure caused by the branch missing the file introduced in a recent main PR
  • Add development/ to not_in_nav in mkdocs.yml so internal development plan docs under src/docs/development/ are excluded from the docs navigation without breaking the "Validating Building the Documentation" CI workflow

Phone editor with country picker

image

Skrypt added 2 commits March 28, 2026 11:51
#19052)

- Add PhoneValidationResult model and Validate() method to IPhoneFormatValidator
  with default interface implementation for backward compatibility
- Implement rich validation in DefaultPhoneFormatValidator using libphonenumber-csharp
  with specific error messages (invalid country code, too short, too long, etc.)
- Add intl-tel-input phone input component with country dropdown, flags, dial codes,
  as-you-type formatting, and inline validation icons
- Bundle as ES module via Vite with dark mode support and OrchardCore asset conventions
- Update UserPhoneNumber.Edit.cshtml and SmsAuthenticator/Index.cshtml to use new component
- Normalize phone numbers to E.164 format on save in UserInformationDisplayDriver
  and SmsAuthenticatorController
- Add DefaultPhoneRegion setting to LoginSettings with admin UI configuration
- Add RegionCode and DefaultRegion to EditUserPhoneNumberViewModel
@Skrypt

Skrypt commented Mar 28, 2026

Copy link
Copy Markdown
Contributor Author

@copilot where should I put these development plans documentation so that it doesn't break the "Validating building the documentation" Github workflow?

Copilot AI commented Mar 28, 2026

Copy link
Copy Markdown
Contributor

@copilot where should I put these development plans documentation so that it doesn't break the "Validating building the documentation" Github workflow?

Development plan docs should go in src/docs/development/ (e.g. src/docs/development/features/phone-validation/plan.md). I've added development/ to the not_in_nav section in mkdocs.yml (commit d483216) so any files under that directory are intentionally excluded from the docs navigation without triggering the strict-mode warning that breaks the CI workflow.

Copilot AI changed the title Enhance phone validation with country picker and richer error message… Enhance phone validation with country picker and richer error messages Mar 28, 2026
@Skrypt Skrypt marked this pull request as ready for review March 28, 2026 16:01
Extract the phone input validation icons and hidden input syncing logic into a new @orchardcore/bloom component (phone-input.ts). The Bloom component is library-agnostic — it receives an intl-tel-input instance rather than importing it directly, keeping Bloom dependency-free. The entry point in OrchardCore.Users handles DOM construction, intl-tel-input initialization, and CSS, then delegates to the Bloom component. This allows the phone input behavior to be reused by any module that needs phone validation.
Comment thread src/OrchardCore/OrchardCore.Sms.Abstractions/IPhoneFormatValidator.cs Outdated
Comment thread src/OrchardCore/OrchardCore.Sms.Abstractions/IPhoneFormatValidator.cs Outdated
Comment thread src/OrchardCore/OrchardCore.Sms.Core/DefaultPhoneFormatValidator.cs Outdated
Comment thread src/OrchardCore/OrchardCore.Sms.Abstractions/IPhoneFormatValidator.cs Outdated
Comment thread src/OrchardCore.Modules/OrchardCore.Users/Views/LoginSettings.Edit.cshtml Outdated

@sebastienros sebastienros left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The Tel editor, should allow users to optionally pick a region. But by default it will be empty and then the phone editor will use the Thread region.

Copilot AI commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

The Tel editor, should allow users to optionally pick a region. But by default it will be empty and then the phone editor will use the Thread region.

Done in commit 07929cfPhoneInput.cshtml no longer pre-computes a region from the thread culture, so data-phone-region is empty by default. The JS initializes intl-tel-input with initialCountry: '' instead of 'auto', meaning no country is pre-selected. When the user picks a country, intl-tel-input formats accordingly; when no region is submitted, the server falls back to Thread.CurrentThread.CurrentUICulture.

@Skrypt

Skrypt commented Apr 6, 2026

Copy link
Copy Markdown
Contributor Author

@copilot

image

We introduced this file in a recent PR I did. Why would it fail to find that file?

Copilot AI commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

@copilot

image

...

The .node-version file was added to main after our branch was created, so our branch didn't have it. Fixed in commit 34c43cc by adding .node-version (content: 24.14.1) matching what's in main.

hishamco and others added 8 commits April 6, 2026 13:17
Co-authored-by: Zoltán Lehóczky <zoltan.lehoczky@lombiq.com>
* File upload limit docs

* Address feedback
…es page (#19113)

Co-authored-by: andy zheng <yunnancanada@gmail.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…tup (#19109)

Co-authored-by: Georg von Kries <gvk@creativbox.net>
Co-authored-by: Georg von Kries <gvk@creativbox.net>
@github-actions

github-actions Bot commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

# Conflicts:
#	src/OrchardCore.Modules/OrchardCore.ContentFields/Views/TextField-Tel.Edit.cshtml
@hishamco

Copy link
Copy Markdown
Member

Is it ready for review?

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

@@ -0,0 +1,18 @@
@{

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a new module that could be moved to a different PR. I refactored the MonacoEditor integrations with a new MonacoEditor shape to show an example and also show the utility of the module. We have a lot of code in OC that could make use of a CommonShape module like this.

@sebastienros

Copy link
Copy Markdown
Member

Content Type picker, Media manager, are these shapes or components (vuejs or viewcomponent). Should we follow the same pattern? I wonder if these should be shapes.

@Skrypt

Skrypt commented Apr 16, 2026

Copy link
Copy Markdown
Contributor Author

I need something shareable. Either shapes (server side) and / or common JS framework "Bloom". Need to think about it. But also, this would centralize components in one place instead of having them everywhere.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

@MikeAlhayek

Copy link
Copy Markdown
Member

@Skrypt FYI: I recently needed something similar, so I created a new field PhoneField along with a PhoneNumberService as you can see here CrestApps/CrestApps.OrchardCore#468 and the service here https://github.com/CrestApps/CrestApps.OrchardCore/blob/c27950bc9e85e2bbfd6e7a17b624332bc42fc159/src/Modules/CrestApps.OrchardCore.PhoneNumbers/DefaultPhoneNumberService.cs#L10

@Skrypt

Skrypt commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

I'm not sure what you mean but the goal, I think, is to enable the Phone Field only when necessary based on features enabled. I need to wrap my head around this PR back again when I get there. I'm lost, too many different tasks lately.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants