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

Block supports: migrate spacing, border, color, dimensions, elements and typography and associated tests. #2500

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Apr 4, 2022

Gutenberg plugin migration to WordPress 6.0 also being tracked in WordPress/gutenberg#39889.

This diff migrates block support spacing, border, color, dimensions, elements and typography and associated tests from Gutenberg 12.9.0 to WordPress 6.0

The goal is to include the latest plugin, while skipping the Style Engine integration, which only features in spacing.php. The Style Engine class doesn't bring any new functionality - it's more of a first iteration in a set of long-term changes.

In summary:

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


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.

@ramonjd ramonjd changed the title Block supports: migrate spacing Block supports: migrate spacing, border, color, dimensions, elements and typography and associated tests. Apr 4, 2022
@gziolo
Copy link
Member

gziolo commented Apr 4, 2022

We already have a Trac ticket https://core.trac.wordpress.org/ticket/55505 that @hellofromtonya opened for all backporting necessary for the Gutenberg plugin.

@aaronrobertshaw
Copy link

The block support serialization changes included in this PR so far (all except layout.php) look good and cover those introduced in WordPress/gutenberg#36293.

I'm happy to review again once the layout.php updates are also added.

@gziolo
Copy link
Member

gziolo commented Apr 5, 2022

I'm happy to review again once the layout.php updates are also added.

This patch looks big already. Can we land that part separately? I could do some additional review pass in 1-2 hours and commit changes if no blockers discovered.

* @param WP_Block_type $block_type Block type.
* @return bool Whether to serialize spacing support styles & classes.
*/
function wp_skip_dimensions_serialization( $block_type ) {
Copy link
Member

Choose a reason for hiding this comment

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

@peterwilsoncc or @hellofromtonya, should we keep functions like this one with no code and depreciation notice for backward compatibility or is it fine to remove as they were marked as private?

Copy link
Contributor

Choose a reason for hiding this comment

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

The practice is to move the functions to deprecated and throw a warning, for example:

/**
* Formerly used internally to tidy up the search terms.
*
* @since 2.9.0
* @access private
* @deprecated 3.7.0
*
* @param string $t Search terms to "tidy", e.g. trim.
* @return string Trimmed search terms.
*/
function _search_terms_tidy( $t ) {
_deprecated_function( __FUNCTION__, '3.7.0' );
return trim( $t, "\"'\n\r " );
}

Unfortunately plugin authors have a habit of ignoring private access statements. To be fair, some of this is self inflicted by things such as WP_List_Table still being marked as private.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks folks! If we decide it's a good idea to reinstate them, I've thrown up a PR here: #2522

@ramonjd
Copy link
Member Author

ramonjd commented Apr 5, 2022

This patch looks big already. Can we land that part separately? I could do some additional review pass in 1-2 hours and commit changes if no blockers discovered.

Layout over here: #2518

@gziolo
Copy link
Member

gziolo commented Apr 5, 2022

Committed with https://core.trac.wordpress.org/changeset/53076.

@gziolo gziolo closed this Apr 5, 2022
ramonjd added a commit to ramonjd/wordpress-develop that referenced this pull request Apr 6, 2022
@ramonjd ramonjd deleted the migrate/wordpress-6.0-block-supports-spacing branch April 6, 2022 00:43
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.

5 participants