Support JS validation for confirmation fields targetting phone fields#2545
Conversation
WalkthroughA conditional was added in js/formidable.js within validateFieldValue to invoke confirmField for 'tel' fields when shouldCheckConfirmField(field, onSubmit) returns true. This extends the confirmation-validation flow to telephone inputs, affecting control flow and error handling for those fields only. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant F as Form (validateFieldValue)
participant C as shouldCheckConfirmField
participant CF as confirmField
participant E as errors
U->>F: Input value for tel field
F->>C: shouldCheckConfirmField(field, onSubmit)?
C-->>F: true/false
alt tel field AND check required
F->>CF: confirmField(field, errors)
CF-->>E: Append confirmation errors (if any)
else Not applicable
F-->>F: Continue existing validation
end
F-->>U: Validation result (with/without errors)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
js/formidable.js (1)
350-352: LGTM! Confirmation validation for tel fields implemented correctly.The implementation properly extends confirmation field validation to telephone inputs. The logic correctly:
- Uses
shouldCheckConfirmFieldto determine when validation should run (on submit or when editing the confirmation field itself)- Delegates to the existing
confirmFieldfunction for consistent error handling- Works alongside pattern validation for tel fields (if a pattern is set)
Optional refinement for structural consistency: Consider creating a dedicated
checkTelFieldfunction (similar tocheckEmailFieldandcheckPasswordField) to encapsulate tel-specific validation logic. This would improve consistency with how email and password fields handle confirmation:function checkTelField( field, errors, onSubmit ) { if ( shouldCheckConfirmField( field, onSubmit ) ) { confirmField( field, errors ); } }Then update the conditional chain around line 346:
} else if ( field.pattern !== null ) { checkPatternField( field, errors ); +} else if ( field.type === 'tel' ) { + checkTelField( field, errors, onSubmit ); } -if ( 'tel' === field.type && shouldCheckConfirmField( field, onSubmit ) ) { - confirmField( field, errors ); -}However, the current implementation works correctly and is perfectly acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
js/formidable.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
js/formidable.js (1)
stripe/js/frmstrp.js (1)
errors(233-233)
⏰ 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). (8)
- GitHub Check: Run PHP Syntax inspection (8.3)
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: Run PHP Syntax inspection (8.3)
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
Related PR https://github.com/Strategy11/formidable-pro/pull/6041