Problem/Motivation

When adding a section before an existing section, there are several issues:

Steps to reproduce

I created a test to make this more visible.
To visually see the issues, comment out the assertions and check the browsertest output.

Proposed resolution

This issue exists because the check in layout_builder_lock_form_layout_builder_configure_section_alter() is not watertight.
There will always be lock configuration in this case, as the $delta refers to an already existing section.

So we need to know if it's a new or existing section.

Unfortunately there is currently no way to know if a section is being created or updated.
the property on the ConfigureSectionForm is protected and no public method exist to expose it.

So IMO the current "solution" (more a hacky way) is to do a similar check as layout builder does: check if a plugin_id is passed in the ConfigureSectionForm::buildForm() method to determine if we are adding or editing a section.
A better solution would obviously be to check this by a public method on the ConfigureSectionForm.

The patch contains this hacky fix and a rework of the existing multiple sections test to include the tests for this case.
It it makes more sense to add an extra test, that's also fine :)

Remaining tasks

Create a core issue to somehow expose the state of the section form.

User interface changes

none

API changes

none

Data model changes

none

CommentFileSizeAuthor
#2 3199556-2.patch6.24 KBmollux
#2 3199556-2-fail.patch2.95 KBmollux

Comments

mollux created an issue. See original summary.

mollux’s picture

Status: Active » Needs review
StatusFileSize
new2.95 KB
new6.24 KB

See the attached patches to reproduce the issue, and to fix it

mollux’s picture

Title: Creating a section before an existing section results in incorrect lock data for new and existing sections. » Creating a section before an existing section results in incorrect lock data for the new and existing section

The last submitted patch, 2: 3199556-2-fail.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

swentel’s picture

Yeah, I've been cursing a lot too on the fact that it's hard to figure that out .. or how hard it is to get to the section storage.

Will do some more manual testing before I commit, but already nice investigation!

  • swentel committed df1b8de on 8.x-1.x authored by mollux
    Issue #3199556 by mollux: Creating a section before an existing section...
swentel’s picture

Status: Needs review » Fixed

committed and pushed thanks!

I also committed a small change in the configure form to add a display:block on the checkboxes form item as in some circumstances (when DS is enabled), everything started floating left. There's no impact when DS is not enabled, so works fine.

(it probably would have been cleaner with an additional css file, but the JS was already included, so been a bit lazy :)

swentel’s picture

Status: Fixed » Closed (fixed)