Problem/Motivation
I applied the patch in #2952967: When allowing content items to have customized layouts, clicking 'Layout' on full view of a node takes you to the configuration for the 'default' view mode to my 8.5.0 project. Even with the patch applied, I was not able to view customized layouts on a per node basis.
Steps to reproduce:
- Enable the "full content" view mode for basic page
- Check "Allow each content item to have its layout customized."
- Create a new basic page
- Click "Layout" and add a block to the layout (I used the powered by Drupal block)
- Save the layout and view the node
I expected to see the customized layout with the new block added, but only the default layout appeared.
It appears that the allow_custom
setting applies on a per-view mode basis. The third party settings config object for the view mode's layout_builder does not include that flag for any view mode configuration other than the default. LayoutBuilderEntityViewDisplay checks this value before it decides whether to return the section configuration from the view mode config or the customized layout stored with the node.
Proposed resolution
I updated the form method in LayoutBuilderEntityViewDisplayForm
to include the layout options form elements for both "default" and "full" view modes and updated my view mode configuration. The customized layout was then used when viewing the node.
I am not sure this is the best solution as it appears that the layout stored with the node is not associated with a particular view mode so the updates would show for any view mode which can be customized. I think this can cause unexpected behavior as it is.
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#28 | 2955065-layout-28-interdiff.txt | 797 bytes | tim.plunkett |
#28 | 2955065-layout-28.patch | 10.19 KB | tim.plunkett |
#27 | 2955065-layout-27-interdiff.txt | 6.6 KB | tim.plunkett |
#27 | 2955065-layout-27.patch | 10.16 KB | tim.plunkett |
#25 | 2955065-25.patch | 6.54 KB | phenaproxima |
Comments
Comment #2
pixlkat CreditAttribution: pixlkat commentedComment #3
tim.plunkettThanks for the super detailed bug report!
Ughhhhhhh I hate hate hate the default/full split. So many strange interactions.
Comment #4
tim.plunkettIf you enable overrides on default before you enable the Full view mode, everything worked fine. Which is why our test didn't catch it.
This should better mimic the situation where you have the Full view mode enabled before you enable overrides.
Comment #6
andypostMaybe instead of uncached query use entity repository? Like
\Drupal\Core\Entity\EntityDisplayRepositoryInterface::getViewModeOptionsByBundle()
Comment #7
tim.plunkettExcept we don't need to load the entities at all, just checking for their existence.
Comment #8
tim.plunkettComment #9
mtodor CreditAttribution: mtodor at Thunder commentedNice work with provided steps to reproduce and patch.
I have tested this patch a bit, what is strange for me is when you enable "Allow each content item to have its layout customized." for default view and then you enable also Full display -> option is suddenly on Full display tab. That's strange and it could lead to some problems and bad UX.
What about keeping that option on Default display? Would that be possible?
Some strange things that occurred to me:
And one additional with following steps to reproduce (I tested on default Article display):
The website encountered an unexpected error. Please try again later.Drupal\Core\Entity\Query\QueryException: 'layout_builder__layout' not found in Drupal\Core\Entity\Query\Sql\Tables->ensureEntityTable() (line 343 of core/lib/Drupal/Core/Entity/Query/Sql/Tables.php).
Comment #10
tim.plunkettRemoving from sprint tag as I'm not actively working on this. Please feel free to retag if someone picks this up!
Comment #11
tim.plunkettComment #12
phenaproximaRerolled.
Comment #13
phenaproximaThis intersects with the work being done in #2936358: Layout Builder should be opt-in per display (entity type/bundle/view mode), so I'm postponing on that.
Comment #15
phenaproximaBlocker is in.
Comment #16
phenaproximaComment #17
phenaproximaRerolled again.
Comment #19
dmsmidtSince #2936358: Layout Builder should be opt-in per display (entity type/bundle/view mode) got in, this issue became more relevant.
It is now possible to NOT enable Layout Builder on the Default view mode and DO enable it on Full view mode.
But in this case it is impossible to enable customization per content item.
Since LayoutBuilder is now set up per view mode, it makes sense to repeat this setting aswell across all view modes.
Although there is no UI to change, for example a teaser view mode for a certain item (afaik), this could be a use case in the future.
Comment #20
tim.plunkettAgreed with the major priority.
#2907413: Consider supporting Layout Builder Overrides for other view modes is the issue to expand this to all view modes.
Comment #21
yareckon CreditAttribution: yareckon at wegewerk commentedJust ran into this while test driving 8.6 for the first time. A less experienced drupaller wouldn't have had the tickle at the back of their mind that said "I wonder if this is a full / default bug?". And it was...
Comment #22
tim.plunkettReroll.
Comment #25
phenaproximaRerolled again!
Comment #27
tim.plunkettFixed another bug and added a lot more test coverage
Comment #28
tim.plunkettLet's future-proof this one addition. Added a comment.
Comment #29
phenaproximaI don't really have any feedback here. The change is very tightly scoped and makes sense, and the test coverage is super extensive. I feel good about this patch, overall.
Comment #31
tim.plunkettRandom media fails :\
Comment #32
alexpottCredited @pixlkat for opening the issue and @andypost, @dmsmidt and @mtodor for contributing to the discussion and patch review.
Committed and pushed 518e8fff0c to 8.8.x and 1d0a02ed7d to 8.7.x. Thanks!
Backported to 8.7.x as a major bugfix + it is great to have layout builder with less bugs in its first stable release - great work.
I ran the tests locally to ensure we had test coverage of the else added in #27 and we do!
Comment #36
bkosborneWith this change, I wondered what would happen to default entity view displays that had layout builder enabled with overrides, since this change prevents you from enabling overrides on default displays.
I performed this test:
Drupal 8.6.15:
Upgraded to 8.7.0:
So, for anyone, like me, that has been using overrides with the "Default" entity view displays, nothing seems to break, but this behavior is a little funky. To get things back to the way they "should be", I will follow these steps on any entity that has overrides enabled for default entity view display: