Problem/Motivation

When there is at least one empty block in a region, PageLayoutSource is not rendered by Drupal because it fails the \Drupal\Core\Render\Element::isRenderArray() check.

It happens for example with help_block which is positioned by default by UI Suite Bootstrap and UI Suite DSFR themes.

Proposed resolution

Clean empty blocks before rendering.

Command icon Show commands

Start 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:

Comments

pdureau created an issue. See original summary.

pdureau’s picture

Status: Active » Needs work

started

pdureau’s picture

Assigned: pdureau » mogtofu33
Status: Needs work » Needs review

Done.

The phpstan fails are probably not related to this work.

mogtofu33’s picture

Assigned: mogtofu33 » pdureau
Status: Needs review » Needs work

A rebase should fix ci.

pdureau’s picture

Assigned: pdureau » mogtofu33
Status: Needs work » Needs review

A rebase should fix ci.

Indeed

mogtofu33’s picture

mogtofu33’s picture

Assigned: mogtofu33 » pdureau
Status: Needs review » Needs work

From User experience perspective, Page Layout seems confusing:

  • Enable Display Builder Layout on an existing website (with standard or existing block layout configuration)
  • Create a Page Layout
  • Build Display

Then you have this 'Page Layout (from active theme)' with slot based on regions.

  • There is no indication on what it is, mostly the fact that slots are not real regions in Drupal sense
  • you can not configure it, or adapt the layout, moving a block to an other region change only the order
  • you can apply styles or visibility on it, which then make it like a container, but it is not and styles will apply to a child it seems...

Obviously the user experience is confusing on creation and after. We need to facilitate adoption, not create strange unexplained behaviors.

Page Layout source was created as a way to merge existing block layout configuration so user do not have an empty page. But then we create a unexplained component with regions without real markup.
And problem is worst when using a proper theme with layout components, you basically on every new Page Layout have to remove the page layout to build your page.

What can we do to fix that and remove confusion?

  • Start from a flat imported list of blocks from blocks layout?
  • Start with the minimal core block layout without looking on existing block layout state? with title, messages, help, tags, content
  • Other option?
pdureau’s picture

Thanks for the review.

Review

There is no indication on what it is, mostly the fact that slots are not real regions in Drupal sense

The source plugin label is "Page layout (from active theme)", it may not be the best label, but I don't have better idea right now. Do you?

you can not configure it, or adapt the layout

That's the point, this source plugin is the exact representation of the rigid, hardcoded, not configurable, page.html.twig.

moving a block to an other region change only the order

Oh! Really? That's a bug, i will a have a look.

you can apply styles or visibility on it, which then make it like a container, but it is not and styles will apply to a child it seems...

Styles: As far as I know, page.html.twig doesn't have attributes, but UI styles is supposed to manage this.
Visibility: Weird.
I will have a look.

UX improvements

Page Layout source was created as a way to merge existing block layout configuration so user do not have an empty page. But then we create a unexplained component with regions without real markup.
And problem is worst when using a proper theme with layout components, you basically on every new Page Layout have to remove the page layout to build your page.

I agree there is something to consider here. The automatic initialization with this PageLayoutSource plugin is relevant for the projects keeping Block Layout as a fallback/default. But not for the projects already using a Page Layout entity without any condition plugins set as a fallback/default.

Today, this situations is managed by:

  • always importing PageLayoutSource plugin when creating a PageLayout entity from scratch
  • but also proposing a "Duplicate page layout feature" so users can start from the default Page Layout entity if they want

But we can do better.

Start from a flat imported list of blocks from blocks layout?
Start with the minimal core block layout without looking on existing block layout state? with title, messages, help, tags, content

I am afraid those proposals are not fixing the fallback/default pages dichotomy because:

  • an user not using Block Layout anymore will not be interested by a flat imported list of blocks from blocks layout
  • an user using a Page Layout as fallback/default will prefer to use it as a starting point instead of a minimal hardcoded list

To improve the UX, we can make this choice more obvious by exposing it directly in the creation page (and not the edit page) of the config entity type:

  • by adding a select input with Block Layout and all page layout entities without any condition plugin set
  • if there is no page layout entities without any condition plugin set, we keep the select as disabled/readonly

Screenshot:
aa

What do you think?

pdureau’s picture

Discussed with Jean.

Review:

  • moving a block to an other region change only the order > ✅ Already fixed by the MR
  • you can apply styles or visibility on it, which then make it like a container, but it is not and styles will apply to a child it seems... > ✅ Styles already fixed by the MR ⚠️ visibility to check again

UX improvements:

  • OK for select list at create but with minimal/ block layout / blank instead of looking for page layout entities without any condition plugin set.
  • Pierre will try to do the change in the current MR. But if too big/risky, it will be a follow-up feature request.
pdureau’s picture

Assigned: pdureau » mogtofu33
Status: Needs work » Needs review

Review

Styles are using the "drilling" system of UI Styles which is sometimes working weird because the Drupal Render API is weird.
For example, with UI suite DaisyUI, it drills until the navbar slots.

So, to have a consistent behavior, I have wrapped the renderable in a html_tag which will host the styles and other dynamic attributes.

For visibility, I don't see surprising behaviour in Preview and final result. But I see weird stuff in Builder/Canvas: the renderable is not rendered anymore, that's normal, but the children are. Not being familiar with Visibility panel, i don't know if it is OK, but it doesn't seem something introduced by the current work which is fixing already a lot.

UX improvments

The create form page will be discussed after beta6.

mogtofu33’s picture

Assigned: mogtofu33 » pdureau
Status: Needs review » Needs work

Test KO ❌

  • Install standard 11.4.4
  • Enable ui_styles ui_patterns
  • Set UI Suite Bootstrap 5.2.3 as default theme
    • Website frontpage OK ✅
  • Enable 'Display Builder for page layout'
  • Go to Page layout, create Default without restirction
  • Build display, just hit 'Publish'
    • Website frontpage KO ❌
  • Build display: move blocks out of 'Page layout (from active theme)'
    • Website frontpage OK ✅
  • Back to display with 'Page layout (from active theme)'
  • On modules/display_builder_page_layout/src/Plugin/UiPatterns/Source/PageLayoutSource.php line 119 replace with return $page;
    • Website frontpage OK ✅
    • Styles apply on 'Page layout (from active theme)' got to child -> KO ❌
pdureau’s picture

Oh shout! you are right.

The current state of the MR is not working with every theme:

  • UI suite DaisyUI: ✅ good layout rendering and styles correctly applied
  • UI suite Material: ✅ good layout rendering and styles correctly applied
  • UI suite DSFR: ✅ good layout rendering and styles correctly applied
  • UI suite Bootstrap: ❌ I reproduce the error
  • UI suite USWDS: ❌same error
  • bootstrap with I suite Bootstrap styles: ✅ good layout rendering and styles correctly applied

I will do a change, test with all themes I have available, so i will send to review only when i am 100% sure.