-
Notifications
You must be signed in to change notification settings - Fork 39
Try breaking field group when frm_first class is added to a field in a group #2661
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughAdds an optional field-group-splitting flow when a field gains the Changes
Sequence Diagram(s)sequenceDiagram
participant AdminUI as Admin UI
participant Handler as liveChanges / changeFieldClass
participant Splitter as maybeBreakFieldGroup
participant DOM as DOM
AdminUI->>Handler: field class update event
Handler->>Handler: read shouldTryBreakFieldGroup (default: true)
Handler->>Splitter: maybeBreakFieldGroup(flag, field, classes, replaceWith)
alt flag enabled and conditions met
Splitter->>DOM: clone current field-group
Splitter->>DOM: move target field into new group
Splitter->>DOM: update DOM structure
Splitter->>Handler: split complete
else no split
Splitter->>Handler: no action
end
Handler->>DOM: apply class replacement to field
Handler->>AdminUI: emit update complete
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
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
js/src/admin/admin.js (1)
6763-6816: Field-group splitting logic has multiple critical correctness and robustness issues that will cause incorrect behavior and runtime errors.The
changeFieldClassenhancement for splitting a row whenfrm_firstis added has good intent but the implementation is flawed:
Updates the wrong DOM node (detached field)
The code removes fields from the original row viafieldsInFieldBox[i].remove()fori >= indexOfTargetField. This detaches the originalfieldelement from the DOM. However, the final mutationfield.className = field.className.replace( replace, replaceWith )happens on this detached node, not on its clone in the new visible row. The user will not see the updated classes.Missing null guard on
fieldBox
const fieldBox = field.parentElement.closest( 'li.frm_field_box.ui-draggable' );may returnnullif the element is not inside a draggable field box. The immediate call tofieldBox.cloneNode( true )will throw a runtime error. SincechangeFieldClassis called generically for any class change, this is a real risk outside the builder context.Splits incorrectly when target is the first field
WhenindexOfTargetField === 0, the loop removes all fields from the original row, leaving it empty. The new cloned box contains all fields. The empty original row persists in the DOM, causing stray empty groups and broken builder state.Cloning breaks drag-and-drop and event listeners
cloneNode(true)does not copy jQuery UI data, event listeners, or draggable/droppable state. The new row is inserted without calls tomakeDroppableormakeDraggable, so drag-and-drop does not work on it. Other JS listeners attached to child elements are also lost.No follow-up layout and builder sync
After restructuring,syncLayoutClasses,updateFieldOrder, andfieldUpdated()are not called, leaving internal state out of sync with the new DOM.Rework this block to move (not clone) existing field nodes, guard against null/non-builder contexts, skip splitting when the target is already first, and reinitialize drag-and-drop and layout after the split.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
js/addons-page.js.mapis excluded by!**/*.map,!**/*.mapjs/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_blocks.js.mapis excluded by!**/*.map,!**/*.mapjs/formidable_dashboard.js.mapis excluded by!**/*.map,!**/*.mapjs/formidable_overlay.js.mapis excluded by!**/*.map,!**/*.mapjs/formidable_styles.js.mapis excluded by!**/*.map,!**/*.mapjs/onboarding-wizard.js.mapis excluded by!**/*.map,!**/*.map
📒 Files selected for processing (6)
css/admin/frm-settings-components.css(1 hunks)css/admin/welcome-tour.css(1 hunks)css/font_icons.css(1 hunks)js/formidable_dashboard.js(1 hunks)js/formidable_overlay.js(1 hunks)js/src/admin/admin.js(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
js/formidable_overlay.js (3)
js/src/components/class-overlay.js (1)
frmOverlay(6-133)js/src/core/utils/animation.js (1)
frmAnimate(1-76)js/src/core/utils/error.js (3)
inputElement(16-16)errorElement(17-17)span(26-26)
🪛 Biome (2.1.2)
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] 527-527: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
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] 588-588: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 848-848: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
🔇 Additional comments (5)
css/admin/welcome-tour.css (1)
1-152: Safe formatting refactor with improved maintainability.The CSS has been expanded from a minified form to well-formatted, readable code with proper indentation and comments. All selectors (
.frm-checklist,.frm-spotlight, etc.) and property values are preserved; this is purely a readability and source-mapping enhancement with no functional styling changes.css/font_icons.css (1)
1-853: Comprehensive CSS formatting refactor—icon library fully preserved.This large icon-library stylesheet has been reformatted from minified to expanded form with proper indentation and structure. All ~250+ icon class definitions (
.frm_*_icon::beforerules) and their content values are preserved. The@font-facerule and all property declarations remain intact. This is a pure readability enhancement with no functional impact.css/admin/frm-settings-components.css (1)
1-627: Well-organized formatting refactor for admin component styles.This stylesheet has been expanded from minified form and reorganized with clear section comments (accordion, color picker, background image, radio buttons, sliders, tabs, miscellaneous). All selector chains, property values, custom variables (
--grey-*,--gap-*, etc.), and complex CSS (animations, transforms, flex layouts) are fully preserved. This is a pure structural and readability enhancement with zero functional impact on component styling.js/src/admin/admin.js (2)
1466-1518: Using a flag to distinguish programmatic vs user‑initiated class changes looks goodPassing
falseas a second argument to thechangeevent (jQuery( layoutClassesInput ).trigger( 'change', [ false ] );) cleanly threads context intoliveChangeswithout altering existing handler contracts. This is a reasonable way to avoid unintended field‑group breaking during automated layout sync.
3257-3279: liveChanges parameter signature is safe and properly integratedThe new signature
function liveChanges( event, shouldTryBreakFieldGroup = true )is consistent with jQuery's trigger semantics and safe for use as an event handler. The parameter is correctly forwarded tochangeFieldClass(), which uses it to conditionally break field groups when thefrm_firstclass is added. No external callers invokingliveChangesdirectly exist outside this file—both usages follow the jQuery event handler pattern via.on('change click', '[data-changeme]', liveChanges), so the default parameter value handles missing arguments correctly.
js/formidable_dashboard.js
Outdated
| // Cleanup observers when page unloads to prevent memory leaks | ||
| window.addEventListener('beforeunload', this.cleanupObservers); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cleanupObservers loses this context when passed as event handler.
The cleanupObservers method uses this.resizeObserver but is passed directly to addEventListener without binding. When called as an event handler, this will be the window object, not the frmTabsNavigator instance, causing the cleanup to silently fail.
// Cleanup observers when page unloads to prevent memory leaks
- window.addEventListener('beforeunload', this.cleanupObservers);
+ window.addEventListener('beforeunload', this.cleanupObservers.bind(this));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Cleanup observers when page unloads to prevent memory leaks | |
| window.addEventListener('beforeunload', this.cleanupObservers); | |
| } | |
| // Cleanup observers when page unloads to prevent memory leaks | |
| window.addEventListener('beforeunload', this.cleanupObservers.bind(this)); | |
| } |
🤖 Prompt for AI Agents
In js/formidable_dashboard.js around lines 122 to 124, cleanupObservers is
passed directly to window.addEventListener so it loses the frmTabsNavigator
instance `this` context; update the registration to preserve context (e.g., bind
the method to the instance or wrap it in an arrow/inline function that calls
this.cleanupObservers()) and ensure any removal uses the same bound reference so
observers are properly disconnected on unload.
js/formidable_dashboard.js
Outdated
| key: "initInbox", | ||
| value: function initInbox() { | ||
| var _this = this; | ||
| new _components_class_tabs_navigator__WEBPACK_IMPORTED_MODULE_1__.frmTabsNavigator('.frm-inbox-wrapper'); | ||
| var userEmailInput = document.querySelector('#frm_leave_email'); | ||
| var subscribeButton = document.querySelector('#frm-add-my-email-address'); | ||
| subscribeButton.addEventListener('click', function () { | ||
| _this.saveSubscribedEmail(userEmailInput.value).then(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing null checks before accessing DOM elements.
If #frm_leave_email or #frm-add-my-email-address elements don't exist in the DOM, this code will throw a runtime error when attempting to call addEventListener on null or access .value of null.
key: "initInbox",
value: function initInbox() {
var _this = this;
new _components_class_tabs_navigator__WEBPACK_IMPORTED_MODULE_1__.frmTabsNavigator('.frm-inbox-wrapper');
var userEmailInput = document.querySelector('#frm_leave_email');
var subscribeButton = document.querySelector('#frm-add-my-email-address');
+ if (!subscribeButton || !userEmailInput) {
+ return;
+ }
subscribeButton.addEventListener('click', function () {
_this.saveSubscribedEmail(userEmailInput.value).then();
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| key: "initInbox", | |
| value: function initInbox() { | |
| var _this = this; | |
| new _components_class_tabs_navigator__WEBPACK_IMPORTED_MODULE_1__.frmTabsNavigator('.frm-inbox-wrapper'); | |
| var userEmailInput = document.querySelector('#frm_leave_email'); | |
| var subscribeButton = document.querySelector('#frm-add-my-email-address'); | |
| subscribeButton.addEventListener('click', function () { | |
| _this.saveSubscribedEmail(userEmailInput.value).then(); | |
| }); | |
| } | |
| key: "initInbox", | |
| value: function initInbox() { | |
| var _this = this; | |
| new _components_class_tabs_navigator__WEBPACK_IMPORTED_MODULE_1__.frmTabsNavigator('.frm-inbox-wrapper'); | |
| var userEmailInput = document.querySelector('#frm_leave_email'); | |
| var subscribeButton = document.querySelector('#frm-add-my-email-address'); | |
| if (!subscribeButton || !userEmailInput) { | |
| return; | |
| } | |
| subscribeButton.addEventListener('click', function () { | |
| _this.saveSubscribedEmail(userEmailInput.value).then(); | |
| }); | |
| } |
🤖 Prompt for AI Agents
In js/formidable_dashboard.js around lines 884 to 893, the code assumes
document.querySelector('#frm_leave_email') and
document.querySelector('#frm-add-my-email-address') always return elements; add
null checks so you only call addEventListener when subscribeButton is non-null
and only read userEmailInput.value when userEmailInput is non-null (e.g., wrap
the listener registration in an if (subscribeButton) {
subscribeButton.addEventListener(...)} and inside the handler guard that
userEmailInput exists before calling saveSubscribedEmail with its value or
early-return), ensuring no runtime errors when those DOM elements are absent.
js/formidable_dashboard.js
Outdated
| }, { | ||
| key: "saveSubscribedEmail", | ||
| value: function saveSubscribedEmail(email) { | ||
| return fetch(window.ajaxurl, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/x-www-form-urlencoded' | ||
| }, | ||
| body: new URLSearchParams({ | ||
| action: this.options.ajax.action, | ||
| dashboard_action: this.options.ajax.dashboardActions.saveSubscribedEmail, | ||
| email: email | ||
| }) | ||
| }).then(function (result) { | ||
| return result.json(); | ||
| }); | ||
| } | ||
| }, { | ||
| key: "closeWelcomeBannerSaveCookieRequest", | ||
| value: function closeWelcomeBannerSaveCookieRequest() { | ||
| return fetch(window.ajaxurl, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/x-www-form-urlencoded' | ||
| }, | ||
| body: new URLSearchParams({ | ||
| action: this.options.ajax.action, | ||
| dashboard_action: this.options.ajax.dashboardActions.welcomeBanner, | ||
| banner_has_closed: 1 | ||
| }) | ||
| }).then(function (result) { | ||
| return result.json(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the js/formidable_dashboard.js file around the mentioned lines
wc -l js/formidable_dashboard.jsRepository: Strategy11/formidable-forms
Length of output: 101
🏁 Script executed:
# Get the imports and the specific methods mentioned
sed -n '1,50p' js/formidable_dashboard.jsRepository: Strategy11/formidable-forms
Length of output: 3068
🏁 Script executed:
# Check the specific lines mentioned (919-952)
sed -n '919,952p' js/formidable_dashboard.jsRepository: Strategy11/formidable-forms
Length of output: 1137
🏁 Script executed:
# Search for frmGlobal or nonce references in the file
rg -n "frmGlobal|nonce" js/formidable_dashboard.js | head -30Repository: Strategy11/formidable-forms
Length of output: 259
🏁 Script executed:
# Check the beginning of the source (after webpack bootstrap) to find imports/globals
sed -n '100,200p' js/formidable_dashboard.js | head -50Repository: Strategy11/formidable-forms
Length of output: 2350
🏁 Script executed:
# Search for server-side nonce validation in PHP files
rg -n -B2 -A5 "saveSubscribedEmail|welcomeBanner" --type=php | head -60Repository: Strategy11/formidable-forms
Length of output: 53
🏁 Script executed:
# Broader search for dashboard action handlers in PHP
rg -n "dashboard_action" --type=php | head -20Repository: Strategy11/formidable-forms
Length of output: 306
🏁 Script executed:
# Search for AJAX action handlers
rg -n -B2 -A10 "wp_ajax.*dashboard|dashboard.*wp_ajax" --type=phpRepository: Strategy11/formidable-forms
Length of output: 1179
🏁 Script executed:
# Look for any nonce validation in dashboard-related code
rg -n "wp_verify_nonce" --type=php | grep -i dashboardRepository: Strategy11/formidable-forms
Length of output: 53
🏁 Script executed:
# Search more broadly for where these actions are handled
fd -e php | xargs rg -l "dashboard_action" | head -10Repository: Strategy11/formidable-forms
Length of output: 117
🏁 Script executed:
# Examine the FrmDashboardController.php file around the AJAX handler
sed -n '320,380p' classes/controllers/FrmDashboardController.phpRepository: Strategy11/formidable-forms
Length of output: 1813
🏁 Script executed:
# Get more context around nonce validation
sed -n '1,360p' classes/controllers/FrmDashboardController.php | tail -100Repository: Strategy11/formidable-forms
Length of output: 3388
AJAX requests missing CSRF nonce protection.
The saveSubscribedEmail and closeWelcomeBannerSaveCookieRequest methods make POST requests to ajaxurl without including a nonce for CSRF verification. The nonce is available from window.frmGlobal.nonce but is not sent. Additionally, the server-side handler in FrmDashboardController::ajax_requests() does not validate a nonce before processing these actions.
Add the nonce parameter to both client-side requests:
body: new URLSearchParams({
action: this.options.ajax.action,
dashboard_action: this.options.ajax.dashboardActions.saveSubscribedEmail,
- email: email
+ email: email,
+ nonce: frmGlobal.nonce
})Apply the same fix to closeWelcomeBannerSaveCookieRequest. Then add server-side nonce verification in FrmDashboardController::ajax_requests() using wp_verify_nonce() before processing the dashboard actions.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In js/formidable_dashboard.js around lines 919 to 952, the two POST AJAX calls
(saveSubscribedEmail and closeWelcomeBannerSaveCookieRequest) do not include the
CSRF nonce; add the nonce from window.frmGlobal.nonce to the URLSearchParams
body for both requests (key name should match server expectation, e.g. 'nonce').
Also update the server-side FrmDashboardController::ajax_requests() to verify
the nonce with wp_verify_nonce($_REQUEST['nonce'], '<expected-action>') before
processing the dashboard_action, and return an appropriate WP_Error/JSON
response if verification fails. Ensure the nonce action string used in
wp_verify_nonce matches the one you use when generating the nonce in PHP.
js/formidable_overlay.js
Outdated
| if (buttons) { | ||
| var buttonsWrapperElementOptions = { | ||
| className: 'frm-overlay--cta frm-flex-box', | ||
| children: buttons | ||
| }; | ||
| return frmDom.div(buttonsWrapperElementOptions); | ||
| } | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Array truthiness check is always true.
The buttons variable is the result of .map(), which always returns an array. This condition is always truthy, even when the array is empty, causing an empty wrapper div to be rendered.
- if (buttons) {
+ if (buttons.length) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (buttons) { | |
| var buttonsWrapperElementOptions = { | |
| className: 'frm-overlay--cta frm-flex-box', | |
| children: buttons | |
| }; | |
| return frmDom.div(buttonsWrapperElementOptions); | |
| } | |
| return ''; | |
| } | |
| if (buttons.length) { | |
| var buttonsWrapperElementOptions = { | |
| className: 'frm-overlay--cta frm-flex-box', | |
| children: buttons | |
| }; | |
| return frmDom.div(buttonsWrapperElementOptions); | |
| } | |
| return ''; | |
| } |
🤖 Prompt for AI Agents
In js/formidable_overlay.js around lines 119 to 127, the condition `if
(buttons)` is always truthy because `buttons` is the result of `.map()` (an
array) and will render an empty wrapper even when no buttons exist; change the
guard to check for non-empty array (e.g., `if (buttons && buttons.length > 0)`)
so the wrapper div is only created when there are actual button elements,
otherwise return the empty string as before.
js/formidable_overlay.js
Outdated
| var addToRequestQueue = function addToRequestQueue(task) { | ||
| return lastPromise = lastPromise.then(task).catch(task); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unusual error handling in request queue may cause unexpected behavior.
When a promise rejects, .catch(task) calls the task function with the rejection reason as its argument. Most task functions likely expect no arguments or specific arguments, not error objects. This could lead to silent failures or unexpected behavior.
Consider using a proper error handler:
var addToRequestQueue = function addToRequestQueue(task) {
- return lastPromise = lastPromise.then(task).catch(task);
+ return lastPromise = lastPromise.then(task).catch(function(error) {
+ console.error('Request queue error:', error);
+ return task(); // Retry or skip based on requirements
+ });
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var addToRequestQueue = function addToRequestQueue(task) { | |
| return lastPromise = lastPromise.then(task).catch(task); | |
| }; | |
| var addToRequestQueue = function addToRequestQueue(task) { | |
| return lastPromise = lastPromise.then(task).catch(function(error) { | |
| console.error('Request queue error:', error); | |
| return task(); // Retry or skip based on requirements | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In js/formidable_overlay.js around lines 346-348, the request-queue catch
currently forwards the rejection value into task by using .catch(task), which
passes an error object as an argument to task; change the error handling so that
task is invoked without the rejection reason (e.g., .catch(() => task()) or
.catch(() => Promise.resolve().then(task))) and optionally log the error before
calling task, ensuring lastPromise is assigned to the recovered promise chain
and that rejected values are not passed as unintended arguments to task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
js/src/admin/admin.js (1)
6763-6801:maybeBreakFieldGrouphas critical bugs that silently fail to apply class changes and break drag/drop behaviorThe field-group splitting logic has several correctness issues that prevent intended changes from being applied to the visible DOM:
Class updates applied to detached, invisible node (Line 6798 + 6801):
maybeBreakFieldGroupremoves the original field viafieldsInFieldGroup.slice( indexOfTargetField ).map( field => field.remove() ).changeFieldClassthen callsfield.className = field.className.replace( ... )on the now-detached original field.- The visible field (inside the cloned
newFieldGroup) never receives the class update, sofrm_firstsilently fails to apply.New field group loses drag/drop behavior (Line 6816):
fieldGroup.cloneNode( true )copies DOM structure but not jQuery UI event handlers or data.maybeBreakFieldGroupnever callsmakeDroppable/makeDraggableonnewFieldGroup, leaving the new group non-interactive in the builder.Unprotected null dereference (Line 6814):
.closest( 'li.frm_field_box.ui-draggable' )can returnnullif the group lacks theui-draggableclass, causingfieldGroup.cloneNode( true )to crash at runtime.Unnecessary empty groups when target is first/only field (Line 6813):
- When
indexOfTargetField <= 0or the group has one field, the code still clones and splits, wasting DOM nodes.The fix requires returning the (possibly moved) field node from
maybeBreakFieldGroupsochangeFieldClassupdates the visible element, moving existing nodes instead of cloning to preserve event bindings, and re-applyingmakeDroppable/makeDraggableto the new group. Edge cases (null fieldGroup, first field, single-field groups) should short-circuit to avoid splits and crashes.
🧹 Nitpick comments (1)
js/src/admin/admin.js (1)
3257-3279: liveChanges signature change is backward‑compatible but semantics should be documentedExtending
liveChangestoliveChanges( event, shouldTryBreakFieldGroup = true )and threading that flag intochangeFieldClassis backward‑compatible for existing callers, and the defaulttruekeeps prior behavior unless explicitly overridden (as fromgetSyncLayoutClass). Consider adding a short JSDoc onliveChangesnoting the meaning ofshouldTryBreakFieldGroupso future internal callers don’t accidentally opt into group‑breaking when they mean to do a pure visual sync.
📜 Review details
Configuration used: Organization 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 (15)
css/admin/frm-settings-components.csscss/frm_admin.csscss/frm_testing_mode.cssjs/addons-page.jsjs/form-templates.jsjs/formidable-settings-components.jsjs/formidable_admin.jsjs/formidable_blocks.jsjs/formidable_dashboard.jsjs/formidable_overlay.jsjs/formidable_styles.jsjs/frm_testing_mode.jsjs/onboarding-wizard.jsjs/src/admin/admin.jsjs/welcome-tour.js
🚧 Files skipped from review as they are similar to previous changes (1)
- css/admin/frm-settings-components.css
🧰 Additional context used
🧬 Code graph analysis (3)
js/formidable_dashboard.js (1)
js/src/admin/addon-state.js (2)
frmDom(3-3)a(198-198)
js/formidable_overlay.js (3)
js/src/admin/admin.js (41)
e(8305-8305)t(718-718)t(743-743)t(9092-9092)t(10283-10283)frmDom(241-241)frmDom(242-242)frmDom(243-243)a(3098-3098)i(440-440)i(1798-1798)i(2283-2283)i(3035-3035)i(3050-3050)i(3122-3122)i(3140-3140)i(3154-3157)i(3180-3188)i(3218-3218)i(3227-3227)i(3488-3494)i(3630-3633)i(5480-5480)i(5549-5550)i(5624-5626)c(723-723)c(748-748)c(7343-7343)c(9168-9168)s(276-276)s(8311-8311)s(9068-9068)s(9145-9145)f(9107-9107)v(3394-3394)v(3414-3414)v(6975-6975)v(7347-7347)v(7898-7898)v(7914-7914)v(7930-7930)js/src/admin/upgrade-popup.js (1)
frmDom(3-3)js/src/admin/addon-state.js (2)
frmDom(3-3)a(198-198)
js/frm_testing_mode.js (1)
js/admin/style.js (5)
frmDom(23-23)frmDom(24-24)frmDom(25-25)frmDom(26-26)frmDom(1164-1164)
🪛 Biome (2.1.2)
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)
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.
min-width 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_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/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_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/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/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/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 (11)
css/frm_testing_mode.css (1)
2-3: No CSS duplication found in this file.The Bootstrap tooltip CSS block appears only once in the stylesheet. The comment closure (
*/) at line 3 correctly terminates the Bootstrap v4.6.1 comment block, and the minified CSS rules that follow contain no duplicate class definitions. Properties liketext-align,min-width, andborder-radiusappear multiple times only because they belong to different CSS selectors—this is normal CSS structure, not duplication.js/welcome-tour.js (1)
1-2: Minified bundle generated by webpack; Biome errors are false positives.This file is the minified output from webpack (built from
js/src/welcome-tour/). Unlike the original comment suggested, this project intentionally commits built bundles to git—the.gitignoreonly excludesjs/*.js.LICENSE.txtfiles, not the compiled.jsfiles themselves.The Biome static analysis errors (switch fallthrough, function reassignments, label conflicts) are expected false positives inherent to minified code and should not drive code review decisions. No action needed.
js/formidable_styles.js (1)
1-1: Minified build artifact with no functional changes.This file is a minified production build. The AI summary confirms no functional changes were detected—only formatting/whitespace adjustments. The static analysis warnings about variable hoisting and function reassignment are expected patterns in minified code and are false positives.
js/addons-page.js (1)
1-1: Minified build artifact with no functional changes.This minified production build shows no functional changes according to the summary. The static analysis warnings are expected minified code patterns.
js/onboarding-wizard.js (1)
2-2: Internal identifier renaming in minified build.The summary indicates only internal parameter identifier changes (e.g., n → r) with no changes to control flow or behavior. The static analysis warnings are false positives for minified code.
js/formidable-settings-components.js (1)
1-1: Structural reflow in minified build with no functional impact.This is a formatting/structural reflow with no functional changes. Static analysis warnings are expected for minified production code.
js/formidable_dashboard.js (1)
1-1: Formatting changes in minified build.The summary confirms only minor formatting/alignment changes with no alterations to runtime behavior, control flow, or logic. The changes appear to be from a rebuild process.
js/frm_testing_mode.js (1)
2-2: Internal parameter renaming in minified build.This minified build shows only internal parameter name changes and minor operand reordering with no behavioral changes.
js/formidable_overlay.js (1)
1-1: Minified variant replacement with preserved behavior.The summary indicates this is a minified variant that preserves the same public surface and behavior. No functional changes to public APIs or event handling.
js/formidable_blocks.js (1)
1-1: Export binding change in minified build.The export name remains
Aybut the internal binding changed fromitol. According to the summary, this is an internal-only change with no impact on external consumers. The functional CSS/class content remains unchanged.js/src/admin/admin.js (1)
1466-1518: Change-event gating for layout class sync is soundPassing
[ false ]into thechangetrigger onlayoutClassesInputcleanly distinguishes programmatic sync from user-initiated edits, so the newshouldTryBreakFieldGroupflag can short‑circuit group‑breaking logic without altering existing handlers. No functional issues spotted here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
js/src/admin/admin.js (1)
6763-6801: Field group splitting currently updates the wrong DOM node (classes applied to detached element)Right now
maybeBreakFieldGroupremoves the original field element from the DOM and keeps a cloned copy in the new group:
fieldGroup.cloneNode( true )creates a full clone (including the target.form-fieldand its siblings).fieldsInFieldGroup.slice( indexOfTargetField ).map( field => field.remove() );removes the original target field and all following siblings from the original UL.- After inserting the cloned
newFieldGroup, you only remove the cloned predecessors, so the live DOM for the target field is the cloned node, not the original one referenced byfield.However,
changeFieldClassthen does:maybeBreakFieldGroup( !! shouldTryBreakFieldGroup, field, classes, replaceWith ); field.className = field.className.replace( replace, replaceWith );so the class replacement runs on the original (now detached) node, while the visible cloned node in the form builder keeps the old classes (and will not gain the new
frm_first). That makes the new “break group whenfrm_firstis added” behavior unreliable and hard to debug.Two concrete improvements are needed here:
Fix correctness: ensure the live DOM node gets updated
Minimal change inside
changeFieldClass:
maybeBreakFieldGroup( !! shouldTryBreakFieldGroup, field, classes, replaceWith );field.className = field.className.replace( replace, replaceWith );
maybeBreakFieldGroup( !! shouldTryBreakFieldGroup, field, classes, replaceWith );// After maybeBreakFieldGroup runs, the original element may have been// replaced by a clone in the DOM. Always update the live element.if ( field.id ) {const liveField = document.getElementById( field.id );if ( liveField ) {field = liveField;}}field.className = field.className.replace( replace, replaceWith );
Prefer moving existing children instead of cloning the entire group
Cloning the group (
cloneNode( true )) duplicates all descendant nodes and their runtime state; you then partially remove originals and clones. Even with the above fix, this risks losing non‑delegated listeners and any state attached to the original nodes. A more robust approach is to:
- Clone only the wrapper structure (e.g.,
fieldGroup.cloneNode( false )andfield.parentElement.cloneNode( false )).- Move
fieldsInFieldGroup.slice( indexOfTargetField )into the new UL withappendChildinstead of cloning.- Then initialize droppable/dragging on the new UL.
That keeps field identity intact and avoids subtle regressions in other builder behaviors.
Given this touches the core builder layout DOM, I’d treat fixing this before release as high priority.
🧹 Nitpick comments (1)
js/src/admin/admin.js (1)
6803-6828: Guardrails and lint for maybeBreakFieldGroupFunctionally,
maybeBreakFieldGroupshould also defensively bail out when the structure is not what you expect:
- If
field.parentElementorfield.parentElement.closest( 'li.frm_field_box.ui-draggable' )isnull, this will currently throw. A simple early‑return guard keeps things safe whenfrm_firstis applied outside of a standard field group.- If the group has only one child, splitting does nothing useful; short‑circuiting on
fieldsInFieldGroup.length <= 1avoids creating empty wrappers.On the style side, ESLint is flagging the JSDoc alignment here (
Expected JSDoc block lines to be aligned). Re‑aligning the*and fixing the param types (e.g.,@param {boolean} shouldTryBreakFieldGroup) will clear the warning without behavioral change.You may also want to replace
.map( field => field.remove() )with.forEach( field => field.remove() )to better express the side‑effect intent.
📜 Review details
Configuration used: Organization 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 (2)
js/formidable_admin.jsjs/src/admin/admin.js
🧰 Additional context used
🪛 GitHub Check: Run ESLint
js/src/admin/admin.js
[warning] 6803-6803:
Expected JSDoc block lines to be aligned
🔇 Additional comments (2)
js/src/admin/admin.js (2)
1466-1518: Good use of an extra argument to gate side‑effects from auto layout syncPassing
falseas the second arg injQuery( layoutClassesInput ).trigger( 'change', [ false ] );cleanly distinguishes programmatic layout sync from user‑initiated class edits, soliveChangescan avoid attempting to break field groups in the former case. This looks correct and backwards‑compatible with otherchangelisteners that ignore extra args.
3257-3279: liveChanges new parameter is fine but relies on all internal trigger sites being updatedThe new
shouldTryBreakFieldGroup = trueparameter andchangeFieldClass( changes, this, shouldTryBreakFieldGroup );wiring are consistent, and the only explicit internal trigger (getSyncLayoutClass) now passes[ false ], so behavior should be gated as intended. Please double‑check that no external/custom code callsliveChangesdirectly with a different second argument that might now inadvertently control the group‑breaking behavior.
Fix https://github.com/Strategy11/formidable-pro/issues/5228
CleanShot.2025-12-25.at.21.45.52.mp4