Custom Blocks with Layout Builder Enabled No Longer Use Front End Theme

DanielVeza → changed the visibility of the branch 3423920-custom-blocks-with to hidden.

Left some comments on the MR - thanks!

Addressed the feedback. I've moved the new functionality to the Layout Builder module.

I was going to wrap the new code in a moduleExists('block_content'), but since Layout Builder is already getting a dependency on block_content in 📌 Hidden dependency on block_content in layout_builder Needs work and adding this route doesn't cause any hard without the block_content module I think it's fine to keep in.

The main reason why I went against this approach is that it means we would need to add the ModuleHandler dependency to LayoutBuilderRoutes, just to then deprecate and remove it after 📌 Hidden dependency on block_content in layout_builder Needs work has been commited.

The main reason why I went against this approach is that it means we would need to add the ModuleHandler dependency to LayoutBuilderRoutes, just to then deprecate and remove it after

Makes sense - the presence of the route in the collection signals the module exist anyway right?

Ah yes you're correct. We check if that route exists before we do anything. That esentially acts as a moduleExists('block_content') anyway :)

Seems @acbramley already did a review.

But landing on the ticket noticed the issue summary could use an update, would be helpful to use the standard issue template. UI changes before/after screenshots, proposed solution, etc.

MR feedback addressed, IS updated.

Ready for review again

Changes look great, IS reads well. Tried to run the test only job but it is currently broken https://git.drupalcode.org/issue/drupal-3423920/-/jobs/1273985

I think this is good to go now regardless, small uber nit on the MR that could be applied on commit.

Need to fix the test file and class name, I had run the test only job and it passed. Got very confused and posted in #gitlab thinking the job itself was broken. Thanks to @fjgarlin for seeing the name was invalid.

🤦‍♂️ Renamed the test file

Test only run failed as expected, good to go!

Should we be fixing this here or in \Drupal\layout_builder\Routing\LayoutBuilderRoutesTrait::buildLayoutRoutes() or in \Drupal\layout_builder\Plugin\SectionStorage\OverridesSectionStorage::buildRoutes()... like should we have a generic fix here for all entity types or something specific for content blocks?

For example: \Drupal\layout_builder\Plugin\SectionStorage\DefaultsSectionStorage::buildRoutes() does $options['_admin_route'] = FALSE; prior to calling LayoutBuilderRoutesTrait::buildLayoutRoutes() - why is OverridesSectionStorage::buildRoutes() not doing the same?

I did have the thought while I was doing this that it was a little odd to be writing a specific override for one entity type.

I don't have the historical context on why OverridesSectionStorage::buildRoutes doesn't set _admin_route to FALSE, so I'm unsure if thats an oversight or a choice that was made.

Either way, I've reworked the solution to disable the _admin_route in OverridesSectionStorage::buildRoutes and renamed the test to be more generic.

Tests green again

Can confirm the issue following the steps in the issue summary
Applying the MR does resolve the issue of layout builder using admin theme

Screenshots are near identical to the issue summary but here's a fresh set.

Before

After

This looks good to me, couldn't find anything to complain about.

Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!

Automatically closed - issue fixed for 2 weeks with no activity.