Field Layout is being deprecated.
Field layout settings are used in layout_builder.install
foreach ($displays as $display) {
// Create the first section from any existing Field Layout settings.
$field_layout = $display->getThirdPartySettings('field_layout');
if (isset($field_layout['id'])) {
$field_layout += ['settings' => []];
$display
->enableLayoutBuilder()
->appendSection(new Section($field_layout['id'], $field_layout['settings']))
->save();
$display_changed = TRUE;
}
}
Additionally, there is a kernel test (LayoutBuilderFieldLayoutCompatibilityTest) that only exists to validate this integration, which is now obsolete and should be removed.
field_layout module enabled.layout_builder.install still attempts to migrate field_layout settings.LayoutBuilderFieldLayoutCompatibilityTest, it is only verifying deprecated behavior.Copy usage and test coverage from layout builder into field_layout
None.
None.
Removes field_layout-related initialization from Layout Builder.
None.
NA
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !13075
Comments
Comment #4
ajinkya45 commentedComment #5
deepakkm commentedI think the test also needs to worked upon, hence moving in progress.
Comment #6
ajinkya45 commentedComment #7
smustgrave commentedSummary appears incomplete
Comment #8
ajinkya45 commentedComment #9
smustgrave commentedStill seems to be the same issue summary.
Talking about the ticket btw
Comment #10
ajinkya45 commentedComment #11
ajinkya45 commentedComment #12
dcam commentedThe changes look OK to me with the exception of one thing. I left a suggestion on the MR.
Comment #13
ajinkya45 commentedComment #14
dcam commentedI don't know the internals of Layout Builder well enough to understand what this install function does. So I'm not comfortable OKing the changes. But I did notice that the old code saved the display entity. The new code does not. That seems like an oversight. If the tests pass despite that, then it probably indicates a lack of test coverage.
Comment #15
ajinkya45 commentedThe failing tests (
LayoutBuilderOptInTest) are asserting that Layout Builder should not be enabled by default, but my patch enables it on all displays. Can we confirm whether the intent is to move to Layout Builder being always enabled by default, or should we only be enabling it when migrating from field_layout?Comment #16
smustgrave commentedShouldn't the test and install be moved to field_layout. This seems to be losing test coverage and functionality moving to contrib
Comment #17
smustgrave commentedComment #18
quietone commentedComment #19
quietone commentedLayoutBuilderFieldLayoutCompatibilityTest was moved to field_layout in #3541499: Move field_layout tests to field_layout module.
Comment #20
larowlanI think what we want here is a `hook_modules_installed` in field_layout module that does what layout_builder.install is doing if the module being installed is layout_builder. I.e flip the responsibility from LB to field layout
Comment #21
quietone commented@larowlan, thanks that is just what I needed.
@ajinkya45, Sorry for not making the issue summary explicit. This isn't a simple removal. We need to retain test coverage but move all the code dependent on field_layout to field_layout itself. That way we can remove the entire field_layout module from core safely.
Restarted this by reverted the previous two 'removal' only commits. The test was moved to field_layout in a sibling issue, that just leaves dealing with
function layout_builder_install():. For that I followed the advice in #20 but as an OO hook.Comment #22
quietone commentedComment #23
smustgrave commentedSeems like a good move.
Comment #26
catchCommitted/pushed to 11.x and cherry-picked to 11.3.x, thanks!