gutenberg icon indicating copy to clipboard operation
gutenberg copied to clipboard

[Patterns]: Remove bundled patterns

Open ntsekouras opened this issue 3 years ago • 14 comments

What?

Resolves: https://github.com/WordPress/gutenberg/issues/46017

The GB bundled patterns have been added to Pattern Directory and tagged as core patterns with 6.1 WP version. With that we don't need to bundle them anymore.

Testing Instructions

  1. Ensure that the patterns from this comment: https://github.com/WordPress/gutenberg/issues/46017#issuecomment-1344742302 are loaded properly.
  2. You could just disable Gutenberg in a 6.1 installation. Since we tagged them as core patterns with 6.1 version in Pattern directory, they should just show up even without Gutenberg being active.

Notes

In my testing it seems I've encountered this issue that some images fail to load. If that's the case for others as well and not something with my environment maybe we need to update some urls, etc.. in the Pattern Directory patterns?

ntsekouras avatar Dec 10 '22 11:12 ntsekouras

Nice! What's a good way to test? Just looking at this PR, I still see patterns showing up in the inserter. But perhaps they are coming from the directory now? Screenshot 2022-12-12 at 10 46 12

Happy to ✅ if that's what's up.

jasmussen avatar Dec 12 '22 09:12 jasmussen

