Thanks a lot to everybody for this hard work ❤️
I have rebased ✨ [PP-1] Allow schema references in Single Directory Component prop schemas Postponed and everything is working perfectly
Rebase is done.
The MR is full green and only 30-40 line of PHP code added to Core (blank lines, comments and tests removed).
✨ Add stream wrappers to access extension files Needs work is merged, I will rebase and send back to review.
Mikael is taking the related UIP2 issue ✨ Slots restrictions according to suggestions and cardinality Active so it is the best candidate for this task
Hello,
I am testing with an existing content display override in Display Builder.
The updb was run successfully:
$ ddev drush updb
ui_patterns_field 10001 hook_update_n 10001 - Add additional columns. Add third_party_settings and node_id to ui_patterns_source fields.
However, when I click on "Publish" button in the content display override in Display Builder, I get this:
Drupal\Core\Entity\EntityStorageException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'field_teaser_third_party_settings' in 'INSERT INTO': INSERT INTO "node__field_teaser" ("entity_id", "revision_id", "bundle", "delta", "langcode", "field_teaser_source_id", "field_teaser_source", "field_teaser_third_party_settings", "field_teaser_node_id") VALUES (...) in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 815 of core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).
I would prefer a way for users to add support for additional units rather than hard coding to what is in the spec.
Let's play safe, avoid any drupalism, and follow the upstream specs for the 11.3 MR.
However, thanks to the plugin system, Drupal projects may be able override the plugin, adding their own units:
use Drupal\Core\Theme\Plugin\DesignTokenValue\Dimension as CoreDimension;
#[DesignTokenValue(
id: 'dimension',
label: new TranslatableMarkup('Dimension'),
)]
class Dimension extends CoreDimension {
public const array UNIT_TYPES = [
'px',
'rem',
'whatever'
];
We may need a ComponentSource plugin type, like Canvas does, with 3 plugins:
- SDC, as we already have
- Layout plugins for the current feature
- Theme page, where every region is a slot and the template is
page.html.twig, to address ✨ Block layout import Active
Update 3 months after.
We have 4 situation to checks when I embed a display (teaser) in the other (full/default) with the ui_patterns_field's: Render Source formatter:
- None of them are overridden: ❌ The teaser is not visible, which may be normal because nothing is stored in the field
- Only Teaser is overridden: ✅ the teaser is embedded
- Only Default is overridden: ❌ The teaser is not visible, which may be normal because nothing is stored in the field
- Both are overridden: ✅ the teaser is
What is happening when we put the formatter of a display in the same display ? Infinite recursion?
Yes, infinite recursion & memory limit.
Is it a way of implementing lock mechanism like layout_builder_lock → is providing?
We have a dedicated issue now: #3551232: Add a lock system →
So, 2 remaining tasks:
- Do we display the entity view display config when the field is empty?
- Prevent infinite recursion when a field overriden a display is rendered in the same display.
HI Jean, we may need your help. It is hard to guess what is causing the playwright fail in await element.click({ position: { x: 5, y: 10 } })
Do we also need to activate js.preprocess for playwright?
I have added the 2 test cases I was able to add.
Only line 190 & 199 may not be covered, but another mechanism is preventing buildSingleBlock() to be reached (which is is good thing in my opinion)
We may need a ComponentSource plugin type, like Canvas does, with 3 plugins:
- SDC, as we already have
- Layout plugins for ✨ [1.0.0-beta2] Layout plugins support Active
- Theme page, where every region is a slot and the template is
page.html.twig, to address this feature
If OK today, let's move this to beta1. If not, let's put it back to beta2.
Hi @juc1, thanks for the work, can you:
- fix pipelines errors (mostly phpcs)?
- move group logic to its own protected property in PresetLibraryPanel?
The prefill of group value with the SDC group is unexpected but looks like a good idea :) Maybe we can do the same with other sources ?
- if Drupal block plugin, the block category
- for other slot sources, just the source label
Then, users will be able to edit the group in admin UI anyway.
Let's move this issue to beta2 as a follow-up of 📌 Follow Core's HTMX integration Active and let's start by:
- adopting the Htmx helper from Core
- Remove unused HtmxTrait::addTarget()
- Move ::setTrigger() to HtmxEvents
I move to beta2 to explore the declarative shadow dom idea following JohnAlbin talk :https://www.youtube.com/watch?v=04YKlSNf89o
Let's be careful about JS events in and out the sandbox.
If it works, we will have 2 proposals:
- we keep display builder in front theme and we display contextual panels with admin theme
- we move display builder to admin theme and we display builder abd preview panel with front theme
[Entity] ➜ [Field] source is not directly exposed anymore and we can directly drop fields in the builder.
However, this source is still the unit of configuration we are manipualting in the contextual panel because we can switch to another field in the first select input.
I see some quirks when JS preprocess is activated. For example:
- the contextual panel title don't change when we switch the field
- most of the config changes (pick a formatter, pick a property...) don't update and we need to click on "Update" button (see 📌 Remove Update button from ContextualFormPanel Active )
But I have the same when JS preprocess is disabled. So, it seems we can remove the "Disable JavaScript files aggregation" advice.
"Update" button is still useful with the "entity -> field" source.
Once Jean has shared the cover HTML file, let's have a look:
- if doable today, lets keep in beta1
- if not, let"s move to beta2
careful, i have rebased the MR.
It works OK for me.
Next steps:
- fix pipeline KOs
- check if components or blocks with a specific asset library, when added for the first time in BuilderPanel, have the expected CSS and/or JS in the HTMX resposne
- use the Htmx helper from Core? Pierre has a branch somewhere with a conversion. But it can be a follow-up issue.
I am still not reproducing while using display_builder_theme_test with Firefox outside of Playwright:
Investigation still ongoing.
An other point is you assumed the 'profile' key in the tests, I changed it for the const ConfigFormBuilderInterface::PROFILE_PROPERTY.
oh! sure! 👍
You left some empty third arguments in the self::assert methods, it's used to describe the test, it makes it easier to read, I personally don't use it enough but that's good practice.
Good to know.
For the split and data provider let me know if you want me to do it.
That would be nice if you do it because I am not sure about the way to do it:
- split: the 2 access handlers are working together, checking 2 layers of the same access, so it seems odd to me to split them (it may be because I am new to kernel testing)
- data provider: i don't see data providers in the other kernel tests we have, I was beliveing it was more a unit test thing
Don't worry. I will rebase your stuff and keep working ConfigFormBuilderTest
Need to fix test ConfigFormBuilderTest, split AccessControlTest for each entity types and use data provider to have lighter and easier to maintain tests.
I would be happy to do this and fix the phpunit fail probably introduced by the rebase: https://git.drupalcode.org/project/display_builder/-/jobs/7264278
Curret work merged in beta1. We keep the ticket for follow-up.
It seems the issue is fixed by 📌 Follow Core's HTMX integration Active :)
Jean, on your 4 comments:
- I have fixed 2 of them
- I have answered the 2 others
We need this Core patch : 🐛 PageDisplayVariant does not transmit cache metadata Active
PageDisplayVariantSelectionEvent implements RefinableCacheableDependencyInterface.
So let's explore this
- we add a custom cache tag in PageDisplayVariantSelectionEvent
- we keep ::isImpactingPageVariantDetection()
- instead of what flushing the cache in
PageLayout::postSave(), we invalidate the tag
Finally, I did more than expected:
in src/Entity/Instance.php and src/ProfileViewBuilder.php I am proposing a fix of the initial scope.
in modules/display_builder_page_layout/src/Entity/PageLayout.php and modules/display_builder_page_layout/src/Plugin/DisplayVariant/DisplayBuilderPageVariant.php I am proposing a fix for page variants maangement.
Functional review: OK for me.
What about the technical implementation? Is it OK too?
It seems to work well everywhere... but in /admin/structure/page-layout/{page_layout}/builder where I get:
Warning: Undefined array key "#display" in Drupal\Core\Render\Element\StatusMessages::generatePlaceholder() (line 54 of core/lib/Drupal/Core/Render/Element/StatusMessages.php).