You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Checkbox and radio field alignment options are now customizable through filters (frm_checkbox_align_options and frm_radio_align_options), allowing alignment labels to be modified.
Refactor
Improved alignment handling logic for checkbox and radio fields with better code organization and maintainability.
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d7ce8773-5192-4ecb-bd35-940ebf48470d
📥 Commits
Reviewing files that changed from the base of the PR and between fa86fd5 and d0c6146.
📒 Files selected for processing (3)
classes/models/fields/FrmFieldCheckbox.php
classes/models/fields/FrmFieldRadio.php
classes/models/fields/FrmFieldType.php
💤 Files with no reviewable changes (3)
classes/models/fields/FrmFieldCheckbox.php
classes/models/fields/FrmFieldRadio.php
classes/models/fields/FrmFieldType.php
📝 Walkthrough
Walkthrough
This PR refactors alignment handling for checkbox and radio fields by removing form_id-dependent computation from the field models and consolidating normalization logic in the field type container class. Checkbox and Radio extra_field_opts() now return empty align values; the field type layer is updated to always apply alignment normalization, even when align is empty.
Changes
Alignment Handling Refactor
Layer / File(s)
Summary
Field model alignment simplification classes/models/fields/FrmFieldCheckbox.php, classes/models/fields/FrmFieldRadio.php
Checkbox and Radio extra_field_opts() methods are simplified to return empty string for align option, removing previous form_id reads and FrmStylesController::get_style_val() calls.
Container class alignment normalization classes/models/fields/FrmFieldType.php
get_container_class() is updated to always apply prepare_align_class() normalization for radio/checkbox fields via instance call, handling empty align values correctly.
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
🐰 Fields once dressed in form IDs and style flow,
Now wear alignment plain, both high and low.
The normalization step takes the lead,
Converting empty strings to what is decreed! ✨
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name
Status
Explanation
Resolution
Docstring Coverage
⚠️ Warning
Docstring coverage is 1.21% which is insufficient. The required threshold is 80.00%.
Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name
Status
Explanation
Description Check
✅ Passed
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
✅ Passed
The title 'Add new align option hooks' directly and accurately describes the main change: introducing new filterable hooks (frm_checkbox_align_options and frm_radio_align_options) for alignment options in checkbox and radio fields.
Linked Issues check
✅ Passed
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
✅ Passed
Check skipped because no linked issues were found for this pull request.
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches📝 Generate docstrings
Create stacked PR
Commit on current branch
🧪 Generate unit tests (beta)
Create PR with unit tests
Commit unit tests in branch new_align_option_hooks
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.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (8)
css/admin/frm-settings-components.css (1)
1-1: Duplicate style block for formidable styles admin
This entire rule set appears to duplicate existing styles for the formidable_page_formidable-styles admin page (color picker, background-image component, .frm-align-component radio toggles, sliders, tabs, unit inputs, etc.), which doesn’t change behavior but does bloat the CSS.
If the duplication comes from the source SCSS/CSS or build pipeline, consider deduplicating there so the compiled stylesheet only includes one copy of these rules.
js/formidable_overlay.js (1)
1-1: Bundled overlay script change looks safe
This is transpiled/minified overlay code with no observable behavioral changes; the animation and overlay-opening logic remain intact. If Biome is flagging helper patterns here, consider scoping linting to the source files instead of built bundles rather than changing this output.
css/frm_testing_mode.css (1)
2-3: Bootstrap tooltip block relocation is fine
The .tooltip/Bootstrap block appears unchanged and only moved; existing .tooltip.frm_tooltip overrides still come later, so behavior should be preserved. The duplicate/shorthand property lint warnings are from pre-existing Bootstrap-style patterns and don’t need to be addressed in this PR unless you want to add an ignore for this file.
js/formidable_blocks.js (1)
1-1: Gutenberg blocks bundle changes appear safe
The only effective changes here are regenerated CSS-module hashes (spinner button class and keyframe name) and associated bundle boilerplate; the block logic, props, and use of ze['frm-loading'] are unchanged. Given this is compiled output, the Biome warnings about helper patterns can be handled at the tooling/config level rather than by editing this file.
classes/models/fields/FrmFieldRadio.php (1)
69-73: Align default and unused $form_id in extra_field_opts
Setting 'align' => '' aligns with moving alignment logic out of the model, but it does change the default value for new radio fields. Please confirm there’s no remaining code path that relies on align being pre-populated from styles rather than treated as “unset” and handled in the view/JS layer.
Also, $form_id is now unused here and can be safely removed.
classes/models/fields/FrmFieldCheckbox.php (1)
78-82: Checkbox align default and unused $form_id
extra_field_opts() now defaults 'align' to an empty string for checkboxes as well. That’s consistent with the new align‑option hooks, but it does rely on downstream code (styles/JS) correctly handling the “unset” case, so a quick double-check of any callers that expect a non-empty align value would be good.
$form_id is no longer used and can be removed as a small cleanup.
67-70: Filterable checkbox align options are good; please finalize docs and confirm $atts scope
The switch to a shared $align_options array plus the frm_checkbox_align_options filter is clean and makes the dropdown extensible. Two small follow-ups:
The @since x.x should be updated to the actual version before release, and you may want to add a short description and a @return array for tooling.
$atts is passed into the filter but is not defined in this file; please confirm it is always defined in the including scope, or defensively default it (e.g., to an empty array) to avoid notices.
Also applies to: 72-79, 85-85
98-105: Radio align filter reuse looks fine; consider same doc/scope tightening
Reusing the same $align_options base for radios and exposing it via frm_radio_align_options keeps behavior consistent and customizable. As with the checkbox filter:
Replace @since x.x with the concrete version and consider expanding the docblock (param descriptions and @return array).
Ensure $atts is guaranteed to exist here, or guard it, since it is passed into the filter.
Also applies to: 111-111
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between d9a590b and 220661b.
⛔ Files ignored due to path filters (9)
js/addons-page.js.map is excluded by !**/*.map, !**/*.map
js/form-templates.js.map is excluded by !**/*.map, !**/*.map
js/formidable-settings-components.js.map is excluded by !**/*.map, !**/*.map
js/formidable_admin.js.map is excluded by !**/*.map, !**/*.map
js/formidable_blocks.js.map is excluded by !**/*.map, !**/*.map
js/formidable_dashboard.js.map is excluded by !**/*.map, !**/*.map
js/formidable_overlay.js.map is excluded by !**/*.map, !**/*.map
js/formidable_styles.js.map is excluded by !**/*.map, !**/*.map
js/onboarding-wizard.js.map is excluded by !**/*.map, !**/*.map
On the formidable styles page you attach onClickPreventDefault to #frm_submit_side_top and call the async pt() handler, which marks the style-form step complete and then reloads the page. Depending on how the styles form is submitted (native form submit vs other JS handlers), this could potentially prevent the actual save from occurring.
Please double-check that:
Any existing JS-based save handler still runs as expected, and
Native form submission isn’t unintentionally suppressed in cases where no JS handler is present.
If there’s any risk here, consider triggering the checklist step completion from the form submit handler instead of blocking the button’s default behavior.
js/frm_testing_mode.js (1)
2-400: Bundled/testing-mode script looks consistent with prior behavior
This appears to be a rebuilt bundle (i18n, hooks, addon state helpers, and test-mode init) with no observable behavioral changes in the testing-mode logic itself. Given this is generated code, I’d avoid hand-editing here and keep linting focused on the unminified source instead.
The updated addons-page bundle reads as a recompiled/transpiled version of the previous implementation (filters, empty state, category counts, upgrade modal, etc.) with no clear functional differences. I’d keep any future maintenance in the original source modules and treat this file as a build artifact.
This looks like a regenerated bundle (renamed internals, same step/state logic) rather than a semantic change: step handling, history updates, consent tracking, and async addon installs all follow the existing patterns. No issues from the onboarding flow side.
js/formidable_dashboard.js (1)
1-260: Dashboard JS bundle recompile looks safe
The dashboard code (widget cascade animation, counters, inbox email, welcome banner dismissal) appears unchanged in behavior and just re-bundled. No issues spotted in the dashboard interactions.
js/formidable_styles.js (1)
1-260: Styles sidebar behavior preserved in regenerated bundle
The formidable_styles bundle (i18n, dependent updaters, colorpicker change propagation, hover highlight, copy-to-clipboard) reads as a straight rebuild with the same logic. No new issues evident; any future tweaks should ideally target the unminified source.
The rebuilt formidable-settings-components bundle preserves the existing radio tracker, slider, token-input, toggle-group, and unit-input behavior. Nothing in this file appears to change how alignment options behave; it looks like a fresh compile of the prior logic.
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue | 🟠 Major
Guard data-align fallback and use the selected option instead of the first match
The new fallback assumes:
setting always has an option[data-align], and
the first such option is the one currently selected.
This can cause issues:
Potential runtime error: If setting is a text input (e.g., .frm_classes) with an empty value, or a <select> without any option[data-align], then setting.querySelector( 'option[data-align]' ) returns null and .getAttribute( 'data-align' ) will throw.
Wrong alignment when multiple options have data-align: querySelector( 'option[data-align]' ) always returns the first matching option, not the selected one, so the applied class may not match the user’s current choice.
Over‑broad application: The fallback should only apply to the align dropdown (.field_options_align), not to every control wired through changeFieldClass.
This keeps existing behavior for non‑align inputs, avoids exceptions when no data-align is present, and correctly respects the user’s selected alignment option.
📝 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.
In js/src/admin/admin.js around lines 6734-6736, the fallback assumes
setting.querySelector('option[data-align]') exists and uses the first match;
change it so the fallback only runs when setting.matches('.field_options_align')
(the align select), then get the selected option
(setting.options[setting.selectedIndex] or
setting.querySelector('option:checked')) and read its data-align attribute; if
no selected option or no data-align is present, do nothing (no-op) to avoid
runtime errors and incorrect alignment application.
We reviewed changes in 229cf3c...d0c6146 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
The reason will be displayed to describe this comment to others. Learn more.
Variable $atts might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $frm_style might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $style might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $atts might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $frm_style might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $style might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
Required for https://github.com/Strategy11/formidable-pro/pull/6102
Test Procedure
https://github.com/Strategy11/formidable-pro/pull/6102#issue-3644154177
Summary by CodeRabbit
Release Notes
New Features
frm_checkbox_align_optionsandfrm_radio_align_options), allowing alignment labels to be modified.Refactor