Make WordPress Core

Opened 18 months ago

Closed 18 months ago

Last modified 17 months ago

#58586 closed task (blessed) (fixed)

Block.json: Refactor and stabilize selectors API

Reported by: ramonopoly's profile ramonopoly Owned by: isabel_brison's profile isabel_brison
Milestone: 6.3 Priority: normal
Severity: normal Version: 6.3
Component: Editor Keywords: has-patch has-unit-tests gutenberg-merge
Focuses: Cc:

Description (last modified by ramonopoly)

This ticket tracks the backporting of PHP files for the following Gutenberg changes:

The selectors API is stabilized as part of this refactor.

cc @aaronrobertshaw

Change History (22)

This ticket was mentioned in PR #4655 on WordPress/wordpress-develop by @ramonopoly.


18 months ago
#1

This is a backport PR for WordPress 6.3 that includes the following PHP Gutenberg changes:

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

cc @aaronrobertshaw

#2 @ramonopoly
18 months ago

  • Description modified (diff)

@ramonopoly commented on PR #4655:


18 months ago
#3

@ramonjd

A pretty rough start. I wasn't sure whether the following change to blocks.php had to be ported.

https://github.com/WordPress/gutenberg/pull/46496/files#diff-12ada35a09133c5735dcc286bb738745777c30420bc40dbc04d751db5ecfe349R21

@ramonopoly commented on PR #4655:


18 months ago
#4

Didn't edit unit tests yet. Failures are expected 😄

@aaronrobertshaw commented on PR #4655:


18 months ago
#5

A pretty rough start. I wasn't sure whether the following change to blocks.php had to be ported.

I can confirm, the changes referred to are expected to remain Gutenberg-only until 6.3 is the minimum supported version. That filter is there to ensure the selectors configuration from block.json metadata is honoured.

Didn't edit unit tests yet. Failures are expected 😄

Most of the test failures will relate to the removal of the "editor-only" selectors from the API.

The theme.json tests look to be failing due to the image/block.json not being updated yet.

@ramonopoly commented on PR #4655:


18 months ago
#6

The theme.json tests look to be failing due to the image/block.json not being updated yet.

🤔 I think we should revert that test until the block library package is backported. That should happen early next week.

Most of the test failures will relate to the removal of the "editor-only" selectors from the API.

I've fixed these for now.

I noticed that https://github.com/WordPress/wordpress-develop/pull/4619 is backporting the duotone changes only, so it looks like we'll have to port over the rest of the changes from

I've also removed the following test from Tests_Theme_WpGetBlockCssSelector, which was failing and I couldn't work out if it was required since the API was stabilized or because there are dependencies in https://github.com/WordPress/wordpress-develop/pull/4619.

public function test_get_duotone_selector_via_experimental_property() {
                $block_type = self::register_test_block(
                        'test/experimental-duotone-selector',
                        null,
                        array(
                                'color' => array(
                                        '__experimentalDuotone' => '.experimental-duotone',
                                ),
                        )
                );

                $selector = wp_get_block_css_selector( $block_type, 'filters.duotone' );
                $this->assertEquals( '.experimental-duotone', $selector );
        }

More to work to come on this one...

@ramonopoly commented on PR #4655:


18 months ago
#7

I'm getting 1 failure for Tests_Theme_wpThemeJson::test_get_stylesheet where the image block class is being output twice for duotone: .wp-block-image .wp-block-image img, .wp-block-image .wp-block-image .components-placeholder{filter: var(--wp--preset--duotone--custom-duotone);}.

Is that also dependent on block.json or the duotone backport changes @aaronrobertshaw ?

@aaronrobertshaw commented on PR #4655:


18 months ago
#8

I'm getting 1 failure

My expectation was that this test would be failing due to the image block.json needing to be updated. I'll go digging and see what I can find and see what else might need backporting.

@aaronrobertshaw commented on PR #4655:


18 months ago
#9

I'm getting one failure

There's a duotone-related change that we'll need to include in this backport as well to get the tests passing.

