WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.48k stars 4.18k forks source link

Inserter: Social Link Pattern listed at the top of Call to Action category #55572

Closed annezazu closed 1 month ago

annezazu commented 1 year ago

Using TT4 and 6.4 RC1, I'm seeing a somewhat rogue social link pattern provided by Core listed at the top of the Call to Action category:

https://github.com/WordPress/gutenberg/assets/26996883/f0681bc5-98ee-4e03-ba42-38f716fbb858

I'm not quite sure what the history of this pattern is but, from what I can gather, this was originally listed under "buttons" which then switched to "call to action". From there, for some reason, this pattern is shown at the very top of the inserter rather than theme provided patterns in this category. In comparison, other bundled patterns from Core are typically shown lower after theme patterns like so with TT4:

https://github.com/WordPress/gutenberg/assets/26996883/0ed996ef-0186-4541-ba2c-cbbffa2d922a

Off the top of my head, I'm not quite sure how these patterns are sorted once you open a category and have the default filtering in place. However, this individual pattern should be deprioritized in comparison to the rest of the patterns.

@richtabor I know this came up recently when we were chatting but my memory is failing on your thoughts here. Mind chiming in/taking a look when you can?

talldan commented 12 months ago

This one here from core by the looks of it (needs a trac ticket to update it): https://github.com/WordPress/wordpress-develop/blob/5333ea1f91892d3bf9c601404315dde4d273a1f2/src/wp-includes/block-patterns/social-links-shared-background-color.php#L4

It has a category of 'buttons' (makes some sense), but at some point all patterns in the buttons category were migrated to 'Call to Action' (which doesn't seem like a relevant category for this pattern): https://github.com/WordPress/wordpress-develop/blob/5333ea1f91892d3bf9c601404315dde4d273a1f2/src/wp-includes/rest-api/endpoints/class-wp-rest-block-patterns-controller.php#L27-L37

These migrations concern me a bit now that users can make categories. It might make sense to at least update patterns we have control over to use the right category names.

AFAIK, there's no way to order patterns (would be a feature request), and I'm not sure how the order is determined. I'd probably lean towards removing this one, as it seems fairly useless (users can insert a social links block)

richtabor commented 12 months ago

other bundled patterns from Core are typically shown lower after theme patterns like so with TT4:

AFAIK, there's no way to order patterns (would be a feature request), and I'm not sure how the order is determined. I'd probably lean towards removing this one, as it seems fairly useless (users can insert a social links block)

Because it's a core pattern (bundled in core itself) it's loaded before all other patterns (https://github.com/WordPress/gutenberg/issues/55512). I'd reverse that and instead have core patterns loaded last (but then again, we shouldn't need patterns bundled directly in core, but reference them from the Pattern Directory, as we do all the other patterns apart from this and the query loop patterns.

But I agree, it should be removed. That pattern is not particularly helpful. cc @mikachan, this is the pattern I mentioned in a previous conversation.

talldan commented 12 months ago

I guess the only thing with removal is any references to it in template will suddenly stop working 🤔

The only usages I could find though is from themes unregistering this pattern 😄 https://www.wpdirectory.net/search/01HE7TDH7G2D80R2Z53Q03AMP5

richtabor commented 12 months ago

Yea, it's unlikely that a theme would directly reference a core pattern in a template (especially this one). It's a pattern designed to be interested into content too.

mikachan commented 3 months ago

I've opened a trac issue for removing this pattern: https://core.trac.wordpress.org/ticket/61708#ticket, and a patch here: https://github.com/WordPress/wordpress-develop/pull/7063.

I can't see any other references to it in Core - there is a performance test that uses it as an example pattern here in GB, but I think it's OK to remove this and it's not directly referencing the bundled pattern, it's just a copy of the markup.

peterwilsoncc commented 1 month ago

This was resolved as part of Core-61708 and merged in r59019 / https://github.com/WordPress/wordpress-develop/commit/a78540b0881a195ec8488bb6bdf878114d75f8b8