Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revisit the bundled block patterns #1103

Conversation

MaggieCabrera
Copy link

This is a follow-up on WordPress/gutenberg#28921 since I discussed with Kjell that this would be better living in core with the rest of the patterns. Follow the discussion on the linked Gutenberg issue for all the details on these patterns. Please Kjell let me know if I should change the wording on the descriptions and titles of these patterns, as well as the categories if the patterns should be organized in a different way.

In order to ensure these patterns will work correctly on any theme, I tested them using Empty Theme, which adds no block CSS and no custom colors to the editor.

Trac ticket: https://core.trac.wordpress.org/ticket/52846


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@youknowriad
Copy link
Contributor

Is this patch up to date with what has been merged in Gutenberg?

Can we get the phpcs issues fixed?

@MaggieCabrera
Copy link
Author

Is this patch up to date with what has been merged in Gutenberg?

Can we get the phpcs issues fixed?

It is not! I will get to it tomorrow and be sure that the format is correct

@MaggieCabrera
Copy link
Author

@youknowriad I just updated this with the latest from trunk in Gutenberg. Composer format didn't bring any errors for me.

*/

return array(
'title' => _x( 'Heading', 'Block pattern title', 'default' ),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these 'default' domain arguments should be dropped since we're already on Core.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's fine though. @swissspidy @gziolo do you know what is the guideline for Core?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It defaults to default so it can be dropped and in practice it usually is.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads up, I just removed all of them 👍

@ryelle
Copy link
Contributor

ryelle commented May 28, 2021

Committed in r50794.

@ryelle ryelle closed this May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants