Fix: Hidden CSS class removes fields from builder #1752
Conversation
WalkthroughAdds utilities and templates to detect and strip frontend-hiding CSS classes in the form builder, shows a "Hidden on frontend" badge for fields that would be hidden, and introduces frontend CSS/LESS utility classes to hide elements. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
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 |
![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 (2)
assets/js-templates/form-components.php (1)
110-140: Builder now filters hide classes and shows a badge – consider DRY and i18n
- Using
filter_builder_css_classes(field.css)in the:classarray is the right way to prevent Tailwind/Bootstrap hide classes from removing the field from the builder preview.- The
has_hidden_css_class(field.css)-based badge clearly surfaces “Hidden on frontend” state without affecting layout.Two minor follow-ups:
- The badge text
"Hidden on frontend"andtitletooltip are hardcoded English; they should go through translation helpers (e.g. localized string injected into JS) to keep builder UI fully translatable.- Column-field inner items in
#tmpl-wpuf-form-column_fieldstill use rawfield.css(lines 951–956) and won’t benefit from filtering or the badge; for consistency with top-level fields, consider applyingfilter_builder_css_classesand optionally the same indicator there as well.admin/form-builder/assets/js/components/builder-stage-v4-1/template.php (1)
17-50: Admin builder template changes are sound but mirror same i18n/consistency concerns
- Switching to
filter_builder_css_classes(field.css)in the main:classbinding avoids hidden CSS classes removing fields from the v4.1 builder stage, which directly addresses the reported bug.- The
has_hidden_css_class(field.css)badge inside the label is a good, non-intrusive affordance to explain why a field won’t appear on the frontend.Same minor concerns as in the JS template:
"Hidden on frontend"and the tooltip string should be localized via translatable strings rather than hardcoded.- Inner column-field templates (in the shared templates file) still rely on raw
field.css; if users apply hide classes there, those fields may still disappear in the builder. Consider aligning behavior by using the same helper(s) in column contexts.
🧹 Nitpick comments (2)
admin/form-builder/assets/js/components/builder-stage-v4-1/index.js (1)
192-247: Helper methods correctly filter/hint on hide classes; consider sharing the logic
filter_builder_css_classes(cssClasses)correctly:
- Guards non-string/empty input.
- Normalizes via
toLowerCase()before matching.- Strips known hide-related classes (
hidden,d-none,hide,invisible,opacity-0,sr-only,visually-hidden) while returning the remaining classes as a space-separated string, which works fine in Vue’s:classarray binding.has_hidden_css_class(cssClasses)is safe and cheap, and only flags classes that fully hide elements (hidden,d-none,hide,invisible), which matches the “Hidden on frontend” badge semantics.Two small points:
- The “forbidden” list here includes
opacity-0,sr-only, andvisually-hidden, buthas_hidden_css_classdoes not. If you intend the badge to represent “visually not present to typical users”, you may want to include at leastopacity-0and possibly the visually-hidden variants for consistency.- The same helpers are implemented verbatim in
assets/js/wpuf-form-builder-components.js; extracting them into a shared mixin/utility would avoid divergence over time.Functionally this is correct and safe for the current PR.
assets/js/wpuf-form-builder-components.js (1)
379-434: Duplicated helper implementations – works now, but centralization would helpThis copy of
filter_builder_css_classes/has_hidden_css_classmatches the implementation inbuilder-stage-v4-1/index.js, so behavior is consistent between the inline Vue registration and the admin component.To reduce maintenance risk:
- Consider moving these helpers into a shared mixin (e.g.
wpuf_mixins.builder_css_utils) or a small utility module, and reuse from both places, so any future change to the list of hide classes only needs to be done once.- As noted in the other file, you may want to align
hiddenClasseswith the full set of classes removed byfilter_builder_css_classesif the badge should cover all visually-hidden scenarios.No blocking issues; the logic itself is correct for the current objectives.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
admin/form-builder/assets/js/components/builder-stage-v4-1/index.js(1 hunks)admin/form-builder/assets/js/components/builder-stage-v4-1/template.php(3 hunks)assets/css/frontend-forms.css(1 hunks)assets/js-templates/form-components.php(3 hunks)assets/js/wpuf-form-builder-components.js(1 hunks)assets/less/frontend-forms.less(1 hunks)
🔇 Additional comments (2)
assets/css/frontend-forms.css (1)
1797-1817: Hide utility classes look correct and well-scopedThe new
.wpuf-form .hidden/.d-none/.hide/.invisible/.sr-only/.visually-hiddenrules are consistent with common utility semantics, scoped to.wpuf-form, and use!importantappropriately so frontend behavior is predictable. No issues from a CSS or UX perspective.assets/less/frontend-forms.less (1)
2089-2114: LESS source matches compiled CSS and is structured correctlyThe LESS block for hide utilities under
.wpuf-form { ... }matches the compiled CSS behavior and keeps the utilities neatly scoped. This will compile to the expected selectors and is safe to maintain.
Extended CSS and LESS selectors to support multiple case variations (e.g., .Hidden, .HIDDEN, .D-none, .Hide, .HIDE, .Invisible, .INVISIBLE) for hiding and making fields invisible on the frontend forms. This improves compatibility with different class naming conventions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
assets/less/frontend-forms.less (1)
2089-2122: Clarify the intent behind multiple case variants of each utility class.The added utilities define three-variants of each hiding class (e.g.,
.hidden,.Hidden,.HIDDENfor display: none). While CSS is case-sensitive and these are technically distinct selectors, this pattern is non-standard and uncommon in modern web development, which typically uses lowercase class names.Is this intentional to support existing codebases with mixed casing conventions, or should this be simplified to lowercase variants only (
.hidden,.d-none,.hide, etc.)? Having multiple case variants increases maintenance burden without clear benefit.Please clarify:
- Is support for mixed case class names a requirement?
- Are there existing forms in the wild using
.Hiddenor.HIDDENthat must be preserved?- If not, consider simplifying to lowercase-only variants to align with CSS conventions and reduce duplication.
Approved aspects:
- Screen-reader utilities (
.sr-only,.visually-hidden) are correctly implemented per accessibility standards.- Scoping within
.wpuf-formis appropriate.- Use of
!importantis justified for utility classes.If mixed casing is not required, simplify to:
- /* CSS utility classes for hiding fields on frontend */ - .wpuf-form { - .hidden, - .Hidden, - .HIDDEN, - .d-none, - .D-none, - .D-None, - .hide, - .Hide, - .HIDE { + /* CSS utility classes for hiding fields on frontend */ + .wpuf-form { + .hidden, + .d-none, + .hide { display: none !important; } - - .invisible, - .Invisible, - .INVISIBLE { + .invisible { visibility: hidden !important; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
assets/css/frontend-forms.css(1 hunks)assets/less/frontend-forms.less(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- assets/css/frontend-forms.css
sapayth
added
needs: author reply
Centralizes the list of hidden CSS classes in a new hiddenClasses() method for the builder-stage-v4-1 Vue component, improving maintainability. Updates Field_Manager.php to add a space before 'wpuf_hidden_field' when setting the CSS class for hidden fields.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
includes/Admin/Forms/Field_Manager.php (1)
256-259: Leading space fix is fine; consider normalizing thecssvalue to avoid noticesAdding the leading space makes concatenation safer when
cssis already populated. However, if$form_field['css']is unset or non-string, this can still emit notices or produce odd spacing. You could defensively normalize it here:- if ( $visibility_selected == 'hidden' ) { - $form_field['css'] .= ' wpuf_hidden_field'; - $show_field = true; - } + if ( $visibility_selected == 'hidden' ) { + $existing_css = isset( $form_field['css'] ) ? trim( (string) $form_field['css'] ) : ''; + $form_field['css'] = $existing_css === '' ? 'wpuf_hidden_field' : $existing_css . ' wpuf_hidden_field'; + $show_field = true; + }This keeps the behavior while avoiding undefined-index notices and normalizing whitespace.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
admin/form-builder/assets/js/components/builder-stage-v4-1/index.js(1 hunks)assets/js/wpuf-form-builder-components.js(1 hunks)includes/Admin/Forms/Field_Manager.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- admin/form-builder/assets/js/components/builder-stage-v4-1/index.js
- assets/js/wpuf-form-builder-components.js
arifulhoque7
added
needs: dev review
|
Review added @sapayth vai |
3 participants
Fix: Hidden CSS class removes fields from builder
Close issue Close issue, Related PRO PR
Files modified:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.