-
Notifications
You must be signed in to change notification settings - Fork 154
enhance: field icon settings conditional options #1779
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: develop
Are you sure you want to change the base?
enhance: field icon settings conditional options #1779
Conversation
WalkthroughThe pull request conditionally renders icon-selector and select field templates based on Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ 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 |
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: 0
🧹 Nitpick comments (1)
includes/Admin/Forms/Admin_Form_Builder.php (1)
124-147: Confirmsignature_fieldshould be enforced as single-instance and slug matches field typeAdding
'signature_field'towpuf_single_form_fieldwill prevent more than one signature field being added to a form. That looks reasonable, but it is a behavior change:
- Confirm that design/UX really requires at most one signature field per form (no valid use cases for multiple signatures).
- Double‑check that the slug
'signature_field'exactly matches the identifier used by the field definition (e.g.,template,input_type, orname) so the uniqueness logic actually applies to the correct field.If both are true, this change is good to go.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
admin/form-builder/assets/js/components/field-icon_selector/template.phpadmin/form-builder/assets/js/components/field-select/template.phpassets/js-templates/form-components.phpincludes/Admin/Forms/Admin_Form_Builder.phplanguages/wp-user-frontend.potreadme.md
🧰 Additional context used
🪛 GitHub Actions: Inspections
admin/form-builder/assets/js/components/field-icon_selector/template.php
[error] 1-1: Overriding WordPress globals is prohibited. Found assignment to $post
🪛 LanguageTool
readme.md
[grammar] ~24-~24: Use a hyphen to join words.
Context: ...nt, and the AI generates a complete post submission form with smart fields, label...
(QB_NEW_EN_HYPHEN)
[grammar] ~30-~30: Use a hyphen to join words.
Context: ...s without any manual steps. One Time Payments Offer simple one...
(QB_NEW_EN_HYPHEN)
[grammar] ~218-~218: Use a hyphen to join words.
Context: ...mail marketing tool for newsletters, opt in forms, and audience growth. * [wePOS]...
(QB_NEW_EN_HYPHEN)
[grammar] ~219-~219: Use a hyphen to join words.
Context: ...ooCommerce point of sale solution for in store sales and real time inventory sync...
(QB_NEW_EN_HYPHEN)
[grammar] ~219-~219: Use a hyphen to join words.
Context: ...ale solution for in store sales and real time inventory sync. * [WP Project Manag...
(QB_NEW_EN_HYPHEN)
[grammar] ~221-~221: Use a hyphen to join words.
Context: ...Readme): Elementor enhancements with 500 plus templates, advanced widgets, Theme ...
(QB_NEW_EN_HYPHEN)
[grammar] ~342-~342: Use a hyphen to join words.
Context: ... Forms in Gutenberg Editor 15. Create AI Powered Post Form ## Changelog ## ### ...
(QB_NEW_EN_HYPHEN)
[grammar] ~527-~527: Use a hyphen to join words.
Context: ...AJAX image upload * Fix – Error on multi select form custom taxonomy * Fix – Fron...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
readme.md
3-3: Bare URL used
(MD034, no-bare-urls)
10-10: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (7)
languages/wp-user-frontend.pot (1)
7-7: POT header and reference updates look consistentThe updated
POT-Creation-Dateand the adjusted source references forincludes/Admin/Forms/Admin_Form_Builder.php(and related admin templates) are all consistent with the PHP line shifts. No msgids changed, so there’s no i18n or compatibility impact here.Also applies to: 451-455, 498-507, 514-523, 1121-1125, 2915-2976
admin/form-builder/assets/js/components/field-icon_selector/template.php (2)
1-67: Note: Pipeline failure appears unrelated to this change.The pipeline reports "Overriding WordPress globals is prohibited. Found assignment to $post", but no
$postvariable assignment is present in this template file. This error may be a false positive from the scanning tool or originate from a different context.
1-1: Themet_dependenciesimplementation is properly defined and working correctly.The computed property exists in
admin/form-builder/assets/js/mixins/option-field.jsand correctly evaluates field dependencies. It returnstruewhen dependencies are met or absent, andfalsewhen any dependency fails validation. The conditional rendering in the template is correct.admin/form-builder/assets/js/components/field-select/template.php (1)
1-1: LGTM! Consistent conditional rendering.This change follows the same pattern as the icon selector template, conditionally rendering the select field panel based on
met_dependencies. The implementation is consistent across components.assets/js-templates/form-components.php (3)
295-295: LGTM! Consistent template implementation.The conditional rendering for the icon selector template matches the component-level implementation. This ensures consistent behavior across the form builder.
631-631: LGTM! Template change is consistent.The conditional rendering for the select field template matches the component-level implementation.
295-295: Clarify the relationship between PR description and linked issue.The PR description states it "closes issue #1334", which requests:
- Display required asterisk for password fields
- Place eye icon inside password input field
However, this PR implements conditional display of icon-related settings in the form builder, which appears unrelated to the password field improvements described in #1334. Please verify that the correct issue is linked, or clarify how these changes address the password field requirements.
Also applies to: 631-631
|
Enough spaces not added @sapayth vai |
those tweaks are mentioned in separate issue and fixed in this PR @Rubaiyat-E-Mohammad bhai |


closes #1334
This PR makes the form builder's icon-related fields only appear when icons are enabled. When the "Show Icon" option is set to "No", the "Field Icon" and "Icon Position" fields now hide automatically, reducing clutter and improving the form building experience.
Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.