From f161b676f5a3ecbed8b0c809f23645811d81eed0 Mon Sep 17 00:00:00 2001 From: Sherv Date: Tue, 30 Dec 2025 16:08:16 +0300 Subject: [PATCH 1/6] Put back check for section_helper but add special handling for divider fields to check section_size before opening section --- classes/helpers/FrmFieldGridHelper.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/classes/helpers/FrmFieldGridHelper.php b/classes/helpers/FrmFieldGridHelper.php index d41e0c47c8..1d130e1329 100644 --- a/classes/helpers/FrmFieldGridHelper.php +++ b/classes/helpers/FrmFieldGridHelper.php @@ -155,13 +155,25 @@ public function maybe_begin_field_wrapper() { * @return bool */ private function should_first_close_the_active_field_wrapper() { - if ( false === $this->parent_li || ! empty( $this->section_helper ) ) { + if ( false === $this->parent_li ) { + return false; + } + + // Fix for issue: When at a divider field itself (section_helper exists but section not yet open), + // check if the section fits in the current row based on section_size. + if ( 'divider' === $this->field->type && ! $this->section_is_open ) { + return $this->current_list_size + $this->section_size > 12; + } + + // Inside an open section - let section_helper handle grouping. + if ( ! empty( $this->section_helper ) ) { return false; } if ( 'end_divider' === $this->field->type ) { return false; } + return ! $this->can_support_current_layout() || $this->is_frm_first; } From 03ec7767924c084a31517d58986f7a682fdfde35 Mon Sep 17 00:00:00 2001 From: Sherv Date: Fri, 2 Jan 2026 21:51:57 +0300 Subject: [PATCH 2/6] Update comments in FrmFieldGridHelper for clarity --- classes/helpers/FrmFieldGridHelper.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/classes/helpers/FrmFieldGridHelper.php b/classes/helpers/FrmFieldGridHelper.php index 1d130e1329..bf0c80b4b3 100644 --- a/classes/helpers/FrmFieldGridHelper.php +++ b/classes/helpers/FrmFieldGridHelper.php @@ -159,13 +159,13 @@ private function should_first_close_the_active_field_wrapper() { return false; } - // Fix for issue: When at a divider field itself (section_helper exists but section not yet open), - // check if the section fits in the current row based on section_size. + // Close the current row before a section if it would overflow the grid. + // @see https://github.com/Strategy11/formidable-pro/issues/3820 if ( 'divider' === $this->field->type && ! $this->section_is_open ) { return $this->current_list_size + $this->section_size > 12; } - // Inside an open section - let section_helper handle grouping. + // When a section is open, let section_helper handle field grouping. if ( ! empty( $this->section_helper ) ) { return false; } From 9b46227d880a62b134b8c3ff4e123633b1709b55 Mon Sep 17 00:00:00 2001 From: Sherv Date: Fri, 2 Jan 2026 23:52:25 +0300 Subject: [PATCH 3/6] Fix field grouping logic to respect frm_first class and prevent unwanted row wrapping --- classes/helpers/FrmFieldGridHelper.php | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/classes/helpers/FrmFieldGridHelper.php b/classes/helpers/FrmFieldGridHelper.php index bf0c80b4b3..b95fde74e7 100644 --- a/classes/helpers/FrmFieldGridHelper.php +++ b/classes/helpers/FrmFieldGridHelper.php @@ -60,6 +60,17 @@ class FrmFieldGridHelper { */ private $section_is_open = false; + /** + * Indicates the row was intentionally grouped. Set when the first field in a row has frm_first. + * + * Example: Two fields grouped in one row. + * - First field has frm_first → is_grouped_row = true. + * - Second field has no frm_first, but is_grouped_row is true → stays in the same row. + * + * @var bool + */ + private $is_grouped_row = false; + /** * @param bool $nested */ @@ -159,10 +170,11 @@ private function should_first_close_the_active_field_wrapper() { return false; } - // Close the current row before a section if it would overflow the grid. + // Handle sections (dividers) before section_helper check. + // section_helper is created in set_field() before this runs. // @see https://github.com/Strategy11/formidable-pro/issues/3820 if ( 'divider' === $this->field->type && ! $this->section_is_open ) { - return $this->current_list_size + $this->section_size > 12; + return $this->current_list_size + $this->section_size > 12 || $this->is_frm_first || ! $this->is_grouped_row; } // When a section is open, let section_helper handle field grouping. @@ -174,7 +186,7 @@ private function should_first_close_the_active_field_wrapper() { return false; } - return ! $this->can_support_current_layout() || $this->is_frm_first; + return ! $this->can_support_current_layout() || $this->is_frm_first || ! $this->is_grouped_row; } /** @@ -185,6 +197,7 @@ private function begin_field_wrapper() { $this->parent_li = true; $this->current_list_size = 0; $this->current_field_count = 0; + $this->is_grouped_row = $this->is_frm_first; } /** From 1409c534346418f871292647dfb8ecacdb8dd830 Mon Sep 17 00:00:00 2001 From: Sherv Date: Mon, 5 Jan 2026 21:57:22 +0300 Subject: [PATCH 4/6] Rename is_grouped_row to is_group_start and add is_section_start tracking --- classes/helpers/FrmFieldGridHelper.php | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/classes/helpers/FrmFieldGridHelper.php b/classes/helpers/FrmFieldGridHelper.php index b95fde74e7..0a97bec554 100644 --- a/classes/helpers/FrmFieldGridHelper.php +++ b/classes/helpers/FrmFieldGridHelper.php @@ -61,15 +61,22 @@ class FrmFieldGridHelper { private $section_is_open = false; /** - * Indicates the row was intentionally grouped. Set when the first field in a row has frm_first. + * True if the first field in the row had frm_first, indicating an intentional group. * * Example: Two fields grouped in one row. - * - First field has frm_first → is_grouped_row = true. - * - Second field has no frm_first, but is_grouped_row is true → stays in the same row. + * - First field has frm_first → is_group_start = true. + * - Second field has no frm_first, but is_group_start is true → stays in the same row. * * @var bool */ - private $is_grouped_row = false; + private $is_group_start = false; + + /** + * True if the first field in the row was a section (divider). + * + * @var bool + */ + private $is_section_start = false; /** * @param bool $nested @@ -174,7 +181,7 @@ private function should_first_close_the_active_field_wrapper() { // section_helper is created in set_field() before this runs. // @see https://github.com/Strategy11/formidable-pro/issues/3820 if ( 'divider' === $this->field->type && ! $this->section_is_open ) { - return $this->current_list_size + $this->section_size > 12 || $this->is_frm_first || ! $this->is_grouped_row; + return $this->current_list_size + $this->section_size > 12 || $this->is_frm_first || ! $this->is_group_start; } // When a section is open, let section_helper handle field grouping. @@ -186,7 +193,7 @@ private function should_first_close_the_active_field_wrapper() { return false; } - return ! $this->can_support_current_layout() || $this->is_frm_first || ! $this->is_grouped_row; + return ! $this->can_support_current_layout() || $this->is_frm_first || ( $this->is_section_start && ! $this->is_group_start ); } /** @@ -197,7 +204,8 @@ private function begin_field_wrapper() { $this->parent_li = true; $this->current_list_size = 0; $this->current_field_count = 0; - $this->is_grouped_row = $this->is_frm_first; + $this->is_group_start = $this->is_frm_first; + $this->is_section_start = 'divider' === $this->field->type; } /** From 4707c6aa896533ec338675ac5335a16a64b49a3a Mon Sep 17 00:00:00 2001 From: Sherv Date: Mon, 5 Jan 2026 22:03:53 +0300 Subject: [PATCH 5/6] Close row before section if it would overflow the grid --- classes/helpers/FrmFieldGridHelper.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/classes/helpers/FrmFieldGridHelper.php b/classes/helpers/FrmFieldGridHelper.php index 0a97bec554..bc692635ce 100644 --- a/classes/helpers/FrmFieldGridHelper.php +++ b/classes/helpers/FrmFieldGridHelper.php @@ -177,14 +177,12 @@ private function should_first_close_the_active_field_wrapper() { return false; } - // Handle sections (dividers) before section_helper check. - // section_helper is created in set_field() before this runs. + // Close the current row before a section if it would overflow the grid. // @see https://github.com/Strategy11/formidable-pro/issues/3820 if ( 'divider' === $this->field->type && ! $this->section_is_open ) { return $this->current_list_size + $this->section_size > 12 || $this->is_frm_first || ! $this->is_group_start; } - // When a section is open, let section_helper handle field grouping. if ( ! empty( $this->section_helper ) ) { return false; } From 25bab5a7705a71f1c6629ffbdfcb7ef9ed0a151b Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Tue, 25 Aug 2026 09:22:40 -0300 Subject: [PATCH 6/6] Get unit tests passing --- classes/helpers/FrmFieldGridHelper.php | 46 +++++++------------ ...er.php.php => test_FrmFieldGridHelper.php} | 17 +++---- 2 files changed, 23 insertions(+), 40 deletions(-) rename tests/phpunit/fields/{test_FrmFieldGridHelper.php.php => test_FrmFieldGridHelper.php} (94%) diff --git a/classes/helpers/FrmFieldGridHelper.php b/classes/helpers/FrmFieldGridHelper.php index 94dd036395..527a7693af 100644 --- a/classes/helpers/FrmFieldGridHelper.php +++ b/classes/helpers/FrmFieldGridHelper.php @@ -60,24 +60,6 @@ class FrmFieldGridHelper { */ private $section_is_open = false; - /** - * True if the first field in the row had frm_first, indicating an intentional group. - * - * Example: Two fields grouped in one row. - * - First field has frm_first → is_group_start = true. - * - Second field has no frm_first, but is_group_start is true → stays in the same row. - * - * @var bool - */ - private $is_group_start = false; - - /** - * True if the first field in the row was a section (divider). - * - * @var bool - */ - private $is_section_start = false; - /** * @param bool $nested */ @@ -178,13 +160,16 @@ private function should_first_close_the_active_field_wrapper() { return false; } - // Close the current row before a section if it would overflow the grid. - // @see https://github.com/Strategy11/formidable-pro/issues/3820 - if ( 'divider' === $this->field->type && ! $this->section_is_open ) { - return $this->current_list_size + $this->section_size > 12 || $this->is_frm_first || ! $this->is_group_start; - } - - if ( $this->section_helper ) { + /** + * Fields that sit inside an open section are wrapped by that section's own helper, so the + * row here is not theirs to close. set_field() opens the section helper for the section + * field itself, one step ahead of this check, and section_is_open is not set until + * sync_list_size() runs afterwards. Testing both is what keeps the section field subject to + * the width check below rather than being treated as though it were already inside itself. + * + * @see https://github.com/Strategy11/formidable-pro/issues/3820 + */ + if ( $this->section_helper && $this->section_is_open ) { return false; } @@ -192,7 +177,7 @@ private function should_first_close_the_active_field_wrapper() { return false; } - return ! $this->can_support_current_layout() || $this->is_frm_first || ( $this->is_section_start && ! $this->is_group_start ); + return ! $this->can_support_current_layout() || $this->is_frm_first; } /** @@ -203,8 +188,6 @@ private function begin_field_wrapper() { $this->parent_li = true; $this->current_list_size = 0; $this->current_field_count = 0; - $this->is_group_start = $this->is_frm_first; - $this->is_section_start = 'divider' === $this->field->type; } /** @@ -290,7 +273,12 @@ public function force_close_field_wrapper() { * @return void */ private function close_field_wrapper() { - $this->maybe_close_section_helper(); + // Only an open section has a nested wrapper to close. A section helper that set_field() has + // just created belongs to the section about to start, so closing this row must leave it alone. + if ( $this->section_is_open ) { + $this->maybe_close_section_helper(); + } + echo ''; $this->parent_li = false; $this->current_list_size = 0; diff --git a/tests/phpunit/fields/test_FrmFieldGridHelper.php.php b/tests/phpunit/fields/test_FrmFieldGridHelper.php similarity index 94% rename from tests/phpunit/fields/test_FrmFieldGridHelper.php.php rename to tests/phpunit/fields/test_FrmFieldGridHelper.php index 3ac2004d32..d43ecce4f2 100644 --- a/tests/phpunit/fields/test_FrmFieldGridHelper.php.php +++ b/tests/phpunit/fields/test_FrmFieldGridHelper.php @@ -427,18 +427,13 @@ public function test_a_section_keeps_its_helper_after_the_row_bookkeeping_runs() } /** - * Documents the behaviour reported in https://github.com/Strategy11/formidable-pro/issues/3820. + * Covers https://github.com/Strategy11/formidable-pro/issues/3820. * - * A section is measured through section_size rather than active_field_size, and - * should_first_close_the_active_field_wrapper() returns early while a section helper - * exists, so the width check never runs for the section itself. The narrow field and the - * full width section end up in one row even though they add up to sixteen columns. - * - * The assertion below records what the helper does today. Closing the row before the section - * is only half the fix: close_field_wrapper() also drops the section helper, which - * test_a_section_keeps_its_helper_after_the_row_bookkeeping_runs covers. + * A four column field followed by a full width section adds up to sixteen columns, so the + * section has to start its own row. The section still has to arrive intact, which is why the + * nested row and the end marker are part of the expected layout rather than a bare '[1][2]'. */ - public function test_a_section_currently_joins_a_row_it_cannot_fit_in() { + public function test_a_section_starts_a_new_row_when_it_cannot_fit() { $layout = $this->get_grid_layout( array( $this->text_spec( 'frm4' ), @@ -448,7 +443,7 @@ public function test_a_section_currently_joins_a_row_it_cannot_fit_in() { ) ); - $this->assertSame( '[1,2[3]4]', $layout, 'Four columns plus a full width section overflow the row, but the row is not closed first.' ); + $this->assertSame( '[1][2[3]4]', $layout, 'Four columns plus a full width section overflow the row, so the row closes first.' ); } /**