Do we want to consider backporting this to 10.4? It looks like if we re-enable compat mode, it might be a small enough change that this would be reasonable to do, but I didn't look into enough to be confident yet.
*/
public function build(): array {
return [
'user' => [
'#lazy_builder' => [
'navigation.user_lazy_builder:renderNavigationLinks',
[],
],
'#create_placeholder' => TRUE,
'#cache' => [
'keys' => ['user_set_navigation_links'],
'contexts' => ['user'],
],
'#lazy_builder_preview' => [
'#markup' => '<a href="#" class="toolbar-tray-lazy-placeholder- link"> </a>',
],
],
];
}
I think there's still an issue here - it now looks like the preview isn't user-specific, but the caching of the preview still is. This could be verified by using the navigation with a couple of different users then checking the cache IDs in the cache_render table.
It could end up being a duplicate of 📌 Investigate using the core "User account menu" in favor of custom Navigation Block for same Active maybe though.
catch → created an issue.
#17 is pretty convincing to me that we can do without the ID attributes.
Don't have time to commit this today (or probably this week - about to head afk for at least a couple of days), but +1 from me.
I think this is a good idea, and I think it's fine if core uses internal libraries more than once.
One question on the MR.
One very small nit on the MR, but overall the approach seems good to me, as you say a good first step to private-by-default services.
Committed/pushed to 11.x and cherry-picked to 11.0.x, thanks!
However, Drupal pages have long been dependent on a strong relationship between and content, with different JS/CSS bundles from page to page, for example.
So this is true, but for AJAX requests we already deliver assets separately outside the head markup and it's also the case for big pipe placeholders (which would be another thing to try a conversion of fairly early on since that relies entirely on the AJAX system with a small amount of custom js)..
I think the existing concepts of ajax_page_state ought to work - i.e. we track the libraries that have already been sent, and when new libraries are sent, they go through the asset rendering system and are sent as a new aggregate - both script and stylesheet link tags are allowed in the body of the document so they can be sent with the HTML (or injected via js wherever). Drupal settings and HTML head links (favicon, feeds etc.) are a bit different though so definitely need to figure out those.
Yes I think so too, it would be a good thing to add alongside a theme builder.
I also opened #3446582: Responsive images recipe → a couple of days ago, to ship some default image styles matching Olivero's break points to start with and linked this issue from there too.
pameeela → credited catch → .
pameeela → credited catch → .
Content replacement with css/js would be a good proof of concept I think especially if that's already partially implement in the contrib module. If the new markup comes with CSS and JavaScript that's a fairly small but also complex use-case to cover.
Committed/pushed to 11.x/11.0.x/10.4.x/10.3.x respectively, thanks!
@trickfun see the child issues of this issue, as you can see, some are fixed, some or not. When they are all fixed, we won't be using jQuery any more. Libraries are only used when they're needed, so the more core libraries don't use jQuery, the more actual sites/pages on sites will be vanilla.
Looks great. Might be the quickest we've been compliant with a coding standard after it's agreed? Committed/pushed to 11.x/11.0.x/10.4.x/10.3.x respectively.
It might be worth a follow-up to make this work for any entity type, we could add an 'entity' option and deprecate the node one probably. MR looks fine but I'm short for time and didn't review enough to actually commit.
Actually two questions.
Nodes are marked for reindex in Node::postSave()
// Reindex the node when it is updated. The node is automatically indexed
// when it is added, simply by being added to the node table.
if ($update) {
node_reindex_node_search($this->id());
}
If we could detect that a translation is being deleted there (can we?) then we would not need to mark the node for indexing, and then delete it again in the hook implementation added here.
Or... is there somewhere else that has this information instead?
One question on the MR.
The starshot group can include as much product people as necessary under the auspices of the technical working group though? I only suggested this to save time, if it doesn't save time, it's not worth considering - but also I will note there is no other issue yet, so when is that expected?
Starshot is not supposed to be lead from a technical standpoint but rather a user needs / product standpoint
It needs to be a combination of these - you can't meet the user / product needs without good technical foundations.
Using attributes in core but recommending both attributes and annotations for contrib seems good. We would need to work out the mechanics of it, but it would be good to run core tests on phpunit 11 by default for Drupal core (composer update maybe once we're compatible?) but lock to phpunit 10 for contrib (maybe with the option to upgrade too, or something like that). Thanks for doing the investigation here, really good to know it's not going to be as bad this time.
Looks good to stop the random failure, we can fix the overlap issue with content preview on in a follow-up I think.
catch → created an issue.
The recipes that make up Standard are meant to be generic, reusable parts -- the kind of things you'd likely need on any site. Umami is a lot more specialized. Do we really expect, for example, that most (or even many) sites will want to have a "recipe" content type, set up in the specific way Umami does it?
I'm in two minds about this - I agree that there are very few bits of Umami that would be re-usable.
However, Umami is also supposed to be a showcase of what can be done with Drupal core and a bit of theming, so if we include recipes in 'what can be done with Drupal core', it might make sense to have the recipes more composable so that it's easier to see how it was put together? I don't know if we'll show installed recipes in a UI somewhere, but if we do, then having Umami made up of smaller recipes would appear in that UI for example, or just if people are looking through the code base to see how it was done.
fwiw I don't think 📌 Umami views should use responsive grid Active needs to be a hard blocker here, we could do it after a recipe conversion too, but also it's nearly ready and just needs review. Didn't realise there was already progress on this issue.
Alpha and beta dates for 11.1 should count backwards approximately two weeks from the release date because it's a minor.
I think there are permissions issues on the test only job when the MR was opened by a core committer.
It's fine to set the ceiling on the OpenTelemetry tests 500k higher than whatever the eventual number is, that way other small CSS/JS changes won't cause this MR to fail.
Committed/pushed to 11.x and 11.0.x, thanks!
I think it would fine if we only blocked it for core tests, but it would be good to 100% enforce it in core because it is easy to do this without realising - as we've found out.
I think this would be worth silencing, but we should probably also open an upstream issue against OpenTelemetry PHP to add the return type hints.
Committed/pushed (from needs review again...) to hopefully get tests back to green. Very nice find and fix for what looked like bizarre test failures, good to know it's just yet another variation of dealing with funny branch names and not anything more tricky.
Given the number of people running into this that seems worth looking into yeah.
My only concern about this being in the development guide is that it a lot of people who don't consider themselves developers would benefit from being able to run a local environment, but that's something that can done separate to this change - once we've got a single page, we can move it around in the IA if necessary. It seems like a good spot to concentrate on.
What I'd love to see on https://www.drupal.org/docs/develop/local-server-setup → is at the very top the commands to get an install running if you already have ddev installed on your computer, and a link to 'how to install ddev' for people who don't.
catch → changed the visibility of the branch 3445211-fix-drupal-version-constant to active.
@JonMcL it's because Facets was incompatible with using GET for AJAX requests, both the contrib views_ajax_get module and the core change that went in to use GET by default. You're better off with the facets patch.
Pretty sure we should have done this instead of 🐛 Update composer metapackages Active so I've reverted that commit and going to go ahead here.
Committed/pushed to 11.x, thanks!
Reverted this because it seems to be causing subtree splits to fail, switching to 🐛 Composer tests fail because of invalid Drupal version Fixed .
I had a play with
$container->registerForAutoconfiguration(DestructableInterface::class)
- ->addTag('needs_destruction');
and didn't get anywhere. Maybe every services.yml needs to specify autoconfigure: true for even this to work? I guess we could do that but it feels like a bigger change than the scope here.
Instead, I'm iterating over the service definitions where we collect the tagged services, and adding the tag to them there, this seems to work (at least the one test I'm running locally works, as did some manual testing). This was @longwave's suggestion in slack as one of several possible options.
Maybe we can do this to un-fatal things then open a follow-up to use autoconfigure more?
Doh it's because the classes are in the same namespace so we don't need a use statement at all. Thanks @longwave.
Pushed up a second branch with another idea which I think is closer to what @dpi is asking for - if we autoconfigure the tag for services with the interface, then decorating services won't get the tag, but inner services still would. However, it's not actually working. We do have one example of autoconfiguring tags in core, but I've never tried it before, hopefully a similarly silly mistake.
#7 was my first thought, but it would probably be good to do something in core so it doesn't crash, even if that's an instanceof check before calling the destruct method.
https://git.drupalcode.org/project/drupal/-/jobs/1537367 is green with some deprecation notices from OpenTelemetry itself, which we can't do much about except fix upstream (and doesn't fail the job).
So just the --verbose to drop.
This is basic “coordinated release” territory where a major initiative with a less than 1 year (~238 days at time of my writing) deadline would already have its documents written and ready to publish the moment the Dries presentation was completed in order to effectively recruit volunteer’s.
You are imagining large teams of people working on this for long periods of time, neither of which exist, hopefully new volunteers can help to write some documentation. It is much easier to do that in public than purely in private, so hopefully it will start soon.
https://www.drupal.org/association/blog/drupal-lead-dries-buytaert-annou... → was extremely misleading when first posted (it's better now but still not great IMO) so not a good basis for documentation.
What exactly is Starshot (is it just a typical Drupal distribution?)
It will be Drupal core + a recipe (likely a collection of recipes using the new Recipes API + the necessary contributed modules to enable that recipe.
This will be 'just a typical Drupal distribution' in the sense that once Recipes is stable in core, install profiles as they currently are will be deprecated. The difference is though that unlike install profiles, you can mix and match recipes and aren't tied to one for the lifetime of the site.
What is Starshot’s new governance model?
TBD. It doesn't exist yet. It won't be exclusively core committers though.
What is the initiative road map?
TBD but there are some existing things it will be built on top of which are already well in progress / running separately:
1. Recipes
2. Project browser and automatic updates (so that 'starshot' can be downloaded from a new install of Drupal core, not just as a separate composer package).
3. Layout builder UX/functionality improvements.
Anything else is TBD, it is likely that it will include some very highly used contrib modules like pathauto in the recipe(s) but specifics of this are also TBD.
I think it's worth knowing the answer to #15 even if it turns out to be a dead-end.
In general thanks for the answers above, it's good to know the low hanging fruit are covered even if that means there's not much low hanging fruit to get things down under 300kb...
Splitting by glyphs sounds like it could reduce size a lot, and in general single users don't load lots and lots of different languages that much even if sites have them - so duplication would be less of an issue than other ways of splitting. However, I have no idea how to split a font by language/glyphs - we wouldn't want to arbitrarily remove support for various languages since in theory we support anything, so it'd be a case of taking them out by default but adding them back when needed?
Related to this has anyone tested what happens with this font and CJK languages, arabic, farsi?
PHPUnit 10 is done! I wonder if we want to keep this open to track issues with the change record and/or discovered by contrib modules once thy start testing against it (or a new issue to track that?) otherwise I think we can close this too.
Should we be doing an instanceof check for destructable interface in DrupalKernel::terminate()?
And/or should hux be removing the terminate tag from the service definition?
I'm not sure that #17 is a review of the correct MR, it does all those things except UserController.
I think we're done?
Tagging this so it's easier to find in 'things that would be good to do for 11.0/10.3 beta'.
Opened 🐛 config_snapshot can contain uninstalled and removed modules Active for the config_snapshot issue, going to move this back to fixed.
Committed/pushed to 11.x and cherry-picked to 11.0.x, 10.4.x, 10.3.x, thanks!
Needs some phpstan tweaking, but class alias seems like a good plan to me.
Add an extra check in NavigatioShortcutsBlock::build() to ensure that Shortcut module is enabled
This is probably a good idea for a quick fix to stop the fatal error. Can then figure out better how optional support from different modules should be added.
A question for me is if a module wants to add itself to the toolbar on install, and not just as a menu link, should it be able to do that? If so, then implementing that in shortcut module would provide an example.
The same MR will need to go into 11.x, 10.4.x and 10.3.x
@smustgrave not sure what you mean here, this is fixing a bc layer we already added to 10.3 so it (hopefully) works for real contrib modules.
It's a bit of a strange one because we're providing bc for the user login form behaviour, which normally we don't provide bc for form behaviour, but because it's the user login form and there isn't a proper API for this stuff, lots of modules actually interact with it. However the previous issue adds an API which contrib modules can implement, and that should hopefully make bc easier to implement if we change anything else in the future.
Once the bc layer works, we can leave it as-is in 10.x and 11.x, and then just drop it in 12.x
Realised we don't actually want separate 10.x and 11.x MRs because the deprecations are for removal in 12.x meaning all the same code is in 11.x too, so hid the 11.x MR.
catch → changed the visibility of the branch 3444978-userauth-bc-layer to hidden.
Still a waste to load the italic variation if it's never going to be used.
The ckeditor team wrote up how they optimized their variable font: https://ckeditor.com/blog/optimize-font-size-with-variable-fonts/ - like separate italic and non-italic variations (where there's apparently no benefit to a single file at all).
Did the second step, and a separate 11.x to match the 10.4.x/10.3.x changes.
catch → changed the visibility of the branch 3444978-userauth-bc-layer to active.
Short on time but did the first bit.
Adding a new class and deprecating the entire UserAuth class sounds good, ideally we'd land this before the beta release but that's not long, however a deprecation during beta is better than completely failed logins.
catch → changed the visibility of the branch 3444978-userauth-bc-layer to hidden.
catch → made their first commit to this issue’s fork.
Cause if we will have lot of font variations this variant (one file for all styles) looks much better.
There's a limit to how much text can be shown in the navigation bar at once, how many variations can there be?
@kostyashupenko mentions splitting italic and the different font weights out, and that seems like a good start. Can still ship them and define the rules, but if they're not used by default, they wont be loaded - can also only pre-load what we use by default then.
MR is green now.
@quietone I think the proposed resolution is still current and somewhat actionable, it's just hard. We need to decide which of the current 10+ local development environment documentation pages, in which of the many documentation sections, will remain. We then need to update the content (to recommend ddev) if it's not up-to-date. At the moment, it's not obvious which one is the 'canonical' one that should remain so figuring that out is the first step.
The links in #16 and #17 should be added to the issue summary (at least, possibly more in the issue) so we have a comprehensive list of pages to remove/redirect too.
Once that that canonical page is identified and is good enough, all of the other documentation pages which duplicate need to be deleted/redirected so that there is a single source for the information which can be maintained/improved over time. This is likely to involve editing other areas of docs too - e.g. if they're part of a multi-step howto, then that somehow needs to incorporate referencing the canonical docs instead of duplicating it.
Noticed this in
🐛
BigPipe injecting Local Actions block creates large janky layout shift in Claro
Needs work
as well, we should probably always output a ul even when there's only one link, could just use #item-list with a theme suggestion.
I think there's another issue around tackling the same thing, although I wasn't able to find it immediately - just noting here in case I find it again.
Split the test improvements change out to 📌 Add extra page request to the across pages asset performance test Active .
Split the Umami theme change out to 🐛 Umami page.tpl.php breaks block placeholders Needs review .
Not yet, but the logic could switch over once navigation is stable.
Committed/pushed to 11.x, see if this helps.