Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions classes/helpers/FrmFieldGridHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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 '</ul></li>';
$this->parent_li = false;
$this->current_list_size = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' ),
Expand All @@ -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.' );
}

/**
Expand Down
Loading