🇺🇸United States @mglaman

the design doesn’t have the menu tabs.

I think that's because it was intended not to be part of /admin/content and friends. That would remain for structured content (nodes, media.) So maybe we keep this as is an open a follow up to clarify/decide? 1) decide tab order, 2) move out of /admin/content

We could try to guess the tab order. But I have a feeling it'd be buggy or random with Views in the mix.

I have a weight set in my link definition. Maybe removing it will fix it? I'm not sure as the ordering between tasks and menu links is not associated at all

Linking to ✨ [PP-1] Add a local action to the pages listing to create a new page Active as it adds the collection route as well.

Got in some tests and fixed issue reported by @tedbow

I only got to dumping some notes in a draft MR

We had this in Commerce and added these helper methods to our entities: https://git.drupalcode.org/project/commerce/-/blob/3.x/src/Entity/Commer...

Seems like we need similar helpers to make sure referenced entities are translated.

I got this

did the change

mglaman → created an issue.

1. Someone may not have access due to hook_entity_access or hook_jsonapi_entity_filter_access which replaces the entity with an inaccessible object
2. Someone may implement hook_field_access and some fields aren't in the response for anonymous users

I'm am thinking having requests shown as unauthenticated by default would be best and give more realistic examples. I think it could start as a checkbox as "Use authentication". Words are hard, maybe borrow whatever the OpenAPI doc viewers use. Eventually it'd be cool to allow picking a Consumer for Simple OAuth to demo the scopes.

mglaman → created an issue.

It has to be something related to the dataDependencies in a code component on the site, but I'm not sure how to track that down.

I also think it proves there's something broken in this libraries.yml logic

canvasData.v0:
  dependencies:
    - canvas/canvasData.v0.baseUrl
    - canvas/canvasData.v0.branding
    - canvas/canvasData.v0.breadcrumbs
    - canvas/canvasData.v0.pageTitle
    - canvas/canvasData.v0.jsonapiSettings
  drupalSettings:
    canvasData:
      v0: []

canvasData.v0.baseUrl:
  dependencies:
    - core/drupalSettings
  drupalSettings:
    canvasData:
      v0:
        baseUrl: null

canvasData.v0 depending on canvasData.v0.baseUrl does not cause canvasData.v0.baseUrl to be null but canvasData.v0 is null (or canvasData is actually empty)

Fix isn't great. I also couldn't reproduce locally but can on a hosted site.

mglaman → created an issue.

Seems like this is really about adding MenuTreeResource. I don't like how this is implemented as piggy-backing the existing resource, decoding the response, and transforming it. It could easily be done as another service.

We could add `MenuTreeResource` and a supported route. That resource uses a tree storage. I don't think it should be exposed automatically but it can be via a feature flag ($settings or config)

mglaman → created an issue.

mglaman → created an issue.

It's not Gin's fault. See

That's because the theme switches from gin to xb_stark.

It's the Canvas theme negotiator's fault.

mglaman → created an issue.

mglaman → created an issue.

here are 132 JavaScriptComponents,

I have no idea. Maybe the AI agent made 132 components.

Still awaiting that confirmation from @mglaman — or whomever else can confirm.

I won't be able to review until may a week and a half from now.

Added to the merge train! Thanks

I don't think this is really postponed on needing more info if the concern is BC.

Fixing the description is a workaround. But 🐛 Improve 'administer content' permission description Needs work is much more verbose. I guess we could link and close this as a dupe. But fixing the description does not fix this issue.

Merged!

LGTM!

Thanks! Merge train ahead

mglaman → created an issue.

mglaman → created an issue.

Looks good! Added to merge train

Tried to rebase via the UI and it failed on conflicts, otherwise good to go

On my side it is not enough when css aggregation is ON.

Ah! I forgot to mention we currently have CSS aggregation off due to early conflicts w/ Experience Builder (now Canvas) and I haven't re-tested with it on in a while.

Read it over. Looks good to me! I like that we're getting the Composer binary path from within it's vendor directory versus vendor/bin/composer.

chatted w/ phenaproxima and I understand the problem, now.

Sometimes after running an update, the cache becomes corrupted and has to be cleared.

This confused me. It's not _update_ as in Drupal database updates but dependency updates. A module is update which has modified it's service constructors. Drupal is still using a previous container cache for the previous version of the module. When the user goes to their Drupal site, the container constructs the services using previous arguments and we WSOD.

📌 Use a better container cache key Active solves this by using dependency version information to vary the container cache. So I think Drupal CMS can provide a Composer script which _kind of_ does the same thing. Without calling `drupal_rebuild`. When calling `drupal_rebuild` all caches get destroyed, which can be bad ahead of actually running updates.

Instead it should have a script which does clears the container cache only. This can be done via drush cc container or a PHP script like:

$autoloader = require_once __DIR__ . '/autoload.php';
$request = Request::createFromGlobals();
$kernel = new DrupalKernel('prod', $class_loader);
$kernel->setSitePath(DrupalKernel::findSitePath($request));
$kernel->invalidateContainer();

