🇫🇷France @pdureau

Let's start as soon as 📌 Navigation look without navigation module Active is merged.

Need review.

Tested with the toolbar, admin toolbar & navigation.

The removal of 📌 [1.0.0-beta1] Evaluate Navigation module's top bar API Active logic can create a weird layout on smaller screen, but I am proposing to address this in 📌 More responsive toolbar Active (see new proposal).

Another proposal would be to show only icon in smaller screen.

All island have icons, but they are not always visible:

• Start region: visible alongside the labels (View Panels)
• Middle region: never visible (View Panels)
• End region: according to configuration: sometimes only icons, sometimes only panel, sometimes both (Icons)

Instead of deciding that on server side rendering, let's always rnder the icons, but let's hide them on browser rendering following the same rules.

So, at smallest viewport width, we can hide all labels, show all icons.

Out of scope: change z-index, absolute positioning. Jean did a lot effort to make them compatible with toolbar, admin toolbar, navigation...

It seems the existing logic is mainly (only?) targeting admin toolbar. So, let's group all offset/positioning logic related to toolbar, admin toolbar, navigation... in a single CSS file and let's complete the missing parts.

Needs review.

Only 128 lines of PHP code (tests, blank lines & comments removed) to add this very important mechanism ;)

Follow-up for post-beta1:

• 📌 Security: avaibility of endpoints according to islands Active
• UI tweaks:
  • Entity view override: show display label in tab when only one display available
  • Entity view override: confusing form when not enough right
  • Page Layout: don't show "Display builder" operation in list builder when no access

DONE:

• We can switch profile even if we have no access to the actual profile >> The select is now disabled.
• Entity view override: no empty select when no access to profiles >> The select is now disabled.
• Can create a page layout without profile >> The submit button is now disabled
• Entity view override: Can't pick a field when no profile >> The select are now disabled, but there is still some tweaks to do
• Entity view override: hide "create field" link when no access to profiles >> Hidden
• Views: no empty dialog when no access to profiles >> We have a message now.

pdureau → created an issue.

I propose to close this issue:

• We are happy with current fragment replacement. No issues since the beginning of the project.
• We are sticking with Drupal Core behaviour anyway, and it seems Drupal sticks with HTMX defaults
• In next major version of HTMX, default will move to morphdom

So just because it's not in the spec, doesn't mean we can't support the units we want in our implementation.

Yes, we will be able to extend it if we want, this is not blocking.

However:

• Let's stick with the upstream specs for the 11.3 MR. We are already bringing a lot of value with this scope, and we need to play safe.
• For 11.4 and later, let's be careful when extending standards with Drupalisms

Moved to post beta1 considering the last changes in 📌 Security: Set Instance access handler Active and the other planned follow-ups.

Remaining work in the current issue:

• Can create a page layout without profile.
• Entity view override: Can't pick a field when no profile.
• We can switch profile even if we have no access to the actual profile. Do we turn the select in readonly?

Follow-up for post-beta1:

• 📌 Security: avaibility of endpoints according to islands Active
• Do we really need "Use Display Builder" permission? It seems this is the same has no "Use the {Profile} Display Builder profile" permission. Let's remove it if not useful.
• UI tweaks:
  • Entity view override: no empty select when no access to profiles
  • Entity view override: hide "create field" link when no access to profiles
  • Entity view override: show display label in tab when only one display available
  • Views: no empty dialog when no access to profiles

📌 Security: Set Instance access handler Active is adding a new method to DisplayBuildableInterface.

work in progress.

Review with 3 different roles

Limited profile is a profile created for this tests in order to check the users can't access the default profile.

Role 1: with all permissions (bit not the administrator)

Permissions:

• Use Display Builder (Display Builder module): YES
• Use the Limited Display Builder profile (Display Builder module): YES
• Use the Default Display Builder profile (Display Builder module): NO
• {entity_type}: Administer display (Field UI module): YES
• Administer views (Views module): YES
• Administer page layouts (Display Builder for page layout module): YES
• Article: Edit any content (Node module, to test entity view display overrides): YES

Testing ConfigFormBuilder:

Page Layout:

