diff --git a/classes/helpers/FrmFieldGridHelper.php b/classes/helpers/FrmFieldGridHelper.php index 30eb37d6cf..527a7693af 100644 --- a/classes/helpers/FrmFieldGridHelper.php +++ b/classes/helpers/FrmFieldGridHelper.php @@ -156,7 +156,20 @@ public function maybe_begin_field_wrapper() { * @return bool */ private function should_first_close_the_active_field_wrapper() { - if ( false === $this->parent_li || $this->section_helper ) { + if ( false === $this->parent_li ) { + return false; + } + + /** + * 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; } @@ -260,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.' ); } /**