gpalf-count-only-non-blank-rows.js: Added support to calculate only non-blank rows in a list field.
#1085
Conversation
… non-blank rows in a list field.
WalkthroughA new JavaScript file was introduced to extend Gravity Forms functionality by customizing how the ":count" merge tag operates for list fields. The script hooks into the Gravity Forms filter for merge tag value calculation and, when a merge tag with the ":count" suffix is encountered, it counts only those rows in a list field that contain at least one non-empty value. If the merge tag or field does not match the expected pattern, the value is returned unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GravityForms
participant CustomScript
User->>GravityForms: Submits form with a list field
GravityForms->>CustomScript: Calls filter for merge tag value with ":count"
CustomScript->>GravityForms: Returns count of non-blank rows in the list field
GravityForms->>User: Displays or processes the calculated count
Suggested reviewers
✨ 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:
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: 1
🧹 Nitpick comments (5)
gp-auto-list-field/gpalf-count-only-non-blank-rows.js (5)
1-11: Documentation looks good but could be improved.The header comment clearly explains the purpose of the snippet, which matches the PR objectives of counting only non-blank rows in a List field. However, the title "Dynamic Row Labels for List Fields" doesn't accurately reflect the functionality being added, which is about counting non-blank rows.
-/** - * Gravity Perks // Auto List Field // Dynamic Row Labels for List Fields +/** + * Gravity Perks // Auto List Field // Count Only Non-Blank Rows * https://gravitywiz.com/documentation/gravity-forms-auto-list-field/ * * Count only non-blank rows in a List field. * * Instructions: * * 1. Install this snippet with our free Custom JavaScript plugin. * https://gravitywiz.com/gravity-forms-code-chest/ */
12-15: Consider adding jQuery dependency check.The code uses jQuery but doesn't verify its availability. It's also missing comments explaining what the filter does and the structure of the
matchparameter.+/** + * Filter the merge tag value before calculation. + * Modifies the behavior of :count modifier to only count non-blank rows in a list field. + * + * @param {string|number} value - The current value of the merge tag + * @param {Array} match - The regex match array from the merge tag pattern + * @param {boolean} isVisible - Whether the field is visible + * @param {object} formulaField - The field object containing the formula + * @param {number} formId - The ID of the current form + * @return {string|number} - The modified value or original value if conditions aren't met + */ gform.addFilter('gform_merge_tag_value_pre_calculation', function (value, match, isVisible, formulaField, formId) { + // Ensure jQuery is available + if (typeof jQuery === 'undefined') { + console.error('jQuery is required for the non-blank rows counter to work.'); + return value; + } + + // Check if this is a :count merge tag if (typeof match[3] === 'undefined' || match[3].indexOf(':count') === -1) { return value; }
17-23: Add error handling for invalid field ID.The code is well-structured but lacks error handling for when the inputId doesn't convert to a valid numeric fieldId.
var inputId = match[1]; var fieldId = parseInt(inputId, 10); + + // Check if the parsed fieldId is a valid number + if (isNaN(fieldId)) { + console.warn('Invalid field ID in merge tag:', inputId); + return value; + } var $fieldWrapper = $(`#gform_${formId} #field_${formId}_${fieldId}`); if ($fieldWrapper.length === 0) { + console.debug(`Field wrapper not found for field ID ${fieldId} in form ${formId}`); return value; }
25-28: Look for all types of form controls, not just inputs.The code currently only checks for list groups but doesn't validate if this is actually a list field before proceeding.
var $rows = $fieldWrapper.find('.gfield_list_group'); if ($rows.length === 0) { + // Check if this is actually a list field + if (!$fieldWrapper.hasClass('gfield_contains_list')) { + console.debug(`Field ${fieldId} in form ${formId} is not a list field`); + } else { + console.debug(`No list rows found for field ${fieldId} in form ${formId}`); + } return value; }
47-48: Function return value lacks type coercion.The return value should match the expected type of the original value for consistency. The merge tag system likely expects a string, but this returns a number.
- return nonBlankCount; + // Convert to string to match the expected return type of merge tags + return String(nonBlankCount); });
| var nonBlankCount = 0; | ||
|
|
||
| $rows.each(function () { | ||
| var isNonBlank = false; | ||
|
|
||
| $(this).find('input').each(function () { | ||
| if ($(this).val().trim() !== '') { | ||
| isNonBlank = true; | ||
| return false; | ||
| } | ||
| }); | ||
|
|
||
| if (isNonBlank) { | ||
| nonBlankCount++; | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve non-blank row detection to include all form controls.
The current implementation only checks for input elements, but list fields might contain other form controls like select, textarea, etc. Additionally, the code uses jQuery's .each() with a return false to break the inner loop, which is correct but could be documented for clarity.
var nonBlankCount = 0;
$rows.each(function () {
var isNonBlank = false;
- $(this).find('input').each(function () {
+ // Check all possible form controls (inputs, selects, textareas)
+ $(this).find('input, select, textarea').each(function () {
if ($(this).val().trim() !== '') {
isNonBlank = true;
+ // Break the .each() loop early since we found a non-blank value
return false;
}
});
if (isNonBlank) {
nonBlankCount++;
}
});📝 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.
| var nonBlankCount = 0; | |
| $rows.each(function () { | |
| var isNonBlank = false; | |
| $(this).find('input').each(function () { | |
| if ($(this).val().trim() !== '') { | |
| isNonBlank = true; | |
| return false; | |
| } | |
| }); | |
| if (isNonBlank) { | |
| nonBlankCount++; | |
| } | |
| }); | |
| var nonBlankCount = 0; | |
| $rows.each(function () { | |
| var isNonBlank = false; | |
| // Check all possible form controls (inputs, selects, textareas) | |
| $(this).find('input, select, textarea').each(function () { | |
| if ($(this).val().trim() !== '') { | |
| isNonBlank = true; | |
| // Break the .each() loop early since we found a non-blank value | |
| return false; | |
| } | |
| }); | |
| if (isNonBlank) { | |
| nonBlankCount++; | |
| } | |
| }); |
saifsultanc
left a comment
saifsultanc
left a comment
There was a problem hiding this comment.
Tests look good. LGTM. S&M!
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/2916143012/82450
Summary
By default, GP Auto List Field calculates the all rows (including blank rows) when we use
:countmodifier. This snippet is useful when you want to count only non-blank rows.