• Can create a page layout. Expected: YES. Results: ✅
• Can load Display Builder for page layout with Limited profile. Expected: YES. Results: ✅
• Can load Display Builder for page layout with Default profile. Expected: NO. Results: ✅<
• Can alter the display builder instance with Limited profile. Expected: YES. Results: ❌ Got HTTP 403
• Can alter the display builder instance with Default profile. Expected: No. Results: ✅ i got HTTP 403 (to do this test, we need to remove Profile access check first)

Entity view display:

• Can switch an entity view display to Display Builder. Expected: YES. Results: ✅
• Can load Display Builder for entity view display with Limited profile. Expected: YES. Results: ✅
• Can load Display Builder for entity view display with Default profile. Expected: NO. Results: ✅<
• Can alter the display builder instance with Limited profile. Expected: YES. Results: ❌ Got HTTP 403
• Can alter the display builder instance with Default profile. Expected: No. Results: ✅ i got HTTP 403 (to do this test, we need to remove Profile access check first)

Entity view override:

• Can pick an existing field. Expected: YES. Results: ✅
• Can create a new field. Expected: NO. Results: ⚠️ We can't but the link is visible anyway. Let's hide it.
• Can switch a views display to Display Builder. Expected: YES. Results:✅
• Can load Display Builder for override with Limited profile. Expected: YES. Results: ⚠️ tab visible and access OK, but tab as "display" label instead of the display label
• Can load Display Builder for override with Default profile. Expected: NO. Results: ✅ tab not visible + HTTP 403
• Can alter the display builder instance with Limited profile. Expected: YES. Results: ❌ Got HTTP 403
• Can alter the display builder instance with Default profile. Expected: No. Results: ??? (to do this test, we need to remove Profile access check first)

Views:

• Can switch a views display to Display Builder. Expected: YES. Results: ???
• Can load Display Builder for views display with Limited profile. Expected: YES. Results: ???
• Can load Display Builder for views display with Default profile. Expected: NO. Results: ???
• Can alter the display builder instance with Limited profile. Expected: YES. Results: ???
• Can alter the display builder instance with Default profile. Expected: NO. Results: ???

Role 2: has the "classic" permissions of Drupal display building, but not the ones from Display Builder

Permissions:

• Use Display Builder (Display Builder module): NO
• Use the Limited Display Builder profile (Display Builder module): NO
• Use the Default Display Builder profile (Display Builder module): NO
• {entity_type}: Administer display (Field UI module): YES
• Administer views (Views module): YES
• Administer page layouts (Display Builder for page layout module): YES

Page Layout:

• Can create a page layout. Expected: NO. Results: ???
• Can load Display Builder for page layout. Expected: NO. Results: ???
• Can alter the display builder instance. Expected: NO. Results: ???

Entity view display:

• Can switch an entity view display to Display Builder. Expected: NO. Results: ???
• Can load Display Builder for entity view display. Expected: NO. Results: ???
• Can alter the display builder instance. Expected: NO. Results: ???

Entity view override:

• Can pick an existing field. Expected: YES. Results: ???
• Can create a new field. Expected: NO. Results: ???
• Can switch a views display to Display Builder. Expected: YES. Results: ???
• Can load Display Builder for override. Expected: NO. Results: ???
• Can alter the display builder instance. Expected: YES. Results: ???

Views:

• Can switch a views display to Display Builder. Expected: NO. Results: ???
• Can load Display Builder for views display. Expected: NO. Results: ???
• Can alter the display builder instance. Expected: NO. Results: ???

Role 3: has the Display Builder permissions, but not the ones from classic Drupal display building

Permissions:

• Use Display Builder (Display Builder module): YES
• Use the Limited Display Builder profile (Display Builder module): YES
• Use the Default Display Builder profile (Display Builder module): NO
• {entity_type}: Administer display (Field UI module): NO
• Administer views (Views module): NO
• Administer page layouts (Display Builder for page layout module): NO

Page Layout:

• Can create a page layout. Expected: NO. Results: ???
• Can load Display Builder for page layout. Expected: NO. Results: ???
• Can alter the display builder instance. Expected: NO. Results: ???

Entity view display:

• Can switch an entity view display to Display Builder. Expected: NO. Results: ???
• Can load Display Builder for entity view display. Expected: NO. Results: ???
• Can alter the display builder instance. Expected: NO. Results: ???

