[7418] Input controls (link, phone, email)#4818
Merged
sumerjabri merged 7 commits intocraftercms:developfrom Mar 17, 2026
Merged
[7418] Input controls (link, phone, email)#4818sumerjabri merged 7 commits intocraftercms:developfrom
sumerjabri merged 7 commits intocraftercms:developfrom
Conversation
…o feature/7418-input-controls # Conflicts: # ui/app/src/components/FormsEngine/lib/validators.ts
60 tasks
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds three new input field types (email, link, phone): new descriptor files, registration in descriptors index and controlMap, value retrievers/serializers entries, and dedicated validators integrated into the FormsEngine validation pipeline. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as ContentType UI
participant Descriptors as Descriptor Registry
participant Renderer as FormsEngine (controlMap)
participant Control as Text Control
participant Validators as FormsEngine Validators
UI->>Descriptors: request control descriptors
Descriptors-->>UI: descriptors for input-email / input-link / input-phone
UI->>Renderer: build form with control type (e.g., 'input-email')
Renderer->>Control: lazy-load Text control component
Control->>Validators: validate input value (uses input-email/link/phone validator)
Validators-->>Control: return validation result/messages
Control-->>UI: display field and validation feedback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 2
🧹 Nitpick comments (1)
ui/app/src/components/FormsEngine/lib/validators.ts (1)
396-440: Consider consolidating duplicated wrapper validators.
inputEmailValidator,inputLinkValidator, andinputPhoneValidatorshare identical structure and only vary the message. A tiny factory/helper would reduce repetition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/src/components/FormsEngine/lib/validators.ts` around lines 396 - 440, Create a small factory that returns a field validator by accepting the pattern message and delegating to inputValidator (e.g., make a function like createInputValidator(patternMessage: string) that calls inputValidator with defineMessage({ defaultMessage: patternMessage })); then replace inputEmailValidator, inputLinkValidator, and inputPhoneValidator with calls to that factory (or simple one-line wrappers) so each just supplies its unique message while reusing the common logic; reference inputValidator and defineMessage to build the pattern option and keep the existing function signatures (field: ContentTypeField, currentValue: string, messages?: FieldValidityMessage[]).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ui/app/src/components/ContentTypeManagement/descriptors/controls/inputLink.ts`:
- Line 77: The default regex in the inputLink control is wrong (uses . instead
of literal \. and [^s]* instead of non-whitespace \S*); update the defaultValue
string used in the inputLink control to escape dots and use \S for
non-whitespace so it accepts valid URLs — e.g., change the pattern to something
like '^(https?://)?[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}(\.[a-zA-Z]{2,})?(/\S*)?$' by
editing the defaultValue value in that file.
In `@ui/app/src/components/FormsEngine/lib/validators.ts`:
- Line 347: Remove the stale JSDoc param entry for customValidationMessages from
the numericInputValidator documentation: locate the JSDoc block above the
numericInputValidator function and delete the line that describes "@param
customValidationMessages" so the doc matches the function signature; ensure the
remaining JSDoc accurately reflects existing parameters for
numericInputValidator.
---
Nitpick comments:
In `@ui/app/src/components/FormsEngine/lib/validators.ts`:
- Around line 396-440: Create a small factory that returns a field validator by
accepting the pattern message and delegating to inputValidator (e.g., make a
function like createInputValidator(patternMessage: string) that calls
inputValidator with defineMessage({ defaultMessage: patternMessage })); then
replace inputEmailValidator, inputLinkValidator, and inputPhoneValidator with
calls to that factory (or simple one-line wrappers) so each just supplies its
unique message while reusing the common logic; reference inputValidator and
defineMessage to build the pattern option and keep the existing function
signatures (field: ContentTypeField, currentValue: string, messages?:
FieldValidityMessage[]).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
ui/app/src/components/ContentTypeManagement/descriptors/controls/index.tsui/app/src/components/ContentTypeManagement/descriptors/controls/inputEmail.tsui/app/src/components/ContentTypeManagement/descriptors/controls/inputLink.tsui/app/src/components/ContentTypeManagement/descriptors/controls/linkPhone.tsui/app/src/components/FormsEngine/lib/controlMap.tsui/app/src/components/FormsEngine/lib/validators.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ui/app/src/components/FormsEngine/lib/validators.ts (1)
396-440: Consider deduplicating the three pattern-specific wrapper validators.This is repetitive and can be simplified with a small factory to keep future additions cleaner.
🧹 Optional refactor
+const createPatternValidator = (defaultMessage: string) => { + return (field: ContentTypeField, currentValue: string, messages?: FieldValidityMessage[]) => + inputValidator(field, currentValue, messages, { + pattern: defineMessage({ defaultMessage }) + }); +}; + -const inputEmailValidator = ( - field: ContentTypeField, - currentValue: string, - messages?: FieldValidityMessage[] -): boolean => { - return inputValidator(field, currentValue, messages, { - pattern: defineMessage({ defaultMessage: 'Please enter a valid email address.' }) - }); -}; +const inputEmailValidator = createPatternValidator('Please enter a valid email address.'); -const inputLinkValidator = ( - field: ContentTypeField, - currentValue: string, - messages?: FieldValidityMessage[] -): boolean => { - return inputValidator(field, currentValue, messages, { - pattern: defineMessage({ defaultMessage: 'Please enter a valid URL.' }) - }); -}; +const inputLinkValidator = createPatternValidator('Please enter a valid URL.'); -const inputPhoneValidator = ( - field: ContentTypeField, - currentValue: string, - messages?: FieldValidityMessage[] -): boolean => { - return inputValidator(field, currentValue, messages, { - pattern: defineMessage({ defaultMessage: 'Please enter a valid phone number.' }) - }); -}; +const inputPhoneValidator = createPatternValidator('Please enter a valid phone number.');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/src/components/FormsEngine/lib/validators.ts` around lines 396 - 440, The three nearly identical wrappers inputEmailValidator, inputLinkValidator, and inputPhoneValidator should be consolidated with a small factory to avoid duplication: create a helper (e.g., makePatternValidator) that accepts a pattern message (defineMessage(...)) and returns a validator function compatible with inputValidator, then replace inputEmailValidator, inputLinkValidator, and inputPhoneValidator by calling that factory with their respective messages; ensure the factory returns the same signature (field, currentValue, messages) and internally calls inputValidator so existing usage sites need no changes.ui/app/src/components/FormsEngine/lib/valueSerializers.ts (1)
96-98: Align new input serializers with text-field escaping behavior.These new text-like controls are currently passthrough. If
escapeContentis configured, they won’t follow the same serialization path as other text inputs usingprepareString.♻️ Suggested adjustment
- 'input-email': undefined, - 'input-link': undefined, - 'input-phone': undefined + 'input-email': (field, value) => prepareString(field, value as string), + 'input-link': (field, value) => prepareString(field, value as string), + 'input-phone': (field, value) => prepareString(field, value as string)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/src/components/FormsEngine/lib/valueSerializers.ts` around lines 96 - 98, The new text-like controls 'input-email', 'input-link', and 'input-phone' were left as passthrough (undefined) so they bypass the escape-aware path; update their entries in the serializer map to use the same serializer as 'text-field' (i.e., the prepareString/ text-field serializer) so they honor escapeContent and follow the same serialization path as other text inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ui/app/src/components/FormsEngine/lib/validators.ts`:
- Around line 396-440: The three nearly identical wrappers inputEmailValidator,
inputLinkValidator, and inputPhoneValidator should be consolidated with a small
factory to avoid duplication: create a helper (e.g., makePatternValidator) that
accepts a pattern message (defineMessage(...)) and returns a validator function
compatible with inputValidator, then replace inputEmailValidator,
inputLinkValidator, and inputPhoneValidator by calling that factory with their
respective messages; ensure the factory returns the same signature (field,
currentValue, messages) and internally calls inputValidator so existing usage
sites need no changes.
In `@ui/app/src/components/FormsEngine/lib/valueSerializers.ts`:
- Around line 96-98: The new text-like controls 'input-email', 'input-link', and
'input-phone' were left as passthrough (undefined) so they bypass the
escape-aware path; update their entries in the serializer map to use the same
serializer as 'text-field' (i.e., the prepareString/ text-field serializer) so
they honor escapeContent and follow the same serialization path as other text
inputs.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ui/app/src/components/ContentTypeManagement/descriptors/controls/inputLink.tsui/app/src/components/FormsEngine/lib/validators.tsui/app/src/components/FormsEngine/lib/valueRetrievers.tsui/app/src/components/FormsEngine/lib/valueSerializers.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- ui/app/src/components/ContentTypeManagement/descriptors/controls/inputLink.ts
rart
approved these changes
Mar 16, 2026
Member
rart
left a comment
rart
left a comment
There was a problem hiding this comment.
@russdanner have we considered having a single input with a type instead of a different control per type? Could help with keeping the control list cleaner
Member
|
@russdanner please see above @jvega190 please fix conflicts |
Member
|
@rart These are pretty thin lightweight wrappers. |
…o feature/7418-input-controls # Conflicts: # ui/app/src/components/ContentTypeManagement/descriptors/controls/index.ts # ui/app/src/components/FormsEngine/lib/controlMap.ts # ui/app/src/components/FormsEngine/lib/validators.ts
Member
Author
|
@sumerjabri conflicts fixed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
craftercms/craftercms#7418
Summary by CodeRabbit