Chiming in that we've been using a patch for this since March 11th.

diff --git a/gin.libraries.yml b/gin.libraries.yml
index 45f93acd..b1212444 100644
--- a/gin.libraries.yml
+++ b/gin.libraries.yml
@@ -14,7 +14,7 @@ gin_base:
       dist/css/theme/variables.css: { minified: false }
   dependencies:
     - gin/tabs
-    - gin/dialog
+    - gin/gin_dialog
     - gin/status

 # Legacy
@@ -165,7 +165,6 @@ gin_dialog:
       dist/css/theme/dialog.css: { minified: false }
   dependencies:
     - gin/dialog
-    - gin/gin_base
     - gin/gin_accent

 gin_description_toggle:

Although I haven't exactly tested without the patch in a while.

++ great idea

I think 📌 Support redirects to non-content entity pages for example a view Active may have fixed this.

Looks like I missed this issue when merging #3180737: Add path_alias module as dependency. → :/. I'll make sure to save credit since it was my mistake to not credit before and merge issues.

Thanks!

Rolling on the merge train. Thanks!

Sorry, didn't catch the fail on type hinted constants breaking previous major

LGTM. Basically this allows redirects to always return their value and not a 404, which is how external redirects work anyway

Now that 📌 Only register decoupled_router.redirect_path_translator.subscriber if the Redirect module is installed Active is in, can we rebase off of that and tidy that up here?

Thanks!

mglaman → changed the visibility of the branch 3111456-resolve-langcode-issue to active.

given this actually works on the old redirect repository as we're only passing in an extra parameter.

oh, I didn't test so I thought it was a breaking change. so it's basically backwards compatible? I see, `findMatchingRedirect` added cacheability. And for folks who don't upgrade we lose the cacheability because we no longer do this, correct?

    // If there is a response object, add the cacheability metadata necessary
    // for the traced URLs.
    array_walk($traced_urls, function ($traced_url) use ($response) {
      $response->addCacheableDependency($traced_url);
    });

LGTM if OK changing -dev requirement to ai -dev

That's because the namespace is `january_theme` but the code is `january`. See https://git.drupalcode.org/project/january_theme

We assume the project namespace matches the code in the theme/module.

Looks good to me!

10 | WARNING | Interface names should always have the suffix "Interface"
    |         | (Drupal.Classes.InterfaceName.InterfaceSuffix)

Didn't read phpcs fail, turns out coding standards dictated the answer.

I'll set this to 1.2.x, since I can't see any reasons that this would be a breaking change?

Yeah nothing should break. The interface isn't adding methods at all.

I'll try to do some more debugging vs just reading calls from the Blackfire graph and report back. We also have several Metatag contrib running.

mglaman → created an issue.

Left a review. I don't love the approach as sites could easily be in a broken state since we can't bump redirect. There's no peerDependency type declaration we could provide

mglaman → made their first commit to this issue’s fork.

Thanks!

Let's do it!

So 🌱 Guidelines for semantic versioning and Drupal core support Active led to https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... → and technically we should do a minor release if dropping 9.x.

🤷‍♂️ although does the minor version mean anything if we merge onto 2.x and just tag 2.1.0? It's not like we're supporting 2.0.x in any true fashion and backporting fixes.

D9 is long gone that I'm +1 to doing this in a patch release.

We're adding extra libraries to the editor using KernelEvents::RESPONSE => ['onRespond', 50] for $route_name === 'entity.xb_page.edit_form' || $route_name === 'experience_builder.experience_builder' and:

        $response->addAttachments([
          'library' => [
            'module/our-library',
          ],
        ]);

mglaman → created an issue.

I've had better luck with Playwright by using their sharding over multiple jobs. We have multiple jobs and pass `--shard=1/3` or `--shard=2/3`, etc to each. Then Playwright knows how to chunk the tests.

We've also used some tagging for specific tests as well, like `--grep=@multilingual`. But then other jobs need `--grep-invert=@multilingual`

Thank you for the work! Left a review

@narendrar ah yeah, I see. I was getting that exception. But the problem is that there are so many AI module exceptions without a way of knowing it is an AI module one without tracking each. I opened ✨ Provide an exception interface all exceptions implement Active so it'd be easier to catch this.

I'm guessing we should just always return the exception message regardless. We're not stable yet. ✨ Dispatch AiExceptionEvent when a provider throws an exception Active is a feature request I made that allows third parties to customize exception messages handled by integrations like XB AI.

Thanks, all!

LGTM!

Merged, thanks!

smustgrave → credited mglaman → .

This would be great for OpenAPI documentation and other JSON schema documentation generation of entities beyond Views.

which is not very helpful in this scenario.

How isn't it very helpful?

mglaman → created an issue.

mglaman → created an issue.

mglaman → created an issue.

@mglaman: that screenshot in the issue summary, which route is that for, and for how big of a component tree is it?

It was the GET on experience_builder.api.layout.get. The tree wasn't huge, maybe 12 components with various nested slots.