Problem/Motivation
When adding a section before an existing section, there are several issues:
- the new section form shows the lock options, while we expect not to (see #3129009: Cannot add sections to default layout because of OutOfBoundsException: Invalid delta error)
- the lock settings are stored on the existing section
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
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3199556-2.patch | 6.24 KB | mollux |
| #2 | 3199556-2-fail.patch | 2.95 KB | mollux |
Comments
Comment #2
mollux commentedSee the attached patches to reproduce the issue, and to fix it
Comment #3
mollux commentedComment #5
swentel commentedYeah, 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!
Comment #7
swentel commentedcommitted 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 :)
Comment #8
swentel commented