- Issue created by @Tim Bozeman
- Status changed to Needs work
12 months ago 12:33am 28 November 2023 - πΊπΈUnited States quicksketch
I read through and tried out the PR. The code rearrangement looks great; I think they'll be a lot of benefits to overriding classes like
UpdateBlockForm
andConfigureSectionForm
, allowing for other changes in the future. I also like the suppression of the layout changes message so there aren't duplicate messages. Seems like a lot of improvements are in the PR.In testing, I encountered some issues:
- The settings page at
admin/config/content/layout-builder-plus
for some reason now is empty; with the message:No entities have layout builder enabled.
, which is not accurate. - Enabling Layout Builder on a content type throws a fatal error:
Drupal\Component\Plugin\Exception\ContextException: The entity context is not a valid context. in Drupal\layout_builder\Plugin\SectionStorage\SectionStorageBase->getContextDefinition() (line 150 of core/lib/Drupal/Core/Plugin/ContextAwarePluginTrait.php).
. This can reproduced by visitingadmin/structure/types/manage/page/display/default
, enabling Layout Builder, then clicking "Manage Layout".
Since I couldn't get to the list of blocks, it was hard to actually test the new functionality. It seems like this would be done by adding a new "Layout Block"? This opens up some other questions/problems:
- How is this functionality enabled/disabled? Is this a new block that can put into the list of promoted blocks?
- Can nesting more than one level be prevented?
- Can only certain layouts be allowed to be nested? For example there's not much point in nesting a single column, and perhaps a site might make layouts that are made to be nested vs those that are made to be root-level
This is such a huge set of changes it might be valuable to separate out the storage and class refactoring into its own issue, then handle the UI problems of nested layouts on top of those changes.
- The settings page at
- πΊπΈUnited States Tim Bozeman
Heya Quicksketch!
Not sure if you remember, but one time I asked you for a "quick sketch" at sandcamp and you drew a portrait of me. I still have it <3
Thank you for trying to review! Sorry about those bugs. It's still a work in progress. I think I got the settings and manage default layout pages going again. π Here are the steps I'm doing to use the nested layouts atm:
Run
drush si -y; drush en lb_plus -y; drush uli;
Add a Layout Block
- Goto
/admin/structure/block-content
and add a Layout Block block type - Remove the body field
- Manage the display
- Check "Use Layout builder"
- Check "Allow each content item to have its layout customized"
- Click Save
- Configure a default layout section
- Promote some blocks (Basic and Layout)
Enable LB+ for a content type
- Goto
/admin/structure/types/manage/page/display
- Check "Use Layout builder"
- Check "Allow each content item to have its layout customized"
- Click Save
- Configure a default layout section
- Promote some blocks (Basic and Layout)
- Goto
- πΊπΈUnited States quicksketch
Not sure if you remember, but one time I asked you for a "quick sketch" at sandcamp and you drew a portrait of me. I still have it <3
Hahaha! I do remember. I think you're the only person that's ever asked me for that. I recall being pretty disappointed with my output, I haven't been doing much drawing for a couple decades now. Ha!
I'll try out the latest PR again. Thanks!
- Status changed to Needs review
12 months ago 9:05pm 6 December 2023 -
Tim Bozeman β
committed 8e5a7f94 on 2.0.x
Resolve #3402217 "Support inline layout"
-
Tim Bozeman β
committed 8e5a7f94 on 2.0.x
- Status changed to Fixed
11 months ago 6:04pm 11 December 2023 Automatically closed - issue fixed for 2 weeks with no activity.