🇹🇼Taiwan @johnalbin

Do we want to mention `nvm use` before the `npm install` command? That's definitely an optional step though, I'd bet it still builds on newer node.js versions.

Since this is the contributing doc, shouldn't we be doing a git clone of the experience_builder source during setup?

composer require --prefer-source drupal/experience_builder

The will add eb to the composer.json file and do a git clone all in one step. HOWEVER, the git clone will be checkout out to the latest release tag and not the needed development branch. So we'd need to add this line after:

cd modules/contrib/experience_builder; git checkout 0.x

johnalbin → created an issue.

Currently only IE browser is not supporting webp images. IE browser will reach eol on June 15, 2022.

webp image should be loaded as fallback image

I completely agree.

However, you can already have a webp fallback image by configuring the fallback image to use an image style that has Drupal Core's "Convert" image style filter (set to convert to webp, of course).

There's no need to unconditionally convert jpegs, etc to webp as this patch does.

nicxvan → credited JohnAlbin → .

I see this all the time.

The problem is on the Drupal side, not on the Next.js side.

The secret is generated when the initial page is created on the Drupal side. By default it has a 30 second timeout. But the user still has to read the page, determine what they want to do and then click the "preview" button. A lot of times that's already more than 30 seconds, so the secret has expired before the user clicks the "preview" button. But the expiration isn't check until the user clicks the button, the request is sent to the Next.js side, the Next.js code then sends a request back to the known Drupal side and finally that Drupal side checks if the secret has expired.

Setting the default to some value great than 30 seconds also won't really fix this issue. Because the page with the "preview" button can be served from cache! And that means the expiration is 30 seconds after the cache was created not 30 seconds after the user views the page with the "preview" button.

The fix will have to be to add an authenticated route that creates a secret on the fly and then redirects to the Next.js side. Then a 30 second expiration will actually be useful.

JohnAlbin → created an issue.

The patch in #61 is malformed because your .editorconfig stripped trailing spaces from the file. I hate when that happens. :p

I pushed @rhristov's change to GitLab.

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

Oh! I forgot to mention this happens when editing menu links too. So it doesn't happen in just the link field's autocomplete.

This bit me recently. It appears the autocomplete returns the 10 most recent nodes ordered by creation date. So if you created an "About" page a long time ago and then later created 10+ nodes that included the word "about" somewhere in the title, you can no longer see the "About" page in the autocomplete list when typing "About" in the link field. And because you've already typed the exact node title you want, there's no way to make Drupal return that result.

I think Jason → ’s solution of sorting by title length is brilliant! Because it means you will always be able to find the node title you want by typing more of the title in the autocomplete.

Here's an example:

1. I type "about" in a link field and Drupal returns 10 items sorted by title length:
   • About
   • About me
   • About CSS
   • …
   • About Drupal
2. I examine the list to see if what I want is there.
   • If what I want is exactly "About", then it's the first option!
   • If what I want is longer than "About Drupal", it's not in that list and I'll have to type more letters into the link field.
3. I didn't want anything in that first list, so I type "about my" and Drupal returns a new list of 10 items sorted by title length:
   • About my job
   • About my dog
   • About my team
   • About my dreams
   • About my team's dogs
   • …
   • About Mycenaean Greece
4. I repeat step 2, type "about my dog", and see the desired title, "about my dog's favorite chew toy".

The only problem this solution doesn't solve is if you have 11 nodes all named "About" exactly, but I think that's a content problem, not one Drupal needs to account for.

I've updated the issue summary to show the proposed solution and (hopefully) to make it easier to understand the problem.

Still working on this. The solutions in 🐛 Improve documentation of hook_theme_suggestions_HOOK_alter() Fixed and 📌 Refactor out theme hook suggestion building from ThemeManager::render() into a separate function. Fixed need to be refactored so they can work with this issue.

It looks like all the new code handling for 🐛 Improve documentation of hook_theme_suggestions_HOOK_alter() Fixed has been reverted during a rebase and force push to the GitLab branch.

I'll work on properly porting this to the main (11.x) branch.

Fix broken link

The change in #2 work great at getting rid of the deprecation error. But it unintentionally causes the list of punctuation characters to be re-generated every time pathauto needs that list (instead of caching it in a array).

I think this is very close to RTBC.

I was just coming here to do the same thing as Joël did.

Welcome, Rajab!

JohnAlbin → created an issue.

Thanks for the review, Qiangjun!

Why not h1?

Yeah, that's weird. The original issue points to the excellent drupal.org guidelines on how to write a README.md and then the patch from the same company doesn't even follow those guidelines. It's almost as if they are copying and pasting the issue description across multiple projects in hopes of maximally getting issue credit with little to no actual contribution to those projects. :-\

@znaeff

Sorry, no. An update hook is the wrong fix. As I said earlier:

When service definitions are changed, a module MUST provide an empty post_update hook to ensure that the caches are cleared.

Change record: "Use hook_post_update_NAME instead of hook_update_N to clear the cache" →

API: hook_post_update_NAME