Fix single row style setting not working as expected #2544

AbdiTolesa · 2025-10-15T11:50:55Z

Fix https://github.com/Strategy11/formidable-pro/issues/6035
Related PR: https://github.com/Strategy11/formidable-pro/pull/6057

Test steps

Add Checkbox and Radio fields to a form.
Go to the Styles tab and set the alignment setting to 'Single row' as in the screenshot below
Preview the form and confirm that the field choices are displayed in a Single row.

coderabbitai · 2025-10-15T11:51:27Z

Walkthrough

Mapping of 'block' alignment to the 'vertical_radio' class in FrmFieldType::prepare_align_class is now conditional based on the form-level style value 'check_align' retrieved via FrmStylesController::get_style_val; behavior unchanged when 'check_align' equals 'inline'. No public API changes.

Changes

Cohort / File(s)	Change Summary
Field alignment handling `classes/models/fields/FrmFieldType.php`	In prepare_align_class, call `FrmStylesController::get_style_val('check_align', $this->field['form_id'])` and only map alignment `'block'` to `'vertical_radio'` when the returned `check_align` value is not `'inline'`. Adds comment referencing issue #6035; preserves prior behavior when `check_align` is `'inline'`.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant FR as Form Renderer
    participant FT as FrmFieldType.prepare_align_class
    participant SC as FrmStylesController

    U->>FR: Render form
    FR->>FT: Request field align class
    FT->>SC: get_style_val('check_align', field.form_id)
    SC-->>FT: check_align value

    alt check_align != "inline"
        note right of FT #DFF2DF: style permits block override
        FT-->>FR: return "vertical_radio"
    else
        note right of FT #FFF4D6: preserve existing mapping
        FT-->>FR: return existing alignment class
    end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Add class required for single column layout in builder when One column layout is selected #2469 — also adjusts handling of "block" alignment mapping to vertical_radio; likely related to runtime/style-driven alignment behavior.

Suggested labels

action: needs qa, run analysis, run tests

Suggested reviewers

lauramekaj1
truongwp
Crabcyborg

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "Fix single row style setting not working as expected" directly and accurately describes the primary change in this pull request. The code modification specifically addresses the single row style setting for checkbox and radio fields by gating the block alignment override with a runtime style check, which aligns perfectly with the title's focus on fixing the single row style setting. The title is concise, specific, and clearly communicates the main objective without unnecessary details.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	The pull request description is clearly related to the changeset. It identifies the specific issue being fixed (#6035), provides concrete test steps for validating the fix (adding checkbox and radio fields, setting alignment to "Single row" in the Styles tab, and confirming the display), and includes a supporting screenshot. The description aligns well with the code changes, which modify the block alignment override logic in FrmFieldType::prepare_align_class to properly respect the check_align style setting. While the description does not explain the implementation details, the criteria specifies that level of detail is not important for passing this check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch issue-6035-fix_single_row_style_not_working

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c259b6d and 03f578c.

📒 Files selected for processing (1)

