gpnf-hide-empty-columns.php: Fixed an issue withsnippet not working for fields having data stored on sub field IDs like Name, Address, etc.
#1166
Conversation
… for fields having data stored on sub field IDs like Name, Address, etc.
WalkthroughRevised gpnf_hec_column_has_value() to scan each entry’s key/value pairs with a regex matching the base field ID and optional subfield suffix (.\d+), returning true on first non-blank match. This enables detecting values in multi-input subfields. Function signature and JS behavior remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Table Renderer
participant PHP as GPNF PHP
participant Func as gpnf_hec_column_has_value()
participant Store as Entries
UI->>PHP: Determine if column is empty
PHP->>Func: gpnf_hec_column_has_value(field_id, entries)
Func->>Store: Iterate entries
loop For each entry
Func->>Func: Regex match keys: ^field_id(\.\d+)?
alt Any matched key has non-blank value
Func-->>PHP: return true
PHP-->>UI: Column has value
end
end
Func-->>PHP: return false
PHP-->>UI: Column empty
Note over Func: Changed: key-level scan with regex to include subfields
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gp-nested-forms/gpnf-hide-empty-columns.php (1)
151-159: Fix client-side parity: scan sub‑input IDs in gpnfHecColumnHasValue.Client JS gpnfHecColumnHasValue only checks entry[fieldId] and entry[
f${fieldId}], ignoring sub‑input keys like "8.3", while the PHP/simple‑detail path uses gpnf_hec_column_has_value — this creates a parity gap that can leave columns hidden. (gravitywiz.com)Apply this diff (mirrors PHP logic):
<script> function gpnfHecColumnHasValue( fieldId, entries ) { - var hasValue = false; - jQuery.each( entries, function( i, entry ) { - if( ( entry[ fieldId ] && entry[ fieldId ].label ) || ( entry[ `f${fieldId}` ] && entry[ `f${fieldId}` ].label ) ) { - hasValue = true; - } - } ); - return hasValue; + var id = String(fieldId); + var esc = id.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + var re = new RegExp('^' + esc + '(\\.\\d+)?$'); + return entries.some(function(entry) { + // Aggregated label helpers first. + var f = entry['f' + id]; + if (f && f.label && String(f.label).trim() !== '') return true; + var base = entry[id]; + if (base && base.label && String(base.label).trim() !== '') return true; + // Scan sub‑inputs. + for (var k in entry) { + if (!Object.prototype.hasOwnProperty.call(entry, k)) continue; + if (!re.test(String(k))) continue; + var v = entry[k]; + var label = (v && typeof v === 'object' && 'label' in v) ? v.label : v; + if (label != null && String(label).trim() !== '') return true; + } + return false; + }); } </script>Quick tests to run:
- Add a Name field; submit only First Name — confirm column visible in default template.
- Add an Address field; submit only Address Line 1 — confirm column visible in default template.
- Repeat both checks in the simple detail template and confirm parity.
🧹 Nitpick comments (2)
gp-nested-forms/gpnf-hide-empty-columns.php (2)
166-172: Nit: precompute the regex once and tighten the comment.Avoid rebuilding the pattern per key and fix the stray parenthesis in the comment.
Apply this diff:
function gpnf_hec_column_has_value( $field_id, $entries ) { foreach ( $entries as $entry ) { - foreach ( $entry as $key => $value ) { - // Match $field_id or subfield IDs (For fields like Name, Address, etc).) - if ( preg_match( '/^' . preg_quote( $field_id, '/' ) . '(\.\d+)?$/', $key ) ) { + // Build once per entry (field_id is constant for this call). + $pattern = '/^' . preg_quote( (string) $field_id, '/' ) . '(?:\.\d+)?$/'; + foreach ( $entry as $key => $value ) { + // Match $field_id or sub‑input IDs (fields like Name, Address, etc.). + if ( preg_match( $pattern, (string) $key ) ) { if ( ! rgblank( $value ) ) { return true; } } } } return false; }
104-116: Avoid recomputing visibility per row (minor perf/clarity).gpnf_hec_column_has_value() is called repeatedly for each row. Compute once per field and reuse.
Example minimal change:
<?php // before rendering the table header + $visible = array(); + foreach ( $nested_fields as $nf ) { + $visible[ $nf->id ] = gpnf_hec_column_has_value( $nf->id, $entries ); + } ?> ... - foreach ( $nested_fields as $nested_field ) : - if ( ! gpnf_hec_column_has_value( $nested_field->id, $entries ) ) : + foreach ( $nested_fields as $nested_field ) : + if ( empty( $visible[ $nested_field->id ] ) ) : continue; endif; ... - foreach ( $nested_fields as $nested_field ) : - if ( ! gpnf_hec_column_has_value( $nested_field->id, $entries ) ) : + foreach ( $nested_fields as $nested_field ) : + if ( empty( $visible[ $nested_field->id ] ) ) : continue; endif;Also applies to: 124-136
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gp-nested-forms/gpnf-hide-empty-columns.php(1 hunks)
🔇 Additional comments (2)
gp-nested-forms/gpnf-hide-empty-columns.php (2)
166-172: Good fix: now correctly detects values on sub‑input IDs.Anchored regex + preg_quote is safe and solves the Name/Address gaps server‑side. Looks good.
Please sanity‑check with entries where only a single sub‑input is filled (e.g., First Name only or Address Line 1 only) to ensure the column shows for:
- nested-entries-detail-simple (PHP check in this file)
- the default nested-entries template (client JS; see separate comment)
169-170: Whitespace‑only and array values (double‑check desired behavior).rgblank() may treat "0" and whitespace differently than expected. If a sub‑input value is " " or an array of mostly blanks (edge cases), columns may show/hide unexpectedly.
If you want to ignore whitespace‑only strings and require at least one non‑blank element in arrays, consider:
$val = $value; if ( is_string( $val ) ) { $val = trim( $val ); } if ( is_array( $val ) ) { $val = array_filter( $val, function( $v ) { return ! rgblank( is_string( $v ) ? trim( $v ) : $v ); } ); if ( ! empty( $val ) ) { return true; } } elseif ( ! rgblank( $val ) ) { return true; }Please confirm whether this stricter behavior aligns with product expectations before changing.
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/3069272683/89066
Summary
Updated
gpnf_hec_column_has_value()to correctly detect values stored on sub-inputs (e.g.8.3,8.6) in addition to the actual field ID (e.g.8).Fields like Name or Address, which store values across multiple sub-input IDs, were not being detected.