Add new upsells in Lite for missing Pro features #2570
Conversation
WalkthroughAdds Pro upsell gating for several field options: controller computes upsell flags when Pro is absent; a new helper builds upsell-aware input attributes; settings view includes upsell partials (Autocomplete, Visibility, Before/After, AI button) and applies a dimmed Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (1)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
classes/views/frm-fields/back-end/settings.php (1)
117-129: Fix incorrect upgrade message and conditionally render attribute.Line 123 has two issues:
- Incorrect message: The
data-upgradeattribute shows "Unique fields" but this is for the Read Only option. This is misleading to users.- The attribute is always rendered even when Pro is installed, similar to the Unique option above.
Apply this diff:
- <label for="frm_read_only_field_<?php echo esc_attr( $field['id'] ); ?>" class="frm_help frm-mb-0" title="<?php esc_attr_e( 'Read Only: Show this field but do not allow the field value to be edited from the front-end.', 'formidable' ); ?>" data-trigger="hover"> - <input type="checkbox" id="frm_read_only_field_<?php echo esc_attr( $field['id'] ); ?>" name="field_options[read_only_<?php echo esc_attr( $field['id'] ); ?>]" value="1" <?php checked( $field['read_only'], 1 ); ?> - class="<?php echo esc_attr( $no_allow ); ?>" - <?php echo 'data-upgrade="' . esc_attr( ! $pro_is_installed ? __( 'Unique fields', 'formidable' ) : '' ) . '"'; ?> - /> + <label for="frm_read_only_field_<?php echo esc_attr( $field['id'] ); ?>" class="frm_help frm-mb-0" title="<?php esc_attr_e( 'Read Only: Show this field but do not allow the field value to be edited from the front-end.', 'formidable' ); ?>" data-trigger="hover"> + <input type="checkbox" id="frm_read_only_field_<?php echo esc_attr( $field['id'] ); ?>" name="field_options[read_only_<?php echo esc_attr( $field['id'] ); ?>]" value="1" <?php checked( $field['read_only'], 1 ); ?> + class="<?php echo esc_attr( $no_allow ); ?>" + <?php + if ( ! $pro_is_installed ) { + echo 'data-upgrade="' . esc_attr( __( 'Read Only fields', 'formidable' ) ) . '"'; + } + ?> + />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
classes/controllers/FrmFieldsController.php(1 hunks)classes/views/frm-fields/back-end/settings.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
classes/controllers/FrmFieldsController.php (1)
classes/helpers/FrmAppHelper.php (2)
pro_is_installed(355-357)FrmAppHelper(6-4664)
classes/views/frm-fields/back-end/settings.php (1)
classes/helpers/FrmAppHelper.php (2)
pro_is_installed(355-357)checked(2271-2275)
🪛 PHPMD (2.15.0)
classes/controllers/FrmFieldsController.php
366-366: Avoid unused local variables such as '$no_allow'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (1)
classes/controllers/FrmFieldsController.php (1)
365-366: LGTM! Variables correctly prepared for the view.The variables
$pro_is_installedand$no_alloware properly defined and will be available in the included settings view. The PHPMD warning about$no_allowbeing unused is a false positive, as static analysis tools cannot track variable usage across included files.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
js/formidable_overlay.js (2)
487-553: Consider fetching URL dynamically to avoid stale state.The module-level
urlandurlParamsare initialized once at load time. Ifwindow.locationchanges (via navigation or history manipulation), these references become stale, causing URL utility functions to operate on outdated data.Consider refactoring to fetch the URL dynamically:
-var url = new URL(window.location.href); -var urlParams = url.searchParams; +function getCurrentUrl() { + return new URL(window.location.href); +} + +function getCurrentParams() { + return getCurrentUrl().searchParams; +} var getQueryParam = function getQueryParam(paramName) { - return urlParams.get(paramName); + return getCurrentParams().get(paramName); }; var removeQueryParam = function removeQueryParam(paramName) { + var url = getCurrentUrl(); + var urlParams = url.searchParams; urlParams.delete(paramName); url.search = urlParams.toString(); return url.toString(); };Apply similar changes to
setQueryParamandhasQueryParam.
584-586: Consider extracting validation logic for clarity.The pattern
element instanceof HTMLElement || console.warn(...) || falseuses short-circuit evaluation cleverly but may be less readable than explicit conditional logic.Consider refactoring for clarity:
var isHTMLElement = function isHTMLElement(element) { - return element instanceof HTMLElement || console.warn('Invalid argument: Element must be an instance of HTMLElement') || false; + if (!(element instanceof HTMLElement)) { + console.warn('Invalid argument: Element must be an instance of HTMLElement'); + return false; + } + return true; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
js/form-templates.js.mapis excluded by!**/*.map,!**/*.mapjs/formidable-settings-components.js.mapis excluded by!**/*.map,!**/*.mapjs/formidable_admin.js.mapis excluded by!**/*.map,!**/*.mapjs/formidable_dashboard.js.mapis excluded by!**/*.map,!**/*.mapjs/formidable_styles.js.mapis excluded by!**/*.map,!**/*.map
📒 Files selected for processing (6)
classes/views/frm-fields/back-end/settings.php(1 hunks)css/admin/frm-settings-components.css(1 hunks)css/font_icons.css(1 hunks)js/formidable_dashboard.js(1 hunks)js/formidable_overlay.js(1 hunks)resources/scss/admin/components/panel/_options-panel.scss(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- resources/scss/admin/components/panel/_options-panel.scss
- css/font_icons.css
🚧 Files skipped from review as they are similar to previous changes (1)
- classes/views/frm-fields/back-end/settings.php
🧰 Additional context used
🧬 Code graph analysis (1)
js/formidable_dashboard.js (3)
js/src/components/class-counter.js (1)
frmCounter(1-46)js/src/components/class-tabs-navigator.js (1)
frmTabsNavigator(1-135)js/src/core/utils/animation.js (1)
frmAnimate(1-76)
🪛 Biome (2.1.2)
js/formidable_dashboard.js
[error] 15-15: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 83-83: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 285-285: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 544-544: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 804-804: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
js/formidable_overlay.js
[error] 16-16: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 224-224: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 483-483: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
🔇 Additional comments (3)
css/admin/frm-settings-components.css (1)
1-619: This is an auto-generated file; review and edit the source SCSS instead.This CSS file is compiled from SCSS via the webpack build pipeline (indicated by the loader chain in lines 1-3 and the source map reference on line 619). Manually editing compiled CSS files can be overwritten during rebuilds.
Verify that you've made the intended changes in the source SCSS file (
resources/scss/admin/frm-settings-components.scss) rather than here. If the changes are simply a result of SCSS reformatting and regeneration, that's acceptable—but confirm the source is the system of record.js/formidable_overlay.js (2)
140-151: Verify thatoverlayData.copyis sanitized.Using
innerHTMLcan introduce XSS vulnerabilities ifoverlayData.copycontains unsanitized user input. Ensure this data is trusted or properly sanitized before rendering.
346-348: Clarify the intent of callingtaskin boththenandcatch.The pattern
.then(task).catch(task)executes the task function in both success and error cases. This seems unusual—typically,catchhandles errors differently rather than re-executing the task. Is this intentional to ensure the queue continues regardless of errors?
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
classes/views/frm-fields/back-end/settings.php (2)
110-110: Conditionally render thedata-upgradeattribute.The
data-upgradeattribute is always rendered, even when Pro is installed (resulting indata-upgrade=""). This matches the previous review feedback. Only output this attribute when Pro is not installed.Apply this diff:
- <?php echo 'data-upgrade="' . esc_attr( $pro_is_installed ? '' : __( 'Unique fields', 'formidable' ) ) . '"'; ?>/> + <?php + if ( ! $pro_is_installed ) { + echo 'data-upgrade="' . esc_attr( __( 'Unique fields', 'formidable' ) ) . '"'; + } + ?>/>
123-123: Conditionally render thedata-upgradeattribute.Same issue as the Unique field: the
data-upgradeattribute is always rendered, even when Pro is installed. Only output this attribute when Pro is not installed.Apply this diff:
- <?php echo 'data-upgrade="' . esc_attr( $pro_is_installed ? '' : __( 'Read only fields', 'formidable' ) ) . '"'; ?> + <?php + if ( ! $pro_is_installed ) { + echo 'data-upgrade="' . esc_attr( __( 'Read only fields', 'formidable' ) ) . '"'; + } + ?>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/views/frm-fields/back-end/settings.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
classes/views/frm-fields/back-end/settings.php (1)
classes/helpers/FrmAppHelper.php (3)
pro_is_installed(355-357)FrmAppHelper(6-4664)tooltip_icon(4570-4582)
🔇 Additional comments (1)
classes/views/frm-fields/back-end/settings.php (1)
376-402: LGTM! Consistent upsell implementation.The Before/After Input upsell blocks are well-implemented with proper readonly attributes, data-upgrade attributes, and consistent styling. All output is properly escaped.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
css/admin/form-templates.css (1)
22-24: Consider additional styling for better UX.The opacity reduction effectively creates a "disabled" appearance. However, consider adding complementary styles to clarify that these elements are not interactive:
.frm_show_upgrade { opacity: .5; + cursor: not-allowed; }If the elements are truly non-interactive and shouldn't trigger any events, you might also consider:
.frm_show_upgrade { opacity: .5; cursor: not-allowed; pointer-events: none; }Note: Only add
pointer-events: noneif you don't need to capture clicks to show an upgrade modal. If JavaScript handles clicks on these elements (via thedata-upgradeattribute), omit this property.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
classes/views/form-templates/index.php(1 hunks)css/admin/form-templates.css(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
classes/views/form-templates/index.php (1)
classes/helpers/FrmAppHelper.php (4)
FrmAppHelper(6-4664)pro_is_installed(355-357)icon_by_class(1275-1312)show_pill_text(4334-4339)
🔇 Additional comments (1)
classes/views/form-templates/index.php (1)
58-65: Logic correctly prevents duplicate buttons.The conditional rendering approach is well-designed:
- When Pro is NOT installed (line 58): Shows a semi-transparent teaser button to encourage upgrades
- When Pro IS installed: The action hook at line 57 allows the Pro add-on to inject a functional button
This prevents duplicate buttons while providing an upgrade prompt to Lite users.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
js/formidable_dashboard.js (1)
73-94: Guard inbox subscription elements before attaching the click handler
initInboxassumes#frm_leave_emailand#frm-add-my-email-addressalways exist. On dashboards where these controls are missing,document.querySelector("#frm-add-my-email-address")will benull, so callingaddEventListeneron it throws and prevents the rest of the dashboard JS from running.Wrap the handler attachment in existence checks, e.g.:
- var t=document.querySelector("#frm_leave_email"); - document.querySelector("#frm-add-my-email-address").addEventListener("click",(function(){ - e.saveSubscribedEmail(t.value).then() - })) + var t = document.querySelector("#frm_leave_email"); + var btn = document.querySelector("#frm-add-my-email-address"); + if (t && btn) { + btn.addEventListener("click", function () { + e.saveSubscribedEmail(t.value).then(); + }); + }This preserves behavior where the controls exist while avoiding runtime errors when they do not.
js/formidable_overlay.js (1)
1-1: Past critical issues remain unresolved.This minified file still has the architectural issues flagged in previous reviews:
- Event handler context binding issue in
initCloseButton- Missing
formidable_domscript dependency- Missing
formidable_admin_globalscript dependencyThese must be addressed in the source files or build configuration.
classes/views/frm-fields/back-end/settings.php (3)
110-110: Conditionally render thedata-upgradeattribute.The
data-upgradeattribute is always rendered, even when Pro is installed (resulting indata-upgrade=""). Only output this attribute when$pro_is_installedis false.Apply this diff:
- <?php echo 'data-upgrade="' . esc_attr( $pro_is_installed ? '' : __( 'Unique fields', 'formidable' ) ) . '"'; ?>/> + <?php + if ( ! $pro_is_installed ) { + echo 'data-upgrade="' . esc_attr( __( 'Unique fields', 'formidable' ) ) . '"'; + } + ?>/>
352-352: Addreadonlyattribute for consistency.The Autocomplete select element is missing a
readonlyattribute, while the Visibility select on line 369 correctly includes it. Since both are upsell fields meant to be non-interactive in the Lite version, they should be consistent.Apply this diff:
- <select name="field_options[autocomplete_<?php echo absint( $field['id'] ); ?>]" id="field_options_autocomplete_<?php echo absint( $field['id'] ); ?>" data-upgrade="<?php esc_attr_e( 'Autocomplete options', 'formidable' ); ?>"> + <select readonly name="field_options[autocomplete_<?php echo absint( $field['id'] ); ?>]" id="field_options_autocomplete_<?php echo absint( $field['id'] ); ?>" data-upgrade="<?php esc_attr_e( 'Autocomplete options', 'formidable' ); ?>">
122-124: Conditionally render thedata-upgradeattribute.Similar to the Unique field above, the
data-upgradeattribute on the Read Only checkbox is always rendered, even when Pro is installed. Only output this attribute when$pro_is_installedis false.Apply this diff:
- class="<?php echo esc_attr( $no_allow ); ?>" - <?php echo 'data-upgrade="' . esc_attr( $pro_is_installed ? '' : __( 'Read only fields', 'formidable' ) ) . '"'; ?> - /> + class="<?php echo esc_attr( $no_allow ); ?>" + <?php + if ( ! $pro_is_installed ) { + echo 'data-upgrade="' . esc_attr( __( 'Read only fields', 'formidable' ) ) . '"'; + } + ?>/>
🧹 Nitpick comments (1)
css/frm_testing_mode.css (1)
2-3: Avoid duplicating the Bootstrap tooltip blockThis
/*! Bootstrap v4.6.1 */ .tooltip { … }section appears to be a second copy of the same tooltip styles already present elsewhere in this file. It adds noise, hurts maintainability, and triggers duplicate‑property warnings without changing behavior.Recommend removing this duplicate tooltip block from
frm_testing_mode.cssand keeping a single shared Bootstrap tooltip definition.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
classes/views/frm-fields/back-end/settings.php(2 hunks)css/admin/frm-settings-components.css(1 hunks)css/frm_testing_mode.css(1 hunks)js/addons-page.js(1 hunks)js/formidable-settings-components.js(1 hunks)js/formidable_blocks.js(1 hunks)js/formidable_dashboard.js(1 hunks)js/formidable_overlay.js(1 hunks)js/formidable_styles.js(1 hunks)js/frm_testing_mode.js(1 hunks)js/onboarding-wizard.js(1 hunks)js/welcome-tour.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- css/admin/frm-settings-components.css
🧰 Additional context used
🧬 Code graph analysis (9)
classes/views/frm-fields/back-end/settings.php (1)
classes/helpers/FrmAppHelper.php (3)
pro_is_installed(355-357)FrmAppHelper(6-4666)tooltip_icon(4561-4573)
js/formidable_dashboard.js (1)
js/src/admin/admin.js (43)
e(8207-8207)t(692-692)t(717-717)t(8988-8988)t(10174-10174)i(438-438)i(1771-1771)i(2249-2249)i(2993-2993)i(3008-3008)i(3080-3080)i(3098-3098)i(3112-3115)i(3138-3146)i(3176-3176)i(3185-3185)i(3446-3452)i(3588-3591)i(5413-5413)i(5482-5483)i(5557-5559)frmDom(239-239)frmDom(240-240)frmDom(241-241)a(3056-3056)s(274-274)s(8213-8213)s(8964-8964)s(9041-9041)c(697-697)c(722-722)c(7245-7245)c(9064-9064)f(9003-9003)v(3352-3352)v(3372-3372)v(6882-6882)v(7249-7249)v(7800-7800)v(7816-7816)v(7832-7832)p(8149-8149)p(8164-8164)
js/frm_testing_mode.js (2)
js/src/admin/addon-state.js (2)
a(200-200)frmDom(3-3)js/src/frm_testing_mode.js (1)
frmDom(34-34)
js/addons-page.js (1)
js/src/admin/admin.js (45)
t(692-692)t(717-717)t(8988-8988)t(10174-10174)e(8207-8207)i(438-438)i(1771-1771)i(2249-2249)i(2993-2993)i(3008-3008)i(3080-3080)i(3098-3098)i(3112-3115)i(3138-3146)i(3176-3176)i(3185-3185)i(3446-3452)i(3588-3591)i(5413-5413)i(5482-5483)i(5557-5559)a(3056-3056)c(697-697)c(722-722)c(7245-7245)c(9064-9064)s(274-274)s(8213-8213)s(8964-8964)s(9041-9041)f(9003-9003)p(8149-8149)p(8164-8164)frmDom(239-239)frmDom(240-240)frmDom(241-241)v(3352-3352)v(3372-3372)v(6882-6882)v(7249-7249)v(7800-7800)v(7816-7816)v(7832-7832)w(9537-9537)wp(258-258)
js/formidable_blocks.js (1)
js/src/admin/addon-state.js (2)
a(200-200)ajaxurl(12-12)
js/formidable_overlay.js (2)
js/src/admin/admin.js (41)
e(8207-8207)t(692-692)t(717-717)t(8988-8988)t(10174-10174)frmDom(239-239)frmDom(240-240)frmDom(241-241)a(3056-3056)i(438-438)i(1771-1771)i(2249-2249)i(2993-2993)i(3008-3008)i(3080-3080)i(3098-3098)i(3112-3115)i(3138-3146)i(3176-3176)i(3185-3185)i(3446-3452)i(3588-3591)i(5413-5413)i(5482-5483)i(5557-5559)c(697-697)c(722-722)c(7245-7245)c(9064-9064)s(274-274)s(8213-8213)s(8964-8964)s(9041-9041)f(9003-9003)v(3352-3352)v(3372-3372)v(6882-6882)v(7249-7249)v(7800-7800)v(7816-7816)v(7832-7832)js/src/admin/addon-state.js (2)
frmDom(3-3)a(200-200)
js/formidable_styles.js (1)
js/src/admin/admin.js (45)
t(692-692)t(717-717)t(8988-8988)t(10174-10174)e(8207-8207)i(438-438)i(1771-1771)i(2249-2249)i(2993-2993)i(3008-3008)i(3080-3080)i(3098-3098)i(3112-3115)i(3138-3146)i(3176-3176)i(3185-3185)i(3446-3452)i(3588-3591)i(5413-5413)i(5482-5483)i(5557-5559)a(3056-3056)s(274-274)s(8213-8213)s(8964-8964)s(9041-9041)c(697-697)c(722-722)c(7245-7245)c(9064-9064)f(9003-9003)p(8149-8149)p(8164-8164)v(3352-3352)v(3372-3372)v(6882-6882)v(7249-7249)v(7800-7800)v(7816-7816)v(7832-7832)w(9537-9537)frmDom(239-239)frmDom(240-240)frmDom(241-241)wp(258-258)
js/onboarding-wizard.js (2)
js/src/admin/admin.js (44)
t(692-692)t(717-717)t(8988-8988)t(10174-10174)e(8207-8207)i(438-438)i(1771-1771)i(2249-2249)i(2993-2993)i(3008-3008)i(3080-3080)i(3098-3098)i(3112-3115)i(3138-3146)i(3176-3176)i(3185-3185)i(3446-3452)i(3588-3591)i(5413-5413)i(5482-5483)i(5557-5559)frmDom(239-239)frmDom(240-240)frmDom(241-241)a(3056-3056)c(697-697)c(722-722)c(7245-7245)c(9064-9064)s(274-274)s(8213-8213)s(8964-8964)s(9041-9041)f(9003-9003)p(8149-8149)p(8164-8164)v(3352-3352)v(3372-3372)v(6882-6882)v(7249-7249)v(7800-7800)v(7816-7816)v(7832-7832)w(9537-9537)js/src/admin/addon-state.js (3)
frmDom(3-3)a(200-200)ajaxurl(12-12)
js/welcome-tour.js (1)
js/src/welcome-tour/ui/spotlight.js (1)
getComputedStyle(123-123)
🪛 Biome (2.1.2)
css/frm_testing_mode.css
[error] 3-3: Unexpected shorthand property background after background-color
(lint/suspicious/noShorthandPropertyOverrides)
[error] 3-3: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
text-align is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 3-3: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
border-radius is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
js/formidable_dashboard.js
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
js/frm_testing_mode.js
[error] 2-2: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 2-2: Do not use the t variable name as a label
The variable is declared here
Creating a label with the same name as an in-scope variable leads to confusion.
(lint/suspicious/noLabelVar)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
js/addons-page.js
[error] 1-1: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
[error] 1-1: Unsafe usage of 'return'.
'return' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
[error] 1-1: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not use the t variable name as a label
The variable is declared here
Creating a label with the same name as an in-scope variable leads to confusion.
(lint/suspicious/noLabelVar)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
js/formidable-settings-components.js
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
js/formidable_blocks.js
[error] 1-1: Unsafe usage of 'return'.
'return' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
[error] 1-1: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
js/formidable_overlay.js
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
js/formidable_styles.js
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not use the t variable name as a label
The variable is declared here
Creating a label with the same name as an in-scope variable leads to confusion.
(lint/suspicious/noLabelVar)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
js/onboarding-wizard.js
[error] 2-2: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 2-2: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
[error] 2-2: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 2-2: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 2-2: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 2-2: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 2-2: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 2-2: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 2-2: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 2-2: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 2-2: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 2-2: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 2-2: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 2-2: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
js/welcome-tour.js
[error] 2-2: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 2-2: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 2-2: Do not use the t variable name as a label
The variable is declared here
Creating a label with the same name as an in-scope variable leads to confusion.
(lint/suspicious/noLabelVar)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
🔇 Additional comments (1)
js/formidable_blocks.js (1)
1-1: LGTM - minified build artifact.This minified file contains only identifier renames (export symbol A→Ay, CSS class/keyframe renames) which are normal for minified builds. No functional changes detected.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
js/formidable_blocks.js (1)
1-380: Views block addon toggle helper looks fine; consider reusing shared helper and adding minimal error handlingThe bundled
Oe.toggleAddonStateimplementation and its use in theformidable/simple-viewblock look consistent with existing addon-toggle patterns and should work in the admin/editor whereajaxurlis globally defined.Since you already have addon-state logic in your source (e.g.,
js/src/admin/addon-state.js/js/frm_testing_mode.js) that also handles theajaxurlvsfrm_js.ajax_urlfallback, it may be worth consolidating these into a single helper at the source level to avoid divergence and keep URL/nonce handling and error behavior in one place. That would also make it easier to surface a basic failure state (e.g., resetisLoadingand optionally show a notice) instead of assuming the request always succeeds.No action needed on this generated bundle; any refactor should target the underlying source modules before rebuilding.
classes/controllers/FrmFieldsController.php (1)
370-379: Upsell flag wiring looks correct; consider centralizing field-type sets and ignore “unused” warningsThe new flags in
load_single_field_settings()are logically sound:
FrmAppHelper::pro_is_installed()is cached once and reused.show_upsell_for_*correctly gates on! $pro_is_installedand the relevantfield_has_*_optionboolean.$no_allowand the various$show_upsell_for_*/$field_has_*_optionvariables are clearly intended forsettings.phpvia theinclude, so PHPMD’s “unused local variable” warnings here are false positives.Two minor suggestions:
- If these same field-type lists (for Unique, Read Only, Before/After Contents) are or become needed elsewhere, it may be worth centralizing them (constants or a small helper) to avoid drift between Lite/Pro behavior.
- It’s worth double-checking that the arrays of field types for each option exactly match the intended Lite-capable types and UI expectations, but the structure here is fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
js/formidable_admin.js.mapis excluded by!**/*.map,!**/*.map
📒 Files selected for processing (3)
classes/controllers/FrmFieldsController.php(1 hunks)js/formidable_blocks.js(1 hunks)js/frm_testing_mode.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
classes/controllers/FrmFieldsController.php (1)
classes/helpers/FrmAppHelper.php (2)
pro_is_installed(394-396)FrmAppHelper(6-4985)
js/formidable_blocks.js (1)
js/src/admin/addon-state.js (2)
a(198-198)ajaxurl(12-12)
js/frm_testing_mode.js (2)
js/src/admin/upgrade-popup.js (1)
frmDom(3-3)js/src/frm_testing_mode.js (1)
frmDom(34-34)
🪛 Biome (2.1.2)
js/formidable_blocks.js
[error] 1-1: Unsafe usage of 'return'.
'return' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
[error] 1-1: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
js/frm_testing_mode.js
[error] 2-2: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 2-2: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 2-2: Do not use the e variable name as a label
The variable is declared here
Creating a label with the same name as an in-scope variable leads to confusion.
(lint/suspicious/noLabelVar)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 2-2: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
🪛 PHPMD (2.15.0)
classes/controllers/FrmFieldsController.php
371-371: Avoid unused local variables such as '$no_allow'. (undefined)
(UnusedLocalVariable)
374-374: Avoid unused local variables such as '$show_upsell_for_unique_value'. (undefined)
(UnusedLocalVariable)
376-376: Avoid unused local variables such as '$show_upsell_for_read_only'. (undefined)
(UnusedLocalVariable)
378-378: Avoid unused local variables such as '$show_upsell_for_before_after_contents'. (undefined)
(UnusedLocalVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 8 tests in WP trunk
🔇 Additional comments (1)
js/frm_testing_mode.js (1)
2-240: Generated bundle change appears non-functional; OK to keep as-isThis file is clearly a compiled bundle (webpack-style). The shown change doesn’t introduce any observable behavioral differences, and the existing
toggleAddonState+ upgrade modal logic still looks consistent. Static-analysis complaints about things like fallthrough and function reassignment are expected on minified output and shouldn’t be addressed here; any cleanups should happen in the source modules before rebuilding.
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)
classes/views/frm-fields/back-end/settings.php (1)
117-129: Conditionally render thedata-upgradeattribute.Similar to the unique field checkbox, the
data-upgradeattribute on line 123 is always rendered (producingdata-upgrade=""when Pro is installed). Apply the same conditional rendering pattern.Apply this diff:
- <input type="checkbox" id="frm_read_only_field_<?php echo esc_attr( $field['id'] ); ?>" name="field_options[read_only_<?php echo esc_attr( $field['id'] ); ?>]" value="1" <?php checked( $field['read_only'], 1 ); ?> - class="<?php echo esc_attr( $no_allow ); ?>" - <?php echo 'data-upgrade="' . esc_attr( $pro_is_installed ? '' : __( 'Read only fields', 'formidable' ) ) . '"'; ?> - /> + <input type="checkbox" id="frm_read_only_field_<?php echo esc_attr( $field['id'] ); ?>" name="field_options[read_only_<?php echo esc_attr( $field['id'] ); ?>]" value="1" <?php checked( $field['read_only'], 1 ); ?> + class="<?php echo esc_attr( $no_allow ); ?>" + <?php + if ( ! $pro_is_installed ) { + echo 'data-upgrade="' . esc_attr( __( 'Read only fields', 'formidable' ) ) . '"'; + } + ?>/>
♻️ Duplicate comments (2)
classes/views/frm-fields/back-end/settings.php (2)
105-115: Conditionally render thedata-upgradeattribute.The
data-upgradeattribute on line 110 is always rendered (producingdata-upgrade=""when Pro is installed). For cleaner markup, only output this attribute when Pro is not installed.Apply this diff:
- <input type="checkbox" name="field_options[unique_<?php echo esc_attr( $field['id'] ); ?>]" id="frm_uniq_field_<?php echo esc_attr( $field['id'] ); ?>" value="1" <?php checked( $field['unique'], 1 ); ?> class="frm_mark_unique <?php echo esc_attr( $no_allow ); ?>" - <?php echo 'data-upgrade="' . esc_attr( $pro_is_installed ? '' : __( 'Unique fields', 'formidable' ) ) . '"'; ?>/> + <input type="checkbox" name="field_options[unique_<?php echo esc_attr( $field['id'] ); ?>]" id="frm_uniq_field_<?php echo esc_attr( $field['id'] ); ?>" value="1" <?php checked( $field['unique'], 1 ); ?> class="frm_mark_unique <?php echo esc_attr( $no_allow ); ?>" + <?php + if ( ! $pro_is_installed ) { + echo 'data-upgrade="' . esc_attr( __( 'Unique fields', 'formidable' ) ) . '"'; + } + ?>/>
337-374: Addreadonlyattribute to Autocomplete select for consistency.The Autocomplete select element on line 352 is missing the
readonlyattribute, while the Visibility select on line 369 correctly includes it. Both are upsell fields that should be non-interactive in Lite.Apply this diff:
- <select name="field_options[autocomplete_<?php echo absint( $field['id'] ); ?>]" id="field_options_autocomplete_<?php echo absint( $field['id'] ); ?>" data-upgrade="<?php esc_attr_e( 'Autocomplete options', 'formidable' ); ?>"> + <select readonly name="field_options[autocomplete_<?php echo absint( $field['id'] ); ?>]" id="field_options_autocomplete_<?php echo absint( $field['id'] ); ?>" data-upgrade="<?php esc_attr_e( 'Autocomplete options', 'formidable' ); ?>">
🧹 Nitpick comments (1)
classes/views/frm-fields/back-end/settings.php (1)
376-402: Consider consistency between Before Input and After Input labels.The Before Input label (line 379) uses
frm-h-stack-xsclass and includes a tooltip, while the After Input label (line 396) has neither. If this is intentional (e.g., the tooltip on line 379 explains both fields), this is fine. Otherwise, consider adding the same styling and tooltip for consistency.If both fields should be consistent, apply this diff:
<p class="frm_form_field frm6"> - <label for="append_<?php echo absint( $field['id'] ); ?>" class="frm_show_upgrade"> + <label class="frm-h-stack-xs frm_show_upgrade" for="append_<?php echo absint( $field['id'] ); ?>"> <span><?php esc_html_e( 'After Input', 'formidable' ); ?></span> + <?php + FrmAppHelper::tooltip_icon( + __( 'A value entered here will show directly after the input box in the form.', 'formidable' ), + array( + 'data-placement' => 'right', + 'class' => 'frm-flex', + ) + ); + ?> </label>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
classes/controllers/FrmFieldsController.php(1 hunks)classes/views/frm-fields/back-end/settings.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
classes/views/frm-fields/back-end/settings.php (1)
classes/helpers/FrmAppHelper.php (3)
pro_is_installed(394-396)FrmAppHelper(6-4985)tooltip_icon(4859-4871)
classes/controllers/FrmFieldsController.php (1)
classes/helpers/FrmAppHelper.php (2)
pro_is_installed(394-396)FrmAppHelper(6-4985)
🪛 PHPMD (2.15.0)
classes/controllers/FrmFieldsController.php
371-371: Avoid unused local variables such as '$no_allow'. (undefined)
(UnusedLocalVariable)
378-378: Avoid unused local variables such as '$show_upsell_for_unique_value'. (undefined)
(UnusedLocalVariable)
380-380: Avoid unused local variables such as '$show_upsell_for_read_only'. (undefined)
(UnusedLocalVariable)
382-382: Avoid unused local variables such as '$show_upsell_for_before_after_contents'. (undefined)
(UnusedLocalVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: Cypress
🔇 Additional comments (1)
classes/controllers/FrmFieldsController.php (1)
370-383: Logic for upsell flags is well-structured.The variables flagged by PHPMD as "unused" are actually used in the included view file (line 383:
classes/views/frm-fields/back-end/settings.php). These are false positives since static analysis cannot track usage acrossincludestatements.The logic correctly:
- Checks Pro installation status once
- Defines field type eligibility for each upsell feature
- Combines both conditions to determine upsell visibility
AbdiTolesa
requested review from
Crabcyborg and
truongwp
and removed request for
Crabcyborg
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
classes/controllers/FrmFieldsController.php (2)
395-396: Replace@since x.xplaceholders with the actual release version.Both new helper methods still have
@since x.xin their docblocks. These placeholders should be updated to the actual plugin version number before shipping.Also applies to: 423-424
443-445: Fix the read‑only upgrade label.The read‑only helper is using
__( 'Unique fields', 'formidable' )as the upgrade text, but this should be a dedicated string for the read‑only feature, such as__( 'Read-only fields', 'formidable' ).Although a past review comment indicated this was addressed, the code still shows the incorrect text.
🔎 Proposed fix
if ( ! $pro_is_installed ) { - $read_only_element_atts['data-upgrade'] = __( 'Unique fields', 'formidable' ); + $read_only_element_atts['data-upgrade'] = __( 'Read-only fields', 'formidable' ); }classes/views/frm-fields/back-end/settings.php (1)
350-352: Addreadonlyattribute to Autocomplete select for consistency.The Autocomplete select element on Line 350 is missing a
readonlyattribute, while the Visibility select on Line 372 correctly includes it. Since both are upsell fields meant to be non‑interactive in the Lite version, they should be consistent.🔎 Proposed fix
- <select name="field_options[autocomplete_<?php echo absint( $field['id'] ); ?>]" id="field_options_autocomplete_<?php echo absint( $field['id'] ); ?>" data-upgrade="<?php esc_attr_e( 'Autocomplete options', 'formidable' ); ?>"> + <select readonly name="field_options[autocomplete_<?php echo absint( $field['id'] ); ?>]" id="field_options_autocomplete_<?php echo absint( $field['id'] ); ?>" data-upgrade="<?php esc_attr_e( 'Autocomplete options', 'formidable' ); ?>">
🧹 Nitpick comments (1)
classes/controllers/FrmFieldsController.php (1)
395-448: Consider simplifying the helper method signatures.Both
get_unique_element_attsandget_read_only_element_attsreceive$pro_is_installedas a parameter, but this value is readily available viaFrmAppHelper::pro_is_installed(). Removing this parameter would simplify the call sites and make the methods more self‑contained.Additionally, as noted in a past review, if you're using
FrmAppHelper::array_to_html_paramsto render these attributes, you don't need to callesc_attrinside the helper methods—array_to_html_paramshandles escaping automatically.Based on past reviews from truongwp and Crabcyborg.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
classes/controllers/FrmFieldsController.phpclasses/helpers/FrmFieldsHelper.phpclasses/views/frm-fields/back-end/settings.phpjs/formidable_admin.js
🧰 Additional context used
🧬 Code graph analysis (2)
classes/views/frm-fields/back-end/settings.php (1)
classes/helpers/FrmAppHelper.php (2)
pro_is_installed(391-393)FrmAppHelper(6-5049)
classes/controllers/FrmFieldsController.php (2)
classes/helpers/FrmAppHelper.php (3)
pro_is_installed(391-393)FrmAppHelper(6-5049)plugin_path(58-60)stripe/helpers/FrmTransLiteAppHelper.php (1)
plugin_path(16-18)
🪛 PHPMD (2.15.0)
classes/controllers/FrmFieldsController.php
378-378: Avoid unused local variables such as '$no_allow'. (undefined)
(UnusedLocalVariable)
381-381: Avoid unused local variables such as '$show_upsell_for_unique_value'. (undefined)
(UnusedLocalVariable)
386-386: Avoid unused local variables such as '$show_upsell_for_read_only'. (undefined)
(UnusedLocalVariable)
387-387: Avoid unused local variables such as '$show_upsell_for_before_after_contents'. (undefined)
(UnusedLocalVariable)
388-388: Avoid unused local variables such as '$show_upsell_for_autocomplete'. (undefined)
(UnusedLocalVariable)
389-389: Avoid unused local variables such as '$show_upsell_for_visibility'. (undefined)
(UnusedLocalVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: Cypress
🔇 Additional comments (3)
classes/helpers/FrmFieldsHelper.php (1)
2700-2778: LGTM: Clean implementation of customizable AI button classes.The changes to
render_ai_generate_options_buttonandget_ai_generate_options_button_classcorrectly allow custom classes and upgrade text to be passed through. The early return when$classesis a string is an appropriate optimization.classes/views/frm-fields/back-end/settings.php (2)
106-126: Verify the dynamic attribute rendering approach.The use of
FrmAppHelper::array_to_html_params()withFrmFieldsController::get_unique_element_atts()andFrmFieldsController::get_read_only_element_atts()is a cleaner approach than the previous inline attribute construction. This properly centralizes the attribute logic in the controller.
335-405: Confirm upsell flag usage aligns with field type capabilities.The new pro-gated blocks for Autocomplete (Line 335), Visibility (Line 357), and Before/After Contents (Line 379) are properly guarded by their respective
$show_upsell_for_*flags. Ensure that the field type checks in the controller (lines 386-389 in FrmFieldsController.php) accurately reflect which field types should display these upsell options.For example, past reviews noted that some field types like hidden fields shouldn't show Autocomplete or Visibility options.
Based on past review comments from Crabcyborg.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
classes/controllers/FrmFieldsController.php (2)
394-419: Update the@sinceplaceholder.The docblock still contains
@since x.xwhich should be replaced with the actual release version before shipping.🔎 Proposed fix
/** - * @since x.x + * @since 6.15 * * @param array $field * @param bool $pro_is_installedBased on past review comments.
422-447: Fix the read-only upsell label and update@sinceplaceholder.Line 443 incorrectly uses "Unique fields" for the read-only feature's
data-upgradeattribute. It should reference read-only functionality instead. Also, the@since x.xplaceholder needs updating.🔎 Proposed fix
/** - * @since x.x + * @since 6.15 * * @param array $field * @param bool $pro_is_installed @@ if ( ! $pro_is_installed ) { - $read_only_element_atts['data-upgrade'] = __( 'Unique fields', 'formidable' ); + $read_only_element_atts['data-upgrade'] = __( 'Read only fields', 'formidable' ); }Based on past review comments.
🧹 Nitpick comments (1)
classes/views/frm-fields/back-end/settings.php (1)
335-355: Addreadonlyattribute to the Autocomplete select for consistency.The Autocomplete select element is missing the
readonlyattribute, while the Visibility select on line 372 correctly includes it. Since both are upsell fields meant to be non-interactive in the Lite version, they should be consistent.🔎 Proposed fix
- <select name="field_options[autocomplete_<?php echo absint( $field['id'] ); ?>]" id="field_options_autocomplete_<?php echo absint( $field['id'] ); ?>" data-upgrade="<?php esc_attr_e( 'Autocomplete options', 'formidable' ); ?>"> + <select readonly name="field_options[autocomplete_<?php echo absint( $field['id'] ); ?>]" id="field_options_autocomplete_<?php echo absint( $field['id'] ); ?>" data-upgrade="<?php esc_attr_e( 'Autocomplete options', 'formidable' ); ?>">Based on past review comments.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
classes/controllers/FrmFieldsController.phpclasses/helpers/FrmFieldsHelper.phpclasses/views/frm-fields/back-end/settings.phpcss/frm_admin.cssresources/scss/admin/components/panel/_options-panel.scss
🚧 Files skipped from review as they are similar to previous changes (2)
- resources/scss/admin/components/panel/_options-panel.scss
- classes/helpers/FrmFieldsHelper.php
🧰 Additional context used
🧬 Code graph analysis (2)
classes/views/frm-fields/back-end/settings.php (1)
classes/helpers/FrmAppHelper.php (2)
pro_is_installed(391-393)FrmAppHelper(6-5054)
classes/controllers/FrmFieldsController.php (1)
classes/helpers/FrmAppHelper.php (3)
pro_is_installed(391-393)FrmAppHelper(6-5054)plugin_path(58-60)
🪛 PHPMD (2.15.0)
classes/controllers/FrmFieldsController.php
380-380: Avoid unused local variables such as '$show_upsell_for_unique_value'. (undefined)
(UnusedLocalVariable)
385-385: Avoid unused local variables such as '$show_upsell_for_read_only'. (undefined)
(UnusedLocalVariable)
386-386: Avoid unused local variables such as '$show_upsell_for_before_after_contents'. (undefined)
(UnusedLocalVariable)
387-387: Avoid unused local variables such as '$show_upsell_for_autocomplete'. (undefined)
(UnusedLocalVariable)
388-388: Avoid unused local variables such as '$show_upsell_for_visibility'. (undefined)
(UnusedLocalVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: Cypress
🔇 Additional comments (5)
classes/controllers/FrmFieldsController.php (1)
377-389: LGTM! Static analysis false positives can be ignored.The upsell flags are correctly computed based on field types and used in the included view file (line 391). The static analysis warnings about unused variables are false positives since these variables are extracted into the view's scope via
include.classes/views/frm-fields/back-end/settings.php (4)
106-115: LGTM!The unique field option is now correctly rendered using
FrmFieldsController::get_unique_element_atts()andFrmAppHelper::array_to_html_params(), with proper conditional display based on the upsell flag. This addresses the previousNonStaticSelfCallissue.
117-126: LGTM!The read-only field option follows the same pattern as the unique field option, correctly using
FrmFieldsController::get_read_only_element_atts()with proper conditional display.
357-377: LGTM!The Visibility upsell block is correctly implemented with proper conditional guard, tooltip, and the
readonlyattribute on the select element to prevent interaction.
379-405: LGTM!The Before Input and After Input upsell blocks are correctly implemented with appropriate guards, tooltips, and
readonlyattributes on both input elements.
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI Agents
In @classes/helpers/FrmSettingsUpsellHelper.php:
- Around line 60-62: Replace the incorrect upsell label assignment for read-only
fields: update the $read_only_element_atts['data-upgrade'] value (in
FrmSettingsUpsellHelper, where the read-only element attributes are set) to use
the correct translated string for the read-only feature, e.g. change __( 'Unique
fields', 'formidable' ) to __( 'Read-only fields', 'formidable' ), preserving
the translation domain and existing attribute key.
In @classes/views/form-templates/index.php:
- Around line 59-61: The upsell include uses a custom pattern and the button
lacks aria-disabled on the button element; either refactor to use
FrmAddonsController::conditional_action_button() for the 'ai' plugin (matching
other upsells) or, if keeping the include (FrmAppHelper::plugin_path() .
'/classes/views/frm-fields/back-end/upsell/ai-upsell-button.php'), update the
button markup inside that file so the button element includes
aria-disabled="true" when disabled and ensure the button has an accessible label
(not just the icon), and follow the same enable/disable conditions used by
FrmAIAppHelper checks to preserve behavior.
In @classes/views/frm-fields/back-end/upsell/ai-upsell-button.php:
- Line 6: Update the docblock @since tag in the ai-upsell-button.php file:
replace the placeholder `@since x.x` with the actual release version string for
this release (e.g., `@since 1.0.0`) ensuring it matches the version format used
across other files in the project.
In @classes/views/frm-fields/back-end/upsell/autocomplete.php:
- Line 6: Update the docblock @since tag in
classes/views/frm-fields/back-end/upsell/autocomplete.php by replacing the
placeholder "x.x" with the actual release version (e.g., "1.4.2"); edit the
@since line in the file's header docblock so it reflects the real version number
for this release.
- Line 24: The upsell select for the autocomplete option is missing the disabled
attribute, so make the select element with name
"field_options[autocomplete_<?php echo absint( $field['id'] ); ?>]" and id
"field_options_autocomplete_<?php echo absint( $field['id'] ); ?>"
non-interactive by adding the disabled attribute (matching the behavior used in
visibility.php for other upsell selects).
- Line 25: The option label uses esc_html_e() with the HTML entity "—"
which will be double-escaped; update the option output (the line calling
esc_html_e) to either use the actual em dash character (— Select —) inside
esc_html_e/esc_html() or output the entity unescaped by removing esc_html_e and
echoing the entity directly; modify that single call to esc_html_e( … )
accordingly so the rendered select shows a real em dash rather than the literal
"—".
In @classes/views/frm-fields/back-end/upsell/before-after-contents.php:
- Line 6: The @since tag in the docblock currently contains the placeholder
"x.x"; replace that placeholder with the correct release version string for this
release (e.g., "1.2.0") in the docblock above the upsell before-after contents
template so the @since annotation reflects the actual version; if your project
exposes a canonical version constant (e.g., PLUGIN_VERSION or a package.json
version), use that value to ensure consistency.
In @classes/views/frm-fields/back-end/upsell/visibility.php:
- Line 6: Update the PHPDoc @since tag in visibility.php by replacing the
placeholder "x.x" with the actual release version for this change (i.e., set the
@since value to the current release number in the file's top docblock or the
PHPDoc for the related class/function such as the upsell visibility class or
method).
- Line 24: The select element for the upsell visibility field uses a name with a
trailing array suffix; change the name attribute from
field_options[admin_only_<?php echo absint( $field['id'] ); ?>][] to
field_options[admin_only_<?php echo absint( $field['id'] ); ?>] (remove the
trailing []), leaving the id and data-upgrade attributes intact; update the
select in the upsell visibility template (the <select> using absint(
$field['id'] )) so it matches other upsell selects and test expectations.
🧹 Nitpick comments (4)
classes/views/frm-fields/back-end/upsell/visibility.php (1)
24-24: Consider usingdisabledinstead ofreadonlyfor the select element.The
readonlyattribute on<select>elements has inconsistent browser support and behavior. For upsell UI elements that should be non-interactive,disabledis more appropriate and widely supported.🔎 Proposed fix
-<select readonly name="field_options[admin_only_<?php echo absint( $field['id'] ); ?>][]" id="field_options_admin_only_<?php echo absint( $field['id'] ); ?>" class="" data-upgrade="<?php esc_attr_e( 'Visibility options', 'formidable' ); ?>"> +<select disabled name="field_options[admin_only_<?php echo absint( $field['id'] ); ?>][]" id="field_options_admin_only_<?php echo absint( $field['id'] ); ?>" class="" data-upgrade="<?php esc_attr_e( 'Visibility options', 'formidable' ); ?>">classes/views/frm-fields/back-end/upsell/before-after-contents.php (2)
25-25: Remove unnecessaryaria-invalidattribute.The
aria-invalid="false"attribute on a readonly input field is unnecessary and adds no semantic value. Readonly inputs in an upsell context don't participate in form validation.🔎 Proposed fix
-<input type="text" readonly name="field_options[prepend_<?php echo absint( $field['id'] ); ?>]" id="prepend_<?php echo absint( $field['id'] ); ?>" aria-invalid="false" data-upgrade="<?php esc_attr_e( 'Before and after contents', 'formidable' ); ?>"/> +<input type="text" readonly name="field_options[prepend_<?php echo absint( $field['id'] ); ?>]" id="prepend_<?php echo absint( $field['id'] ); ?>" data-upgrade="<?php esc_attr_e( 'Before and after contents', 'formidable' ); ?>"/>
12-31: Improve consistency between "Before Input" and "After Input" labels.The two labels have inconsistent styling and features:
- "Before Input" (line 12) has
frm-h-stack-xsclass and a tooltip icon- "After Input" (line 29) lacks both the layout class and tooltip
For better UX consistency, consider adding similar styling and a tooltip to the "After Input" label, or document why they should differ.
🔎 Proposed fix
<p class="frm_form_field frm6"> - <label for="append_<?php echo absint( $field['id'] ); ?>" class="frm_show_upgrade"> - <?php esc_html_e( 'After Input', 'formidable' ); ?> + <label class="frm-h-stack-xs frm_show_upgrade" for="append_<?php echo absint( $field['id'] ); ?>"> + <span><?php esc_html_e( 'After Input', 'formidable' ); ?></span> + <?php + FrmAppHelper::tooltip_icon( + __( 'A value entered here will show directly after the input box in the form.', 'formidable' ), + array( + 'data-placement' => 'right', + 'class' => 'frm-flex', + ) + ); + ?> </label>classes/helpers/FrmSettingsUpsellHelper.php (1)
9-9: Replace@since x.xplaceholders with the actual release version.Both method docblocks use
@since x.xplaceholders that should be updated to the actual plugin version for this release (e.g.,6.?.?).Also applies to: 41-41
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
classes/controllers/FrmFieldsController.phpclasses/helpers/FrmFieldsHelper.phpclasses/helpers/FrmSettingsUpsellHelper.phpclasses/views/form-templates/index.phpclasses/views/frm-fields/back-end/settings.phpclasses/views/frm-fields/back-end/upsell/ai-upsell-button.phpclasses/views/frm-fields/back-end/upsell/autocomplete.phpclasses/views/frm-fields/back-end/upsell/before-after-contents.phpclasses/views/frm-fields/back-end/upsell/visibility.php
🚧 Files skipped from review as they are similar to previous changes (1)
- classes/helpers/FrmFieldsHelper.php
🧰 Additional context used
🧬 Code graph analysis (7)
classes/controllers/FrmFieldsController.php (1)
classes/helpers/FrmAppHelper.php (2)
pro_is_installed(391-393)FrmAppHelper(6-5054)
classes/views/frm-fields/back-end/upsell/autocomplete.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5054)tooltip_icon(4925-4938)
classes/views/frm-fields/back-end/upsell/before-after-contents.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5054)tooltip_icon(4925-4938)
classes/views/frm-fields/back-end/upsell/visibility.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5054)tooltip_icon(4925-4938)
classes/views/frm-fields/back-end/upsell/ai-upsell-button.php (1)
classes/helpers/FrmFieldsHelper.php (2)
FrmFieldsHelper(6-2813)render_ai_generate_options_button(2735-2783)
classes/views/form-templates/index.php (1)
classes/helpers/FrmAppHelper.php (1)
FrmAppHelper(6-5054)
classes/helpers/FrmSettingsUpsellHelper.php (1)
classes/helpers/FrmAppHelper.php (2)
pro_is_installed(391-393)FrmAppHelper(6-5054)
🪛 PHPMD (2.15.0)
classes/controllers/FrmFieldsController.php
380-380: Avoid unused local variables such as '$show_upsell_for_unique_value'. (undefined)
(UnusedLocalVariable)
385-385: Avoid unused local variables such as '$show_upsell_for_read_only'. (undefined)
(UnusedLocalVariable)
386-386: Avoid unused local variables such as '$show_upsell_for_before_after_contents'. (undefined)
(UnusedLocalVariable)
387-387: Avoid unused local variables such as '$show_upsell_for_autocomplete'. (undefined)
(UnusedLocalVariable)
388-388: Avoid unused local variables such as '$show_upsell_for_visibility'. (undefined)
(UnusedLocalVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: PHP 8 tests in WP 6.9
🔇 Additional comments (5)
classes/views/frm-fields/back-end/upsell/ai-upsell-button.php (1)
10-13: Clarify ifbutton_textandupgrade_textshould differ for the upsell scenario.
button_textdisplays on the button itself (line 8 of generate-options-with-ai.php), whileupgrade_textappears in the upgrade modal message. Both are set to'Create with AI'in this file, but their default fallbacks differ ('Generate with AI'vs'Generate options with AI'), suggesting they're designed to be customized independently. Confirm whether the upgrade modal should display distinct messaging to clarify the upgrade action for users.classes/controllers/FrmFieldsController.php (1)
377-389: LGTM!The upsell flag logic is correct and appropriately gates Pro features based on field type. The variables are used by the included settings view file (line 391), so the static analysis warnings about unused variables are false positives.
classes/views/frm-fields/back-end/settings.php (3)
106-115: LGTM!The unique field rendering correctly uses the helper method and conditionally applies the upgrade class. The logic properly shows the control when either the display setting allows it or when an upsell should be displayed.
117-126: LGTM!The read-only field rendering correctly uses the helper method and follows the same pattern as the unique field above. Note: the helper method has a label text issue that was flagged separately.
335-347: LGTM!The new upsell blocks are properly gated by their respective flags and positioned correctly before the image-related options. The conditional includes ensure upsell views are only loaded when needed.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @classes/views/frm-fields/back-end/generate-options-with-ai.php:
- Around line 9-11: The conditional currently uses isset($args['show_pill'])
which treats falsy values like false, 0, or "" as present; update the if check
to use ! empty($args['show_pill']) so FrmAppHelper::show_pill_text( 'BETA' )
only runs when the flag is truthy, leaving the rest of the block intact.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/views/frm-fields/back-end/generate-options-with-ai.php
🧰 Additional context used
🧬 Code graph analysis (1)
classes/views/frm-fields/back-end/generate-options-with-ai.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5054)show_pill_text(4693-4698)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
🔇 Additional comments (1)
classes/views/frm-fields/back-end/generate-options-with-ai.php (1)
8-8: LGTM! Clean implementation of the suggested simplification.The null coalescing operator with proper escaping is the right approach for optional button text. This successfully implements the earlier suggestion from the review.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @classes/helpers/FrmSettingsUpsellHelper.php:
- Around line 6-11: The class docblock for FrmSettingsUpsellHelper contains a
placeholder "@since x.x"; replace "x.x" with the actual release version number
in the docblock above class FrmSettingsUpsellHelper (update the @since tag to
the real version string before merging).
- Around line 49-54: The read-only element attribute array in
get_read_only_element_atts is missing a class attribute for consistency with
get_unique_element_atts; update the $read_only_element_atts array in
get_read_only_element_atts to include 'class' => 'frm_mark_read_only' (matching
the naming pattern used by get_unique_element_atts) so the checkbox receives the
same styling/behavior hook as other similar elements.
🧹 Nitpick comments (3)
classes/helpers/FrmSettingsUpsellHelper.php (3)
19-38: Consider adding input validation.The method assumes
$fieldis an array with anidkey. Adding validation would prevent potential PHP warnings if the structure is unexpected.🔎 Example defensive validation
public static function get_unique_element_atts( $field ) { + if ( ! is_array( $field ) || ! isset( $field['id'] ) ) { + return array(); + } + $pro_is_installed = FrmAppHelper::pro_is_installed();
47-65: Consider adding input validation.Similar to
get_unique_element_atts, this method would benefit from validating that$fieldis an array with the required keys.🔎 Example defensive validation
public static function get_read_only_element_atts( $field ) { + if ( ! is_array( $field ) || ! isset( $field['id'] ) ) { + return array(); + } + $pro_is_installed = FrmAppHelper::pro_is_installed();
19-65: Reduce code duplication between methods.Both
get_unique_element_attsandget_read_only_element_attsfollow nearly identical patterns. Consider extracting the common logic into a private helper method to improve maintainability.🔎 Example refactored implementation
/** * Generic method to build upsell checkbox attributes. * * @since x.x * @param array $field * @param string $option_key The field option key (e.g., 'unique', 'read_only'). * @param string $id_prefix The HTML ID prefix (e.g., 'frm_uniq_field_', 'frm_read_only_field_'). * @param string $upgrade_label The localized upgrade label. * @param string $class Optional CSS class to add. * @return array */ private static function build_upsell_checkbox_atts( $field, $option_key, $id_prefix, $upgrade_label, $class = '' ) { if ( ! is_array( $field ) || ! isset( $field['id'] ) ) { return array(); } $pro_is_installed = FrmAppHelper::pro_is_installed(); $atts = array( 'type' => 'checkbox', 'name' => 'field_options[' . $option_key . '_' . $field['id'] . ']', 'id' => $id_prefix . $field['id'], 'value' => '1', ); if ( ! empty( $class ) ) { $atts['class'] = $class; } if ( ! empty( $field[ $option_key ] ) ) { $atts['checked'] = 'checked'; } if ( ! $pro_is_installed ) { $atts['data-upgrade'] = $upgrade_label; } return $atts; } public static function get_unique_element_atts( $field ) { return self::build_upsell_checkbox_atts( $field, 'unique', 'frm_uniq_field_', __( 'Unique fields', 'formidable' ), 'frm_mark_unique' ); } public static function get_read_only_element_atts( $field ) { return self::build_upsell_checkbox_atts( $field, 'read_only', 'frm_read_only_field_', __( 'Read only fields', 'formidable' ) ); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/helpers/FrmSettingsUpsellHelper.php
🧰 Additional context used
🧬 Code graph analysis (1)
classes/helpers/FrmSettingsUpsellHelper.php (1)
classes/helpers/FrmAppHelper.php (2)
pro_is_installed(391-393)FrmAppHelper(6-5054)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: Cypress
🔇 Additional comments (2)
classes/helpers/FrmSettingsUpsellHelper.php (2)
1-4: LGTM!Standard WordPress security check is properly implemented.
60-62: Previous copy-paste error has been fixed.The past review correctly identified that line 61 had "Unique fields" text. This has been corrected to "Read only fields". Great work!
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)
classes/helpers/FrmFieldsHelper.php (1)
750-750: Inconsistent handling of optional parameters.Line 750 uses the null coalescing operator (
??) for$args['upgrade_text'], while line 740 usesempty()for$args['class']. These operators have different behavior:
empty($args['class'])returnstruefornull,false,0,"",[], etc.$args['upgrade_text'] ?? defaultonly checks if the key doesn't exist or isnullThis means if a caller passes
$args['upgrade_text'] = ''(empty string), the empty string will be used rather than the default text. However, for class, an empty string would trigger the default class. This inconsistency could lead to confusion.🔎 Suggested fix for consistency
Consider using the same pattern for both parameters:
-$attributes['class'] = empty( $args['class'] ) ? self::get_ai_generate_options_button_class() : $args['class']; +$attributes['class'] = $args['class'] ?? self::get_ai_generate_options_button_class();Or explicitly handle empty strings for both:
-'upgrade' => $args['upgrade_text'] ?? __( 'Generate options with AI', 'formidable' ), +'upgrade' => ( ! empty( $args['upgrade_text'] ) ? $args['upgrade_text'] : __( 'Generate options with AI', 'formidable' ) ),
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/helpers/FrmFieldsHelper.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: Cypress
🔇 Additional comments (1)
classes/helpers/FrmFieldsHelper.php (1)
737-741: Good refactoring of attribute initialization.Moving the
classattribute out of the initial array and setting it conditionally on line 740 is a cleaner approach than including conditional logic within the array initialization. The blank line on 741 also improves readability.
Crabcyborg
left a comment
Crabcyborg
left a comment
There was a problem hiding this comment.
@AbdiTolesa It doesn't look like the upgrade button in the modal is pulling a utm_content.
It looks like utm_medium is off as well, and utm_content is blank.
We should be passing something like utm_content=unique_fields.
Also, utm_medium should be either pro or lite. For all of these new settings, I think we can just add a lite string.
|
@AbdiTolesa I also see this Learn More button but it tries to link me to 
|
Closes https://github.com/Strategy11/formidable-pro/issues/5873