Work has started. It is going well.
We may need a ComponentSource plugin type, like Canvas does.
It was not necessary, there is a more elegant way: a small SourceWithSlotInterface for source plugins.
Rebased.
A follow-up issue could be to replace DisplayBuilderUiHelpers::getInstancesFromProviders() (and related private methods) by a new DisplayBuildableInterface::getInstances() static method.
The work on š Provide a way to show styles Needs work with the introduction of
ThirdPartySettingsInterface::getSummary()could be a good way of manage the lock indicator, and the opportunity to:
- move to
third_party_settingsinstead of having our own first level key which will be not managed byui_patterns_fieldanyway- use
InstanceInterface::setThirdPartySettings()instead of specificInstanceInterface::switchLock()- use
ApiControllerInterface::thirdPartySettingsUpdate()instead of specificApiControllerInterface::switchLock()- use
DisplayBuilderEvents::ON_UPDATEinstead of specific , or introduce something likeDisplayBuilderEvents::ON_INVISIBLE_UPDATEfor updates which are not updating the renderable like this one
ā
Done. DisplayBuilderEvents::ON_INVISIBLE_UPDATE has not been added but the proposal is not discarded.
Also, instead of a boolean, it would be better to have a ternary value:
- locked
- unlocked
- neutral: following the locked or unlocked status of the closest ancestor
ā Done.
So we are nearly there. The remaining part is blocking actions.
There are 2 way of doing that:
- the current work has started altering the UI. See :
ThirdPartySettingsInterface::alterNodeRenderable()in https://git.drupalcode.org/issue/display_builder-3551232/-/blob/3551232-... It is not good because UI change from panel to panel (ex: BuilderPanel & LayerPanel). - a better solution would be to leverage mechanisms which will be added to š Cardinality constraint for slots Active so we don't need to manipulate the UI ourselves.: a lock slot is a slot with a minimum, maximum and suggested equal to the current content. This may not be enough (how to prevent dragging out? for example), but it is a good way of doing most of the job as a start)
I don't reproduce the issue anymore! With the current MR state and a vanilla Drupal 11.3.0-rc2
Still working after a composer update to get Drupal 11.3.1.
It is very good news but it is a bit myterious.
What do you have on your side?
Work in progress.
No pipeline error anymore (all green!). But still some unresponsiveness.
Good idea.
Would it be a good idea to completely hide the tab (main contextual form, but also the third party settings ones) when empty?
So, in a profile with 2 contextual panels (contextual form & styles):
- if empty contextual form but styles form: we directly display the styles form
- if empty contextual form and empty styles form: we display a message like 'No configuration required'
Each node has 4 keys:
node_id(string)source_id(string)source(array)third_party_settings(array)
The summary of the config stored in source is already managed by the plugin identified in source_id through \Drupal\ui_patterns\PluginSettingsInterface::settingsSummary().
So, for component props, would it be enough to simply extends ComponentSource::settingsSummary() (which is today only adding the variant)?
Do we release without or wait next year?
We will tag 1.0.0-beta1 with this fix, as soon as possible. If it takes us 10 days to do the fix, that means we will release next year.
Some Playwright tests can be flaky, but it's not the whole test, when you check the report it's only a small step.
I will investigate this small step this afternoon. Don't hesitate to share insights if something come to mind.
The Playwright test seems ok on local.
Indeed, the playwright tests are green in local, on the same commit at the one of the MR.
Our current attachBehaviors and HTMX core usage will need to be updated, but at least it works out of SSE.
SSE must work for beta1 as well as 21 days ago. And I am afraid there is something bigger here, something we may regret to ship in beta1 (with or without SSE).
It seems it is related to a change introduced in š Follow Core's HTMX integration Active :
function triggerDrupalBehaviorsFromHtmxEvent(htmxLoadEvent) {
Drupal.attachBehaviors(
htmxLoadEvent.detail.elt?.parentElement.closest('.display-builder'),
drupalSettings,
);
}
window.addEventListener('htmx:oobAfterSwap', triggerDrupalBehaviorsFromHtmxEvent);
window.addEventListener('htmx:load', triggerDrupalBehaviorsFromHtmxEvent);
Because the element manipulated here is the one with the SSE HTMX attributes:
<section hx-ext="sse" sse-connect="/api/display-builder/page_layout__hello/sse" class="display-builder" data-once="dbKeyboardGlobal dbInit dbContextualMenu dbFullscreenRestore">
Doing this change seems to fix the issue:
- htmxLoadEvent.detail.elt?.parentElement.closest('.display-builder'),
+ htmxLoadEvent.detail.elt?.parentElement.closest('.display-builder').children,
However, there is a few things to check:
- most of unresponsiveness are gone, but I still got a few
- the first parameter of
Drupal.attachBehaviorsexpects a singleHTMLElement, not aHTMLCollection - a pipeline error:
waiting for locator('.db-island-builder [data-node-type="textfield"]').first() at objects/DisplayBuilder.ts:146 - there is already a similar mechanism in Core (
/core/misc/htmx/htmx-behaviors.js). Can we leverage it instead of kind of duplicating it?
htmx.on('htmx:drupal:load', ({ detail }) => { attachFromHtmx = true; Drupal.attachBehaviors(detail.elt, drupalSettings); attachFromHtmx = false; });
This make the UX close to fullscreen mode,
Second drawer (right) should displace like left one
Great. Closing the UX gap between the 2 modes (normal/full) and the 2 drawers may also lead to a simpler, more maintainable, implementation.
It will also get us closer to š [1.0.0-beta1] Evaluate Navigation module's top bar API Active
Instead of a ComponentTree like Canvas, we have a SourceTree, we nest data sources and each source plugin has its own config schema. So, by nesting the source plugins, we have a big config schema tree and we can extract the translatable strings.
This abstraction is the core of our architecture since the start of UI Patterns 2 development in June 2023 .
Is there a selection that happens at a certain point by someone to determine what gets translated?
Such selection is not necessary.
For minimum and maximum, would
minItemsandmaxItemsbe closer aligned to JSON schema semantics, which uses the latter pair for arrays whereas the former pair is only for numbers?
I like minItems and maxItems because it is more explicit that minimum and maximum but it has been a few weeks I am telling the SDC community that "we have reached a consensus, you can start implementation even if not merged yet" ;)
So, if we change that, we need to share the information with:
- Display Builder & UI Patterns team: š Cardinality constraint for slots Active and ⨠Slots restrictions according to suggestions and cardinality Active
- Canvas team
- Any other contrib project which may have started to implement mechanisms around those properties
Maybe we could find a way to easily disable keyboard shortcuts in the Profile configuration.
Why not. But the most important is to fix the bug for the people using the shortcuts.
From Yannick:
BTW I am always using cmd + l to move my cursor into my browser's address bar and the Library panel is always opening
So, it is not related to specific environments.
Hi Laurii,
What are you planning to use for determining which props should be translated? Asking because it might be good for us to coordinate on this so that we could provide a good experience for people who are using both Canvas and Display Builder.
Our logic (which is not really the current MR, we didn't push yet the current work which is very promising) is not based in props, it is based on our data source plugin type:
- Prop types (what you call "Prop shapes" in Canvas) are guessed from props schema
- Data source plugins are targeting prop types
- Data source plugins have a different data model than the prop shape they are targeting. Sometimes it happens there is a one-to-one mapping between the two models, but it is not the rule.
- Data source plugins data model is using config schema API so can have translatable strings here. This is where translation is managed.
I believe it would be a mistake to think about "translatable props":
- this is not part of JSON schema AFAIK
- this is not part of UI modeling and it is not up to the component authors to care about that
- this is a mechanism related to the CMS application state, so out of SDC
What do you think about that?
There is still conflict on rebase with current 1.0.x.
There will always have conflicts because the MT is altering a lot of files and everytime we are committing to 1.0.x a conflict may occurs.
I am rebasing twice a week to keep track of the 1.0.x changes.
There are 3 phpunit errors:
- 1 in EntityViewDisplayTest::testSaveSources
- 2 in ConfigFormBuilderTest::testConfigFormBuilderBuild
about the general idea: Is it relevant? Is it the right time to do such a task? Is it bringing benefits without making too much mess?
Because of #3562991: Instances list from config and not storage ā and the challenges of the upcoming ⨠Symmetric config translation Active , I am now convinced this change is relevant.
Once the phpunit errors are fixed, before wrapping up (reoganizing the interface, idying up, documentation...), I would be glad to have the input of someone:
- about the implementation itself: Am I going the right way?
- about the opportunity of simplification and streamlining it may bring later
- about naming and discovery of the plugins (the current implementation is temporary)
(this is not a review before merge, just chit-chatting)
It seems this change has been done in https://git.drupalcode.org/project/display_builder/-/commit/463f693c51f0...
Hi debdeep,
Thanks for your contribution. Your patch is doing neither of the 2 proposals:
- Jean's proposal: Switch button groups to dropdowns
- Pierre's proposal: show only icon in smaller screen.
But your little change may be welcomed. It will be evaluated during beta2 phase.
In UI Skins 1.x, there is no differences between "global CSS variables" and "component CSS variables".
In UI Skins 2.x, we will have su h difference thanks to the work on Core's ⨠Add a CSS variables API Active but it will work the opposite way:
- when selecting a component, we will have both the global variables and the component variables
- outside the component, we will have only the global variables
Work has started. Interface added but implementations not complete yet.
Is it not too risky that close from beta1?
The proposal has changed: now we render the display builder with admin theme and we display builder & preview panels with front theme (and let's not forger the preview popovers in library).
It was the opportunity of cleaning and fixing a lot of thing in the DeclarativeShadowDomRenderer but it sill don' twork:
- if we render in the slotted area, we have the same bleeding issue than before
- if we render in the shadow area, wome features are not working (like sortablejs drag and drop)
Christian will continue the work.
It looks great now!
The merge split columns remarks need discussion. For now They are useful for me like that.
It was just a suggestion. If it is OK for you, it is OK for me.
They asked to double-check, so I did in a new test-only MR to avoid disrupting the RTBC MR: I removed the recursion, which results in failures
I got the same results.
pdureau ā created an issue. See original summary ā .
There is a remaining ['third_party_settings']['ui_styles'] after migration from Layout Builder.
The main issues are fixed:
- The styles don't bleed anymore, because we removed the placeholder from DeclarativeShadowDomRenderer. Thos placeholders were bubbling until the HTTP response rendering.
- We can applied DSD at the initial load by adding a
DeclarativeShadowDomRenderer::render()call to ProfileViewBuilder and by moving the existing call from DisplayBuilderEventsSubscriber to HtmxTrait::addOutOfBand()
So, Drupal side, we are good (except script_bottom, skipped in the initail ticket, it may be the opportunity to reconsider this)
However, we have 2 JS issues to solve:
- the
link rel="stylesheet"calls are not applied in the DSD - the shadowroot is lost after an OOB swap
pdureau ā created an issue. See original summary ā .
pdureau ā changed the visibility of the branch 3549867-first-step-is to hidden.
The pipeline was green. I manually run the "playwright (all)" job which was omitted and i have a warning. Not sure it is related to the current change because this job may not be run at every MR.
Once merged, we need to do some changes on display_builder_dev_tools too.
It is OK to me, but I am also not reproducing the bug anymore on branch 2.0.x š²
Maybe I am missing something, maybe we did something else on UI Patterns or Display Builder which is preventing the trigger of the warning.
Do we merge?
I did a bit more than only PHP interfaces, but i didn't touch the islands plugin themselves, to stay safe.
I don't know if it this task is better being merged before beta1 (to provide stable interfaces and to avoid complex rebasing later in the project) of after beta1 (to not introduce some risk and instability before shipping) :
The work on
š
Provide a way to show styles
Needs work
with the introduction of ThirdPartySettingsInterface::getSummary() could be a good way of manage the lock indicator, and the opportunity to:
- move to
third_party_settingsinstead of having our own first level key which will be not managed byui_patterns_fieldanyway - use
InstanceInterface::setThirdPartySettings()instead of specificInstanceInterface::switchLock() - use
ApiControllerInterface::thirdPartySettingsUpdate()instead of specificApiControllerInterface::switchLock() - use
DisplayBuilderEvents::ON_UPDATEinstead of specific , or introduce something likeDisplayBuilderEvents::ON_INVISIBLE_UPDATEfor updates which are not updating the renderable like this one
Also, instead of a boolean, it would be better to have a ternary value:
- locked
- unlocked
- neutral: following the locked or unlocked status of the closest ancestor
We deactivate the source from BlockLibraryPanel for beta1, and we will do the real fix for beta2
My feedback from š Render and preview not working Active :
- Entity display: Tokens >> š„ I am reproducing
- Entity display: Fields image >> š„ I am reproducing. Discussed in UI Patterns weekly. It may be related to
FieldItemInterface::generateSampleValue()with entity reference fields. - Views display: Views rows >> Is it already covered by this issue ⨠Preview of view sources Active planned for a later beta release?
Following the updates of issue description.
Critical Bug
š Undefined array key "variant_id" in ComponentSource::settingsSummary() Needs review (better if hotfix alone as dev is currently slower)
No need to wait to tag beta1 because we are not pinning the ui_patterns dependency, right? With "drupal/ui_patterns": "^2.0.12", a project doing a composer update will get 2.0.13 and further... am I right?
UI styles when apply with Bootstrap is slow (500-700ms): could be it apply and update everything and not only the target
I am not reproducing this:
- ā between 150ms and 220ms even with both UI Suite DaisyUI & UI Suite Bootstrap installed.
- ā the OOB swap replace only the expected component
š Render and preview not working Active
- Entity display: Tokens >> š„ I am reproducing
- Entity display: Fields image >> š„ I am reproducing. Discussed in UI Patterns weekly. It may be related to
FieldItemInterface::generateSampleValue()with entity reference fields. - Views display: Views rows >> Is it already covered by this issue ⨠Preview of view sources Active planned for a later beta release?
š Drag problem: after adding a style to a component, cannot drag in component slot Active
I am still not reproducing this one.
š WYSIWYG block behavior problems with ajax Active
Not checked yet.
Page layout: š First step is not properly savable Active
š„ I am reproducing this one and I agree it is beta1 priority.
š Beta fixes on search, preview Needs work
Not checked yet.
Preset name do not handle special chars, one with utf8 will crash all builders: Unable to encode schema array as JSON: Malformed UTF-8 characters, possibly incorrectly encoded
š„ I am reproducing this one and I agree it is beta1 priority.
Medium Bug
WYSIWYG popup (like link or table) do not show
I am reproducing this one
š Page conditions for content type Active
Not checked yet.
UI Suite DSFR: Instance form checkbox styled by dsfr.js, to see after ⨠Theming Display Builder Active
I also hope ⨠Theming Display Builder Active will fix this.
That's just because the class you changed it to doesn't have that method :) We just need an assertion that this is in fact still the expected implementation (Canvas' own).
This is what I thought.
I forgot about https://git.drupalcode.org/project/ui_patterns/-/blob/2.0.x/src/Componen... ā sorry! It's been a hell of a ride towards 1.0 š«
Don't worry about that :)
So I think you're saying that Canvas MUST switch back to service decoration? And if so: how is this not a duplicate of š ComponentPluginManager decorator should call decorated service instead of parent Active ? If so: happy to push that forward š
indeed, I believe š ComponentPluginManager decorator should call decorated service instead of parent Active is the solution for the root cause. This little MR was to test a quickfix until then. I will keep it on my side to iterate locally and maybe propose a little something.
paved the path to REMOVE Canvas' class
That would be cool. This is also the goal of UI Patterns, removing our ComponentPluginManager class and rely only on Core's, but it will take some time. I am targeting mid 2026.
Not as smooth as last time:
Line src/PropShape/PropShape.php
------ -----------------------------------------------------------------------
97 Call to an undefined method
Drupal\Core\Theme\ComponentPluginManager::resolveJsonSchemaReferences
().
šŖŖ method.notFound š
I am proposing to target only Layers for now:
1. Add new info slot in layer component which is displayed in the top right corner:
2. Add a ThirdPartySettingsInterface (not the same as the one in Core) for islands storing data in node's third_party_settings with a ::getSummary() method
3. In LayersPanel::buildSingleBlock() and LayersPanel::buildSingleComponent(), for each third_party_settings of the node: if the key is an island plugin ID an the island plugin implementing the ThirdPartySettingsInterface, execute ::getSummary() and send the value to layer component's info slot.
I can do a MR if you want.
2 thoughts:
- It may also be the opportunity to drop
wysiwyg_fixes.js š - If a JS is not working as expected, it may be because we are not loading it in
DeclarativeShadowDomRenderer. See snippet.
public function render(array $content, bool $admin = FALSE): array {
...
<js-placeholder token="{{ placeholder_token }}">
<!--js-bottom-placeholder token="{{ placeholder_token }}"-->
...
// Create placeholder strings for these keys.
$types = [
'styles' => 'css',
'scripts' => 'js',
// We are not rendering script_bottom placeholder, in order to not
// trigger BigPipe. Is it a mistake?
];
Maybe a context is not injected properly to the condition plugin.
Let's compare https://caniuse.com/declarative-shadow-dom with https://www.drupal.org/docs/getting-started/system-requirements/browser-... ā
- The latest release of each of the latest two supported major versions of:
- Desktop browsers
- Google Chrome ā
- Firefox ā
- Safari ā
- Microsoft Edge ā
- Opera ā
- Mobile browsers
- Safari for iOS ā
- Desktop browsers
- The latest supported release of the latest major version of:
- Desktop browsers
- Firefox ESR ā (current version: 140)
- Mobile browsers
- Chrome for Android ā
- Chrome for iOS ā
- Opera Mini (except for 'extreme data savings' mode) ā (0,03% of market share)
- Samsung Internet ā
- Desktop browsers
I have added a CSS zoom property to address the issue of admin theme spacing VS available space in drawer.
I don't see anything wrong with phpmd in your MR and I don't know why the job is red.
Thanks for the work, it looks ready to review, we can move it t to beta2 scope.
Update documentation
We have a post-beta1 ticket for that: š Update documentation Active
Critical bug
Wait for ui patterns release that include variant_id fix š Undefined array key "variant_id" in ComponentSource::settingsSummary() Needs review
It is only a warning, and out of Display Builder scope, so not a beta1 blocker in my opinion.
Token render and preview not working on entity display
š„ I am reproducing and I agree it must be addressed in beta1 scope.
Drag problem: after adding a style to a component, cannot drag in other slot
I am not reproducing this one. Which component are you testing with?
WYSIWYG block do not load CkEditor in instance form (right drawer) properly
š„ It may worth to try with ⨠Theming Display Builder Active which is displaying the full form in the admin theme, isolated in a shadow dom, so it must work in a more expected way.
Page layout: Publish and restore buttons are always enabled even if current is published version, related to š First step is not properly savable Active
š„ Indeed, it looks related to š First step is not properly savable Active which is a nice-to-have for beta1. Maybe we can have a look on why the phpunit fails there and merge it for beta1.
Page layout: The message status is always after the page layout (create node with url /test create page layout, edit / save the node, message is at the bottom)
I am not reproducing this one:
- create node with url /test: ā
- create page layout (with no conditions): ā (with the messages block still at the default position, between title and tabs)
- edit / save the node: ā the message is displayed at the expected position
Page layout: Apply a style to a component, the response in xhr contain error:
Drupal\Core\Render\Element\ComponentElement::generateComponentTemplate(): Argument #1 ($id) must be of type string, null given, called in /var/www/html/web/core/lib/Drupal/Core/Render/Element/ComponentElement.php on line 81
Could be seen on preview panel.
I am not reproducing this one. Which component are you testing with?
Entity: All fields entity preview are not working (broken...)
š„ Indeed, they are broken in the preview on hover in the block library. Is its something easy and quick to fix or do we need to address this in beta2?
Low bug
Libraries drawer (left): if filter set on components and switch, block list will be filtered without the text filled
I am not reproducing and I agree it is low priority subject.
Out of full screen after opening drawer, the top margin is behind toolbar
I am reproducing and I agree it is low priority subject.
Have the configuration use Drupal ajax when saved
Is it something which will fix this bug?
- I uncheck a checked island (or similar changes)
- I open the dialog of another island config
- I save the form inside the dialog, the dialog closes
- ā The island is checked again
Works again after new rebase from 1.0.x.
Jean, do you want to have a look? It is not a proper review because the pipeline is not green yet, but it would be great to have your opinion.
Pipeline running...
This is a silent failure. Shouldn't this be an explicit failure, to inform developers?
Done. Simply by commiting Wim's proposal.
Could you make this available for contrib modules (such as Canvas) to call, too?
Done, but I have added a new public method instead of changing the existing one, in order to keep internal mechanisms private. IS it OK for you?
Can you please add a test case that points to module://some_thing or theme://some_thing ā i.e. a module or theme name with an underscore in it?
It is already the case. Theme for tests is sdc_theme_test_references with references to URL like such: theme://sdc_theme_test_references/schema.json#image
Can we now bump core's requirement to require 6.6.2?
Done. From ^5.2 || ^6.5.2 to ^6.6.2?
Can we now bump core's requirement to require 6.6.2?
Can I add it this to the MR or does it need a specific MR?
That was so efficient! It was such a good idea to go upstream. Thanks @longwave.
I am still reproducing after the merge of š Follow Core's HTMX integration Active . Let's keep it in beta2 scope