Entity view override:

• Can pick an existing field. Expected: YES. Results: ???
• Can create a new field. Expected: NO. Results: ???
• Can switch a views display to Display Builder. Expected: YES. Results: ???
• Can load Display Builder for override. Expected: NO. Results: ???
• Can alter the display builder instance. Expected: YES. Results: ???

Views:

• Can switch a views display to Display Builder. Expected: NO. Results: ???.
• Can load Display Builder for views display. Expected: NO. Results: ???
• Can alter the display builder instance. Expected: NO. Results: ???

@jean: Do we really need "Use Display Builder" permission? It seems this is the same has no "Use the {Profile} Display Builder profile" permission. Let's remove it if not useful.

We also need to update entity_view.spec.ts playwright test because Navigation is adding a second "Display builder" button in the page:

--- a/tests/src/Playwright/Tests/entity_view.spec.ts
+++ b/tests/src/Playwright/Tests/entity_view.spec.ts
@@ -32,7 +32,7 @@ test(
 
       await page.goto(config.contentTypesDisplay.replace('{content_type}', name))
       // Save the fields for copy in the builder.
-      await expect(page.getByRole('button', { name: 'Display builder' })).toBeVisible()
+      await expect(page.locator('main').getByRole('button', { name: 'Display builder' })).toBeVisible()

Hi @larowlan: Test added. Is it OK?

I will rebase, add a kernel test and do some change in the logic.

pdureau → created an issue.

Issue summary updated according to last discussion and MR in review.

Jean, can you have a look?

OK, I will add or extend a test case in core/tests/Drupal/KernelTests/Components/ComponentRenderTest.php

I am sorry @miiimooo but I will close this ticket because a lot happened last 9 months with Attributes management both at Core level and at UI Patterns level. There is a big chance the issue you met has been fixed or evolved to something else.

If this proper issue is met again, let's reopen it or create a new ticket.

Done in 📌 [2.0.9] Filter out noUi components Active

Some details about the duplicate..

1. Added when the JavaScript file is directly loaded.

https://cdn.jsdelivr.net/npm/htmx-ext-sse@2.2.4/dist/sse.js
https://ui-patterns-2.ddev.site/modules/contrib/htmx/js/htmx/htmx.min.js

2. Added when drupal.init.js attaches the behaviors to the DOM

https://cdn.jsdelivr.net/npm/htmx-ext-sse@2.2.4/dist/sse.js
https://ui-patterns-2.ddev.site/modules/contrib/htmx/js/htmx/htmx.min.js
https://ui-patterns-2.ddev.site/modules/contrib/htmx/js/htmx-drupalBehaviors.js
https://ui-patterns-2.ddev.site/core/misc/drupal.js
https://ui-patterns-2.ddev.site/core/misc/drupal.init.js

Work has started

Work has started

Something else to add:

We use third_party_settings because Display Builder is not the "owner" of this config entity type ("entity view display", so we will not override the "natural" config there and use a feature which is allow us to store information in our own namespace into the config

For Page Layout config entity type, it is our own, so we store everything directly in the config root

For views, we use a mechanism similar to third_party_settings but plugin-based: Display Extender

Each time, we try to act nicely and to position ourself in the expected way

Also, let's do the new screenshot with Gin instead of Claro.

I also agree we need to jump directly into Core's HTMX API stuff without passing through drupal/htmx 1.5.

We have an issue for that: 📌 Follow Core's HTMX integration Active

@jean: do we close this issue as "won't fix" ?

To also credit: @schillerm which has helped in the contribution room in Vienna.

Hi Viktor,

As you can read in the issue description, this is not a duplicate of 🐛 Convert attributes props to an Attribute object before Attribute::merge() Active but a follow-up.

I am pretty confident about this because I am partially responsible of introducing this bug in 🐛 Convert attributes props to an Attribute object before Attribute::merge() Active where we check one of the two attributes data ($element['#attributes']) without checking the other ($element['#props']['attributes']) 😳

Thanks Christian for your suggestion. It makes sense but I would prefer to keep the MR focused on the fatal error we are already observing in the wild.

Let's wait to see what Canvas is doing.

I have another issue doing the same steps:

with display builder entity view, take the article tags field, compoenet per item, a component, and boom!

I will create an issue in Display Buildeer issue queue. I don't know the status of the current one.

I am not reproducing the layout builder issue, so it looks good to me.

pdureau → created an issue.

pdureau → created an issue. See original summary → .

pdureau → created an issue.

Being unable to reproduce the issue and, to be honest, even to fully understand it , I still believe we are not addressing the root cause here but doing a workaround.

However, I guess I am OK for it anyway because:

• @lauriii assured me we are not fixing in Core something which must be fixed at Canvas level
• we are doing the check in ComponentValidator instead of ComponentMetadata so it seems we are not messing with the real component schema and we are not doing something like 📌 Remove addition of a object type in all props Needs review

That's great we are that close to 11.3 merge ;)

Needs work. Don't use FormAjaxException but ApiController::responseMessageError() because we already handle this kind of situations.

📌 Move DisplayBuildableInterface to a Provider plugin type Active is postponed and we still don't know if it will land.

However, we keep in "postponed" because it will be easier to do it after 🐛 Drupal\layout_builder\Entity\LayoutBuilderEntityViewDisplay class does not correspond to an entity type. Needs work land into Drupal 11.3

i will have a look

OK, so:

1. rebasing and repushing the branch (because of conflicts with 1.0.x)
2. move to post beta1 because not as easy as a removal, it is the logic itself we need to move to display_builder_page_layout

Too big and too late for beta1.

Weird. Because I don't think I have changed the composer files

But ok, I rebase

Some information from Mateu, the original author (not a proper quote):

it was maybe added in order to accommodate how Drupal can pass a render array in place of any type of data with the intent that it will be rendered down the line. I may be a remnant of the early days CL Components (the ancestor of SDC), when there was no slots yet, only props, so we needed a way to pass renderables in props.

Today, we have slots and we don't pass renderables in props. That may be the reason why this mechanism looks ok to be removed. Anyway, we need to be careful.

Thanks @shubham_pareek_19

I will also ping the original author of this snippet to have its opinion and maybe some information about the context of this addition.

What are you recommending? We move to dedicated issue or not?

I think that's prudent, yes.

All changes to ComponentMetadata and ComponentMetdataTests has been moved to specific issue 📌 Remove addition of a object type in all props Needs review .

Remove "11.3.0 release priority" tag because it is not blocking ✨ [PP-1] Allow schema references in Single Directory Component prop schemas Postponed anymore and can be merged after 11.3 if we need more time.

This issue is related for ✨ [PP-1] Allow schema references in Single Directory Component prop schemas Postponed

• if ✨ [PP-1] Allow schema references in Single Directory Component prop schemas Postponed is merged first, the current issue will need to also alter Drupal\KernelTests\Components\SchemaReferenceResolverTest before merge
• if the current issue is merged first, ✨ [PP-1] Allow schema references in Single Directory Component prop schemas Postponed will need to alter Drupal\KernelTests\Components\SchemaReferenceResolverTest before merged

pdureau → created an issue.

"11.3.0 release priority" tag added because needed by ✨ [PP-1] Allow schema references in Single Directory Component prop schemas Postponed

The MR has been simplified since comment #45. Here is the content (tests excluded).

The API itself:

• JSON schema: core/assets/schemas/v1/style.schema.json
• Form element: core/lib/Drupal/Core/Render/Element/Styles.php
• A new plugin type:
  • core/lib/Drupal/Core/Theme/Style/Exception/StyleDefinitionErrorException.php
  • core/lib/Drupal/Core/Theme/Style/StyleDefinition.php
  • core/lib/Drupal/Core/Theme/Style/StyleDefinitionInterface.php
  • core/lib/Drupal/Core/Theme/Style/StylePluginManager.php
  • core/lib/Drupal/Core/Theme/Style/StylePluginManagerInterface.php
  • core/core.services.yml
• Asset libraries handling:
  • core/lib/Drupal/Core/Render/HtmlResponseAttachmentsProcessor.php
  • core/lib/Drupal/Core/Render/BubbleableMetadata.php

Mechanisms for local application:

• Via a render property: core/lib/Drupal/Core/Render/Renderer.php
• Via a new Drupal\Core\Template\Attribute::addStyle() method: core/lib/Drupal/Core/Template/Attribute.php
• With: core/lib/Drupal/Core/Template/AttributeHelper.php

Add a new #accept_attributes render property to be used in ElementInterface::getInfo() to know in which render element the #attributes object can be added/modified:

• core/lib/Drupal/Core/Render/Element/InlineTemplate.php
• core/modules/filter/src/Element/ProcessedText.php

Use the new API with core_resize library to address ✨ Refactor system/base library Needs work so we have an use case in Core and we can see the benefit of the API:

• core/modules/system/system.styles.yml
• core/lib/Drupal/Core/Form/FormPreprocess.php
• core/lib/Drupal/Core/Render/Element/Textarea.php
• core/themes/claro/templates/form/textarea.html.twig
• core/themes/starterkit_theme/templates/form/textarea.html.twig
• core/profiles/demo_umami/themes/umami/templates/classy/form/textarea.html.twig

There are now 4 usages of this API.

As a style provider:

• UI suite Bootstrap contrib theme: ✨ POC: Core styles API and UI Styles 2 Active
• System core module: core/modules/system/system.styles.yml in the current MT

As a style plugin consumer:

• UI Styles contrib module: ✨ Support styles which are not HTML classes Active
• Layout Builder Style contrib module: 📌 Try Core's Style API proposal Active

POC is advanced enough for opening a discussion ;)

pdureau → created an issue.

pdureau → created an issue.

pdureau → created an issue.

Work in progress: https://git.drupalcode.org/project/display_builder/-/merge_requests/148

Done:

1. Add a new "display_buildable" plugin type.
2. Move DisplayBuildable logic for Entity View, Entity View Overrides, and View extender, to plugins
3. Replace display_builder_provider_info by the plugin type manager

Next steps:

1. check dependency injection and don't create too much plugin instances
2. simplify DisplayBuildableInterface as much as possible
3. move as much logic in DisplayBuildablePluginBase
4. simplify the controllers, event subscribers...
5. Move configFormBuilder to this plugin type?
6. Merge similar ::getInstance() methods
7. Rename: getBuilderUrl() -> getUrl(), getDisplayUrlFromInstanceId() -> getDisplayUrl(), getInitialContext() > getContext()
8. Update documentation and graphical schemas

After upgrading 2.0.10 (and could be related, installing ui_patterns_field and ui_patterns_ui), i no longer able to reproduce this issue.

Great news. So I am closing. Thanks for the report and the update.

For information, related issue on Canvas side: 🐛 ComponentPluginManager decorator should call decorated service instead of parent Active

Let's check if removing "widget" is enough.

OK for review.

The duplicate of 📌 Vet require-dev dependency justinrainbow/json-schema as a require dependency Active has been removed.

The duplicate of ✨ Add stream wrappers to access extension files Needs work is still here.

No phpunit error anymore. A functional test error (Drupal\Tests\field_ui\FunctionalJavascript\ManageDisplay) but doesn't seem related to the current work.

@wim, we have 2 ongoing discussions on https://git.drupalcode.org/project/drupal/-/merge_requests/13457/diffs

I have dropped the commits adding dependency injection. In review again.

Notes:

• We keep the 'profile' stream wrapper. Profiles are planned to be deprecated but we're a very long way off being able to actually do it.
• A functional test also failing but it may not be related to the current task: SettingsTrayBlockFormTest, ManageDisplayTest
• I am afraid I misunderstood this advice from @alexpott: "try to use the extension list in the container if possible to have less dependencies within the stream wrapper"

Thanks for the info. Back to work ;)

Remaining tasks:

• Rebase ✅
• Fix ExtensionStreamTest:
  • ✅ olivero theme exists but its structure has changed
  • ✅ Don't use the same module for testStreamWrapperMethods && testStreamWrapperDirnameOnMissingExtensions
• try to use the extension list in the container if possible to have less dependencies within the stream wrapper >> We are using dependency injection everywhere now ✅

Notes:

• We keep the 'profile' stream wrapper. Profiles are planned to be deprecated but we're a very long way off being able to actually do it.
• Some functional tests also failing but they may not be related to the current task: LayoutBuilderTest, ManageDisplayTest