Add validation for phone number and PAN card on contribution form#446
Add validation for phone number and PAN card on contribution form#446tarunnjoshi wants to merge 1 commit intodevelopfrom
Conversation
WalkthroughThe changes in Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (2)
wp-content/themes/goonj-crm/main.js (2)
Line range hint
6-46: Refactor message handling to reduce duplication and improve maintainabilityThe current implementation contains repeated HTML templates and styling. Consider refactoring to:
- Extract messages into a configuration object
- Create a reusable template rendering function
- Consolidate common styles
Here's a suggested improvement:
const MESSAGE_TEMPLATES = { 'not-inducted-volunteer': { title: 'You are not registered as a volunteer with us.', message: 'To set up a collection camp, please take a moment to fill out the volunteer registration form below.' }, 'dropping-center': { title: 'You are not registered as a volunteer with us.', message: 'To set up a dropping center, please take a moment to fill out the volunteer registration form below.' }, // ... other messages }; function renderMessage(type) { const template = MESSAGE_TEMPLATES[type]; return ` <div class="w-520 mt-30"> <p class="fw-600 font-sans fz-20 mb-6">${template.title}</p> <p class="fw-400 font-sans fz-16 mt-0 mb-24">${template.message}</p> </div> `; }
71-82: Consider adding more robust validation patternsThe current regex patterns might be too strict:
- Phone number pattern doesn't account for country codes or different formats
- PAN card validation is case-sensitive
Consider updating the validation patterns:
const fields = [ { labelText: "Mobile Number", regex: /^(\+\d{1,3}[- ]?)?\d{10}$/, errorMessage: "Please enter a valid mobile number. Include country code if applicable.", }, { labelText: "Enter PAN Card Number", regex: /^[A-Za-z]{5}[0-9]{4}[A-Za-z]$/i, errorMessage: "Please enter a valid PAN card number (e.g., ABCDE1234F).", } ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
wp-content/themes/goonj-crm/main.js(2 hunks)
🔇 Additional comments (1)
wp-content/themes/goonj-crm/main.js (1)
67-106: Ensure server-side validation is implemented
While client-side validation improves user experience, it can be bypassed. Ensure that robust server-side validation is also implemented for these fields to maintain data integrity and security.
| setTimeout(function () { | ||
| var resetButton = document.querySelector('button[type="reset"]'); | ||
|
|
||
| if (resetButton) { | ||
| resetButton.addEventListener("click", function (event) { | ||
| event.preventDefault(); | ||
|
|
||
| // Refresh the page to reset all fields | ||
| location.reload(true); | ||
| }); | ||
| } | ||
| }, 1000); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace page reload with proper form reset implementation
The current implementation uses a page reload as a temporary solution, which is inefficient and provides a poor user experience.
Consider implementing a proper form reset:
document.addEventListener("DOMContentLoaded", function () {
const form = document.querySelector('form');
const resetButton = form?.querySelector('button[type="reset"]');
if (resetButton && form) {
resetButton.addEventListener("click", function (event) {
event.preventDefault();
form.reset();
// Reset any custom form state here
const inputs = form.querySelectorAll('input, select, textarea');
inputs.forEach(input => input.classList.remove('error', 'valid'));
});
}
});Also, the setTimeout seems unnecessary. Consider removing it unless there's a specific reason for the delay.
| fields.forEach(field => { | ||
| // Select the label by labelText and find the associated input | ||
| const label = Array.from(document.querySelectorAll("label")).find(el => el.textContent.includes(field.labelText)); | ||
| if (label) { | ||
| const input = document.querySelector(`input[name="${label.getAttribute("for")}"]`); | ||
|
|
||
| if (resetButton) { | ||
| resetButton.addEventListener("click", function (event) { | ||
| event.preventDefault(); | ||
| if (input) { | ||
| const form = input.closest("form"); | ||
|
|
||
| // Refresh the page to reset all fields | ||
| location.reload(true); | ||
| }); | ||
| } | ||
| }, 1000); | ||
| // Attach validation on form submission | ||
| if (form) { | ||
| form.addEventListener("submit", function (event) { | ||
| const value = input.value; | ||
| if (!field.regex.test(value)) { | ||
| event.preventDefault(); | ||
| alert(field.errorMessage); | ||
| input.focus(); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve form validation UX and error handling
Current implementation has several areas for improvement:
- Relies on label text for field identification (brittle)
- Uses alert() for error messages (poor UX)
- No real-time validation
- No visual feedback on input fields
Consider implementing:
document.addEventListener("DOMContentLoaded", function () {
const form = document.querySelector('form');
fields.forEach(field => {
const input = form?.querySelector(`[data-validation="${field.type}"]`);
if (!input) return;
// Add error container
const errorSpan = document.createElement('span');
errorSpan.className = 'error-message';
input.parentNode.appendChild(errorSpan);
// Real-time validation
input.addEventListener('input', validateField);
function validateField() {
const isValid = field.regex.test(input.value);
input.classList.toggle('error', !isValid);
input.classList.toggle('valid', isValid);
errorSpan.textContent = isValid ? '' : field.errorMessage;
}
});
form?.addEventListener('submit', function(event) {
const hasErrors = fields.some(field => {
const input = form.querySelector(`[data-validation="${field.type}"]`);
return input && !field.regex.test(input.value);
});
if (hasErrors) {
event.preventDefault();
}
});
});Also add corresponding CSS:
.error { border-color: red; }
.valid { border-color: green; }
.error-message {
color: red;
font-size: 12px;
display: block;
margin-top: 5px;
}
|
Close as not needed |
1 participant
Add validation for phone number and PAN card on the contribution form
Summary by CodeRabbit
New Features
Improvements