@jasmussen I think the best way to check would be to just disable Gutenberg in a 6.1 installation. Since we tagged them as core patterns with 6.1 version, they should just show up. (I'll also update the description)

ntsekouras avatar Dec 12 '22 10:12 ntsekouras

Rather than trying to use a specific Gutenberg plugin for this PR, I followed what you said above and just disabled Gutenberg on a 6.1.1 install. At first, everything was working as expected, except the Header category included some "banner" patterns. I also noticed the navigation block was acting strangely, requiring that a new menu be created. After I did that, the patterns entirely broke:

https://user-images.githubusercontent.com/26996883/207214945-c06dab7c-188b-4a7c-9ee6-3f4952dd72b5.mov

Using 6.1.1, no Gutenberg, TT3. It broke other pattern categories too, like Gallery, but not Buttons.

annezazu avatar Dec 13 '22 02:12 annezazu

Rather than trying to use a specific Gutenberg plugin for this PR, I followed what you said above and just disabled Gutenberg on a 6.1.1 install. At first, everything was working as expected, except the Header category included some "banner" patterns. I also noticed the navigation block was acting strangely, requiring that a new menu be created. After I did that, the patterns entirely broke:

Thanks for testing Anne! Is this something known for Pattern Directory that can be fixed? I remember similar issues before and I had the impression they were fixed.. --cc @beafialho @ryelle

ntsekouras avatar Dec 13 '22 07:12 ntsekouras

the Header category included some "banner" patterns

Can you point out which patterns these are? Maybe the patterns in the pattern directory need to be recategorized? (note that we haven't implemented the new categories yet, so this might be expected)

I also noticed the navigation block was acting strangely, requiring that a new menu be created.

Looks like the pattern from the directory includes a ref, while the patterns deleted from this PR omit that. The ref'd menu 80 doesn't exist on your site, so it errors. Maybe we can strip out the ref when outputting the pattern code from the directory?

<!-- wp:navigation {"ref":80,"layout":{"type":"flex","justifyContent":"center"}} /-->

After I did that, the patterns entirely broke

I think you're still in the Navigation block, so you're restricted to what blocks you can insert. That's why you can insert the social links pattern but nothing else.

ryelle avatar Dec 13 '22 17:12 ryelle

Looks like the pattern from the directory includes a ref, while the patterns deleted from this PR omit that. The ref'd menu 80 doesn't exist on your site, so it errors. Maybe we can strip out the ref when outputting the pattern code from the directory?

This should be omitted in the pattern directory, right? If it's not omitted there, then it can not work in any site that doesn't have a menu with that ref(with the PD flow to copy the pattern).

the Header category included some "banner" patterns

What do you mean banner? There is no such category as far as I know.

ntsekouras avatar Dec 13 '22 17:12 ntsekouras

This should be omitted in the pattern directory, right?

I believe the nav block is created with a ref by default, either that or they were created in the directory with a menu (maybe because @beafialho has higher permissions than a regular user). We don't do anything to override the way the block is saved. In any case, I've already created an issue to fix this in the directory.

ryelle avatar Dec 13 '22 17:12 ryelle

Can you point out which patterns these are? Maybe the patterns in the pattern directory need to be recategorized? (note that we haven't implemented the new categories yet, so this might be expected)

Here are the names:

  • Media and text in a full height container
  • Media and text with image on the right
  • Media and text with image on the left
  • Larger header with text and a button
  • Larger header with left aligned text
  • Event

annezazu avatar Dec 15 '22 01:12 annezazu

@annezazu I've updated those patterns in the pattern directory, so they've been removed from the Headers category and added to the Banners.

ryelle avatar Jan 06 '23 22:01 ryelle

Noting for clarity (mainly for my future self) that we're blocked right now due to this issue that remains to resolve in the pattern directory: https://github.com/WordPress/pattern-directory/issues/545

annezazu avatar Jan 16 '23 21:01 annezazu

What do you mean banner? There is no such category as far as I know.

I also just saw this and wanted to note the banner category being added previously: https://github.com/WordPress/gutenberg/pull/44203

annezazu avatar Jan 19 '23 13:01 annezazu

Noting for clarity (mainly for my future self) that we're blocked right now due to this issue that remains to resolve in the pattern directory: https://github.com/WordPress/pattern-directory/issues/545

This has been fixed in the pattern directory, so it should be good to go 👍🏻 (there might still be old patterns in cache for a few hours, in case anyone jumps on this right away)

ryelle avatar Jan 27 '23 17:01 ryelle

This has been fixed in the pattern directory, so it should be good to go 👍🏻 (there might still be old patterns in cache for a few hours, in case anyone jumps on this right away)

Thank you @ryelle! I tested and this looks good! I'd need someone else to test too and 🟢 this PR to land. -- @annezazu @jasmussen

ntsekouras avatar Jan 28 '23 09:01 ntsekouras

P.S. I noticed that some patterns like "Header with hero image" are rendered broken because the image fails to load.

It seems we have the same issue for Cover blocks, as it was for Image. -- cc @ryelle

ntsekouras avatar Jan 30 '23 15:01 ntsekouras

It seems we have the https://github.com/WordPress/pattern-directory/issues/443 for Cover blocks, as it was for Image.

That issue was for the pattern creator, inserting images from the Openverse media flow, and was fixed (for all images in the openverse client).

The issue you're seeing sounds like this one, which should have been fixed with this PR. In fact, if you copy the pattern content directly from wp.org, it works. So I think there's some extra encoding/sanitization on the core side that's converting the &amp; to \u0026amp;. I think it's the wp_kses_post here.

If there's anything that should change on the API side to fix this, add a new issue here.

ryelle avatar Jan 30 '23 18:01 ryelle

P.S. I noticed that some patterns like "Header with hero image" are rendered broken because the image fails to load.

+1.

Here's a screenshot of header patterns, loaded with Twenty Twenty Three:

CleanShot 2023-01-30 at 14 45 31

richtabor avatar Jan 30 '23 19:01 richtabor

Tested and can confirm the images failing to load. Of note that they are broken after adding too. For example:

Screen Shot 2023-01-30 at 6 30 35 PM

annezazu avatar Jan 31 '23 02:01 annezazu

I'm going to merge this for now and handle separately the link issue for Cover blocks, since these patterns are not even shown since the core keyword` has been added in PD. Thank you all!

ntsekouras avatar Feb 01 '23 09:02 ntsekouras