Unfortunately, I don't have permission to push to your fork or this PR branch. Instead, below is a small diff that you can apply.

  • src/wp-includes/class-wp-theme-json.php

    diff --git a/src/wp-includes/class-wp-theme-json.php b/src/wp-includes/class-wp-theme-json.php
    index 97f6d12511..92abe30f1c 100644
    a b class WP_Theme_JSON { 
    23892389
    23902390                // 3. Generate and append the rules that use the duotone selector.
    23912391                if ( isset( $block_metadata['duotone'] ) && ! empty( $declarations_duotone ) ) {
    2392                         $selector_duotone = static::scope_selector( $block_metadata['selector'], $block_metadata['duotone'] );
    2393                         $block_rules     .= static::to_ruleset( $selector_duotone, $declarations_duotone );
     2392                        $block_rules .= static::to_ruleset( $block_metadata['duotone'], $declarations_duotone );
    23942393                }
    23952394
    23962395                // 4. Generate Layout block gap styles.

@ramonopoly commented on PR #4655:


18 months ago
#10

Thanks for doing the heavy thinking here @aaronrobertshaw I'll get these in tomorrow 👍

@ramonopoly commented on PR #4655:


18 months ago
#11

@MaggieCabrera @aaronrobertshaw

Just checking, as there might be some overlap between this PR and

Suggesting that one (maybe this PR) gets merged first and then we rebase https://github.com/WordPress/wordpress-develop/pull/4619?

@onemaggie commented on PR #4655:


18 months ago
#12

@MaggieCabrera @aaronrobertshaw

Just checking, as there might be some overlap between this PR and

Suggesting that one (maybe this PR) gets merged first and then we rebase #4619?

I think that's the right order, yes

@aaronrobertshaw commented on PR #4655:


18 months ago
#13

Suggesting that one (maybe this PR) gets merged first and then we rebase https://github.com/WordPress/wordpress-develop/pull/4619?

Merging this PR first works for me, thanks 🙂

I'm happy to help out if I can on the duotone updates as well.

@ramonopoly commented on PR #4655:


18 months ago
#14

I'm happy to help out if I can on the duotone updates as well.

That'd be great, thank you!

I think this one seems to have fallen through the cracks? https://github.com/WordPress/gutenberg/pull/50678 Not sure.

@aaronrobertshaw commented on PR #4655:


18 months ago
#15

I think this one seems to have fallen through the cracks? https://github.com/WordPress/gutenberg/pull/50678 Not sure.

Thanks for flagging that @ramonjd. It does appear to have been missed. It is related to duotone filters rather than the block selectors API so I think belongs in https://github.com/WordPress/wordpress-develop/pull/4619.

cc/ @MaggieCabrera

#16 @audrasjb
18 months ago

  • Type changed from enhancement to task (blessed)

Converting to a task as this can ship after beta 1 is released.

#17 @isabel_brison
18 months ago

  • Owner set to isabel_brison
  • Resolution set to fixed
  • Status changed from new to closed

In 56058:

Editor: refactor and stabilize selectors API.

Restructures the block.json selectors API by moving __experimentalSelector props into their own config, stabilizing the selectors API, and enabling more flexible styling options.

Props ramonopoly, spacedmonkey, aaronrobertshaw, onemaggie.
Fixes #58586.

@isabel_brison commented on PR #4655:


18 months ago
#18

committed in r56058 / 8b9d8d8.

#19 @mukesh27
18 months ago

@isabel_brison the multiline comment indention is wrong in src/wp-includes/block-supports/settings.php line no 99. https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/block-supports/settings.php#L99-L102

This ticket was mentioned in PR #4803 on WordPress/wordpress-develop by @isabel_brison.


17 months ago
#20

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

Fix comment indentation issue noted here

#21 @isabel_brison
17 months ago

In 56146:

Editor: fix comment indentation.

Fixes multiline comment indentation in wp-includes/block-supports/settings.php. Also fixes typo in the comment.

Props mukesh27.
See #58586.

@isabel_brison commented on PR #4803:


17 months ago
#22

Committed in r56146 / 21c74f1.

Note: See TracTickets for help on using tickets.