- π¬π§United Kingdom catch
I think that this is an existing core bug, because we should already be doing the same thing for layout builder and navigation modules, but we don't.
Fixing it requires adding a new API to core, don't see another way, I opened π Block plugins need to be able to update their settings in multiple different storage implementations Active which doesn't have a nice API but I think probably does have the places that API would need to be implemented and roughly what it would need to do.
I'm not postponing this issue on that one yet, but I think it probably should be.
The sentence that got me to that point was this one from the parent issue:
However β¦ it can't actually work because we won't know which of those post-update hooks are associated with which block plugins! In theory, we could run all those block-targeting post-update hooks, but then we'd need to track for every single block-sourced component instance which update hooks have already been executed (equivalent to how core does the system-wide UpdateRegistry). That doesn't scale, just like it doesn't scale to update potentially millions of (revision) rows.
This is the missing piece that the new BlockPluginUpdater API would address.
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
- π¬π§United Kingdom longwave UK
Don't we also need a constraint adding to
depth
? - πΊπΈUnited States phenaproxima Massachusetts
Wrote the update path. But it does need an explicit validation test.
- @phenaproxima opened merge request.
- π§πͺBelgium borisson_ Mechelen, π§πͺ
The analysis in #4 looks great! That means we can do it easily with the same flexibility of constraint we have today. +1 to this approach
- πΊπΈUnited States phenaproxima Massachusetts
Wouldn't it be better to make depth optional, aka nullable: true? Then we don't need to assign a special meaning to 0
Not sure we should do that, as it would constitute an API change for...what benefit, exactly? Maybe a separate issue would be better here.
- πΊπΈUnited States phenaproxima Massachusetts
$this->menuTree->maxDepth()
Fun fact:
\Drupal\Core\Menu\MenuLinkTree::maxDepth()
calls\Drupal\Core\Menu\MenuTreeStorage::maxDepth()
, which does no computation at all, but merely returns...a constant (\Drupal\Core\Menu\MenuTreeStorage::MAX_DEPTH
).That constant is older than two of my nieces. It was created in #2301239: MenuLinkNG part1 (no UI or conversions): plugins (static + MenuLinkContent) + MenuLinkManager + MenuTreeStorage β and appears to have been completely unchanged since.
So, to get this issue done faster, I propose that we don't even need to worry about having a special constraint to determine the max depth dynamically. Let's just hard-code it, at least for now, based on the value of that constant, which has not been changed in core since it was introduced.
That way, we'll get pretty strong validation done quickly, without a ton of effort, and we could make it a more flexible constraint later in a follow-up.
- π©πͺGermany Anybody Porta Westfalica
Just wanted to leave a note here, that we need something similar, but not for the alt-text, but for a subset of fields of the media entity to override in the referencing media reference field wiget.
https://www.drupal.org/project/media_library_media_modify β is close, but implements its own field type, which is a problem for existing sites. Instead, it should be a field widget for the existing media references.
- π¬π§United Kingdom longwave UK
1. We need to write an update path + update path test that does the exact same thing for a search block stored in an XB component tree.
Are we thinking here that we need block implementations to provide separate code paths to update XB blocks? If so, what happens if a contrib author decides not to do that? Or, who takes responsibility for core block updates, if core does not depend on XB?
On the other hand, I am not sure if this will work, but I wonder if we can do this without contrib maintainers needing to do anything at all! Block updates are generally written like this:
function search_post_update_block_with_empty_page_id(&$sandbox = []) { $config_entity_updater = \Drupal::classResolver(ConfigEntityUpdater::class); $config_entity_updater->update($sandbox, 'block', function (BlockInterface $block): bool { ...
Can we decorate ConfigEntityUpdater so as well as updating config entities, we additionally reconstruct config entities for blocks in XB content, and allow the closure to make changes to them in the same way? The closure is not responsible for saving the config entity - so because these won't be real config entities, we never need to directly save them; we can just extract the settings (or whatever else might have changed?) and put them back into the XB storage.
- π§πͺBelgium borisson_ Mechelen, π§πͺ
Validate level: must be between 0 and $this->menuTree->maxDepth()β Range constraint?
Can a range constraint be variable like this, or should we fall back to the same thing with did for weight and say it is somewhere between 0 and PHP_INT_MAX?
- Issue created by @wim leers
- π¨π¦Canada smulvih2 Canada π
This would be great to get done, so we don't need to rely on layout_builder_st which has been lagging for D11 support.
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Plan articulated β at π± [META] Production-ready ComponentSource plugins Active .