-
Notifications
You must be signed in to change notification settings - Fork 91
gpld-populate-new-minimum-date-into-linked-date-field.js
: Fixed an issue with default value of field notgetting used to apply minimum date.
#1099
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…issue with default value of field notgetting used to apply minimum date.
WalkthroughA constant for the source field ID was introduced, and event listeners for the Changes
Suggested reviewers
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
gp-limit-dates/gpld-populate-new-minimum-date-into-linked-date-field.js (1)
13-13
: Hardcoded sourceFieldId may limit reusabilityThe introduction of a constant for the source field ID is good practice, but hardcoding it to 25 may limit the snippet's reusability across different forms. Consider adding a comment suggesting users replace this value with their actual field ID.
-const sourceFieldId = 25; // Replace with the ID of the source field (Field A) +const sourceFieldId = 25; // IMPORTANT: Replace with the ID of your source field (Field A)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gp-limit-dates/gpld-populate-new-minimum-date-into-linked-date-field.js
(1 hunks)
🔇 Additional comments (2)
gp-limit-dates/gpld-populate-new-minimum-date-into-linked-date-field.js (2)
14-22
: Good implementation of post-render event handlerThis new event listener effectively addresses the core issue by triggering input and change events when a default value is present. The use of
requestAnimationFrame
is a good practice to ensure the events fire after the browser has completed rendering.
24-26
: LGTM: Existing callback properly handles date settingThe existing callback effectively sets the datepicker's date when the minimum date is updated. This works well with the new code that triggers the necessary events when the form has default values.
gp-limit-dates/gpld-populate-new-minimum-date-into-linked-date-field.js
Outdated
Show resolved
Hide resolved
const value = $field.val(); | ||
if ( value ) { | ||
requestAnimationFrame( function(){ | ||
$field.trigger( 'input' ).trigger( 'change' ); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for field selection
The code assumes the field will always be found, but this may not be true if the field ID is incorrect or the field is conditionally hidden. Consider adding error handling.
const $field = jQuery( `#input_GFFORMID_${sourceFieldId}` );
+ if ($field.length === 0) {
+ console.error(`Source field with ID ${sourceFieldId} not found. Please check the field ID.`);
+ return;
+ }
const value = $field.val();
if ( value ) {
requestAnimationFrame( function(){
$field.trigger( 'input' ).trigger( 'change' );
});
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const value = $field.val(); | |
if ( value ) { | |
requestAnimationFrame( function(){ | |
$field.trigger( 'input' ).trigger( 'change' ); | |
}); | |
} | |
const $field = jQuery( `#input_GFFORMID_${sourceFieldId}` ); | |
if ($field.length === 0) { | |
console.error(`Source field with ID ${sourceFieldId} not found. Please check the field ID.`); | |
return; | |
} | |
const value = $field.val(); | |
if ( value ) { | |
requestAnimationFrame( function(){ | |
$field.trigger( 'input' ).trigger( 'change' ); | |
}); | |
} |
…issue with default value of field notgetting used to apply minimum date.
…issue with default value of field notgetting used to apply minimum date.
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/2929104779/83029
Summary
The snippet above doesn't work on initial load if the Date field A has a default date value.
Loom of the issue and update:
https://www.loom.com/share/763dfa9262a44ead841b3fdb2818fac4