classes/models/fields/FrmFieldType.php (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

classes/models/fields/FrmFieldType.php (1)

classes/controllers/FrmStylesController.php (2)

FrmStylesController (6-1362)

get_style_val (1178-1183)

🔇 Additional comments (1)

classes/models/fields/FrmFieldType.php (1)

985-989: Good fix addressing the single row style issue.

The conditional check correctly gates the 'vertical_radio' mapping based on the form-level style value. This ensures that the single row style setting takes effect as intended. The logic properly handles the case where get_style_val returns null (when the style value is not set), as the strict comparison will evaluate to false.

coderabbitai · 2025-10-15T11:56:23Z

classes/models/fields/FrmFieldType.php

+			$check_align = FrmStylesController::get_style_val( 'check_align', $this->field['form_id'] );
+			// Check needed to fix issue #6035


⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify that form_id exists before using it.

$this->field['form_id'] is accessed without checking if it exists. If the field array doesn't have a form_id key, this will trigger an undefined index notice.

Run the following script to check if there are safeguards elsewhere or if this can occur:

🏁 Script executed:

#!/bin/bash # Description: Check how prepare_align_class is called and if form_id is always set # Search for calls to prepare_align_class to understand the context rg -nP --type=php -C5 '\bprepare_align_class\b' # Search for where $this->field is populated to see if form_id is always set ast-grep --pattern $'function __construct($field = $_, $type = $_) { $$$ }' # Check if there are other places where $this->field['form_id'] is accessed rg -nP --type=php "\\\$this->field\['form_id'\]"

Length of output: 1862

🏁 Script executed:

#!/bin/bash # Check FrmFieldType class and its constructor to ensure form_id is initialized rg -nP --type=php 'class\s+FrmFieldType\b' -C5 classes/models/fields/FrmFieldType.php rg -nP --type=php 'function __construct' -C10 classes/models/fields/FrmFieldType.php

Length of output: 562

🏁 Script executed:

#!/bin/bash # List all field-type subclass files fd --type f 'FrmField*.php' classes/models/fields # Find where field-type objects are instantiated rg -nP 'new\s+FrmField\w+\s*\(' -n classes

Length of output: 776

🏁 Script executed:

#!/bin/bash # Show context for FrmFieldDefault instantiation rg -nP -C5 'new\s+FrmFieldDefault' -n classes/factories/FrmFieldFactory.php # Find assignment to $field in the factory rg -nP -C5 '\$field\s*=' -n classes/factories/FrmFieldFactory.php

Length of output: 1300

🏁 Script executed:

rg -nP --type=php 'function\s+getOne' -C10 classes/models/FrmField.php

Length of output: 741

Handle both object and array when retrieving form_id.
In prepare_align_class, replace

$check_align = FrmStylesController::get_style_val( 'check_align', $this->field['form_id'] );

with something like:

$form_id = is_object( $this->field ) ? $this->field->form_id : ( $this->field['form_id'] ?? null ); $check_align = FrmStylesController::get_style_val( 'check_align', $form_id );

[this prevents fatal errors for object‐typed fields and undefined index notices]

🤖 Prompt for AI Agents

In classes/models/fields/FrmFieldType.php around lines 985 to 986, the code assumes $this->field is an array and accesses $this->field['form_id'], which can cause fatal errors for object-typed fields or undefined index notices; update the code to normalize $form_id first by checking is_object($this->field) and using $this->field->form_id if so, otherwise use $this->field['form_id'] if set (or null), then pass that normalized $form_id into FrmStylesController::get_style_val('check_align', $form_id).

lauramekaj1 · 2025-10-20T21:03:29Z

@AbdiTolesa I tested this and noticed the following issues:

After adding radio and checkbox fields and saving the form, an error appears on the Build Form page (https://imgur.com/dLutuMk).
A console error occurs when I navigate to the Styles page and click 'Edit' on a style (https://imgur.com/FDVUtC7).
When switching from the Single Row to the Multiple Row option and updating the form, one of the checkboxes is not aligning correctly with the new layout (https://www.loom.com/share/673e1e8e5e1d4e898804c49857433878).

AbdiTolesa · 2025-10-21T11:49:24Z

After adding radio and checkbox fields and saving the form, an error appears on the Build Form page (https://imgur.com/dLutuMk).

@lauramekaj1 This one should be fixed now.

AbdiTolesa · 2025-10-21T11:50:41Z

3. When switching from the Single Row to the Multiple Row option and updating the form, one of the checkboxes is not aligning correctly with the new layout

@lauramekaj1 I couldn't reproduce this one but I already have another PR that is needed to make the alignment work properly in the builder. https://github.com/Strategy11/formidable-pro/pull/6057

Could you check out this branch and the Pro part and test if this issue is gone?

AbdiTolesa · 2025-10-21T11:51:57Z

A console error occurs when I navigate to the Styles page and click 'Edit' on a style (https://imgur.com/FDVUtC7).

@lauramekaj1 This fix just focuses on fixing the Single vs Multiple rows layout for field choices and I can reproduce that issue on the master branch too. Can we create another issue to track that?

lauramekaj1

@AbdiTolesa I tested this and verified that the issue is fixed. Thank you!

truongwp · 2025-10-31T18:08:07Z

@Crabcyborg If I set to "Single Row" in the Styles then set to "One Column" in the field setting, it prefers the Styles setting. Is that the right behavior?
I think we should add a "Use Styles setting" option to the dropdown in field settings.

AbdiTolesa · 2025-11-03T11:35:02Z

@Crabcyborg If I set to "Single Row" in the Styles then set to "One Column" in the field setting, it prefers the Styles setting. Is that the right behavior? I think we should add a "Use Styles setting" option to the dropdown in field settings.

I think we might not add additional option to resolve conflict that might happen between the two settings (Style & Option Layout) since users could adjust update either of those two settings to make it display as they wish in my opinion.

truongwp · 2025-11-04T20:12:13Z

@AbdiTolesa Currently if I set the field setting to any value except "One Column", it always uses the field setting.

AbdiTolesa · 2025-11-05T11:11:34Z

@AbdiTolesa Currently if I set the field setting to any value except "One Column", it always uses the field setting.

@truongwp The more I think about those two settings, the more I feel uncertain about their purpose since their purpose seem to overlap. Both of them can be used to make the options in single row vs multiple rows but can also be conflicting with each other easily.

truongwp · 2025-11-05T15:02:24Z

@Crabcyborg Can you give your opinion?

Crabcyborg · 2025-11-06T15:38:45Z

Thanks @AbdiTolesa and @truongwp.

These two settings seem to directly conflict.

I'm bringing this up with Steve, Dalton and Nat, to see what they think.

I think we might want to overhaul these settings a bit.

Crabcyborg · 2025-11-19T19:41:43Z

@truongwp @AbdiTolesa

We're going to add some new options to both the style and field settings.

I have some related PRs where I started working on this. https://github.com/Strategy11/formidable-pro/pull/6102 and #2593. Most of the details to get started are in https://github.com/Strategy11/formidable-pro/pull/6102, and below.

This still needs a lot of work. @AbdiTolesa, if you can take these over again, that would be great.

What we need to make sure works

Any one with the field setting set will use the field setting, needing to ignore whatever the style setting is (removing all of those other classes).
The new options for style settings will need to work. These are the three Two Columns, Three Columns, and Four Columns options. I've renamed some options for consistency, so "Multiple Rows" is now "One Column" and "Single Row" is now "Inline Options", but the value and behaviour for those should remain the same.
I made one change, but we'll likely need others that make sure when $field['align'] is empty (When "Use Style" is selected), that we use the style settings.

Thank you!

AbdiTolesa · 2025-11-20T14:10:06Z

This still needs a lot of work. @AbdiTolesa, if you can take these over again, that would be great.

@Crabcyborg Sure, working on it.

Fix single row style setting not making any effect

03f578c

AbdiTolesa changed the title ~~Fix single row style setting not making any effect~~ Fix single row style setting not working as expected Oct 15, 2025

Make code self explanatory

5fa338f

coderabbitai bot reviewed Oct 15, 2025

View reviewed changes

AbdiTolesa requested a review from lauramekaj1 October 15, 2025 11:58

Prevent warning when adding fields in the builder

cb98046

lauramekaj1 approved these changes Oct 30, 2025

View reviewed changes

AbdiTolesa requested a review from truongwp October 31, 2025 11:36

		$check_align = FrmStylesController::get_style_val( 'check_align', $this->field['form_id'] );
		// Check needed to fix issue #6035

Fix single row style setting not working as expected #2544

Are you sure you want to change the base?

Fix single row style setting not working as expected #2544

Uh oh!

Conversation

AbdiTolesa commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test steps

Uh oh!

coderabbitai bot commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

lauramekaj1 commented Oct 20, 2025

Uh oh!

AbdiTolesa commented Oct 21, 2025

Uh oh!

AbdiTolesa commented Oct 21, 2025

Uh oh!

AbdiTolesa commented Oct 21, 2025

Uh oh!

lauramekaj1 left a comment

Choose a reason for hiding this comment

Uh oh!

truongwp commented Oct 31, 2025

Uh oh!

AbdiTolesa commented Nov 3, 2025

Uh oh!

truongwp commented Nov 4, 2025

Uh oh!

AbdiTolesa commented Nov 5, 2025

Uh oh!

truongwp commented Nov 5, 2025

Uh oh!

Crabcyborg commented Nov 6, 2025

Uh oh!

Crabcyborg commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AbdiTolesa commented Nov 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AbdiTolesa commented Oct 15, 2025 •

edited

Loading

coderabbitai bot commented Oct 15, 2025 •

edited

Loading

Crabcyborg commented Nov 19, 2025 •

edited

Loading