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.

Sign up for GitHub

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

Jump to bottom

Backporting block supports [border, color, elements, spacing] from Gutenberg to WP 6.1 #3204

Closed
wants to merge 10 commits into from
Closed

Backporting block supports [border, color, elements, spacing] from Gutenberg to WP 6.1 #3204

wants to merge 10 commits into from

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Sep 7, 2022 •
edited by ockham
Loading

❗ BLOCKED by #3199 Edit: #3199 has been merged.

Backporting border, color, elements, spacing block supports changes from Gutenberg to WP 6.1

Typography is taken care of over in #3203

See tracking issue: WordPress/gutenberg#43440

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

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.

This was referenced Sep 7, 2022
Closed
Closed
@ramonjd ramonjd force-pushed the update/backport-border-color-elements-spacing-block-supports-6-1 branch from 1614ef8 to c20ddd2 Compare September 9, 2022 11:32
@glendaviesnz glendaviesnz mentioned this pull request Sep 15, 2022
Closed
@ockham ockham force-pushed the update/backport-border-color-elements-spacing-block-supports-6-1 branch from c20ddd2 to 569db97 Compare September 15, 2022 13:25
@ockham
Copy link
Contributor

ockham commented Sep 15, 2022

Rebased, now that #3199 has landed.

@ockham
Copy link
Contributor

ockham commented Sep 15, 2022

Unit tests are failing -- apparently because of extra whitespace in the "expected" string. Errors look like this:

1) Test_Block_Supports_Border::test_border_color_slug_with_numbers_is_kebab_cased_properly
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 (
     'class' => 'has-border-color has-red-border-color'
-    'style' => 'border-radius: 10px; border-style: dashed; border-width: 1px;'
+    'style' => 'border-radius:10px;border-style:dashed;border-width:1px;'
 )

/var/www/tests/phpunit/tests/block-supports/border.php:70
phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:97
/var/www/vendor/bin/phpunit:118

@ockham
Copy link
Contributor

ockham commented Sep 15, 2022

A similar issue seems to be present over at #3237 (comment)

@ockham ockham mentioned this pull request Sep 15, 2022
Closed
@ramonjd
Copy link
Member Author

ramonjd commented Sep 18, 2022

I'll rebase and fix. Sorry, was out of action last week.

@ramonjd ramonjd force-pushed the update/backport-border-color-elements-spacing-block-supports-6-1 branch from 569db97 to ec6ea9d Compare September 19, 2022 00:43
@ramonjd
Updating CSS test output strings in the tests to non-prettified versi…
9a01f76 
…ons. The Style Engine does not prettify by default.
@ramonjd ramonjd force-pushed the update/backport-border-color-elements-spacing-block-supports-6-1 branch from 7b8a48c to 9a01f76 Compare September 19, 2022 01:17
@ramonjd
Copy link
Member Author

ramonjd commented Sep 19, 2022

Unit tests are failing -- apparently because of extra whitespace in the "expected" string. Errors look like this:

@ockham Good to go I think.

ockham
ockham reviewed Sep 19, 2022
View reviewed changes
src/wp-includes/block-supports/elements.php
Comment on lines +115 to +121
wp_style_engine_get_styles(
$link_block_styles,
array(
'selector' => ".$class_name a",
'context' => 'block-supports',
)
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we have to do something with the return value? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried a paragraph block with a link and the link color set to orange, and it really isn't reflected on the frontend:

image

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, looks like this needs #3273 in order for the relevant styling to be added!

Copy link
Member Author

Choose a reason for hiding this comment

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

Don't we have to do something with the return value? 🤔

Not here since the style engine will register styles in a store using the context as key. Gutenberg fetches stored styles at render time and prints them out.

ockham
ockham approved these changes Sep 19, 2022
View reviewed changes
Copy link
Contributor

@ockham ockham left a comment

Choose a reason for hiding this comment

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

LGTM. We might want to land #3273 first and rebase this PR (see #3204 (comment)).

@audrasjb audrasjb self-assigned this Sep 19, 2022
costdev
costdev requested changes Sep 19, 2022
View reviewed changes
Copy link
Contributor

@costdev costdev left a comment

Choose a reason for hiding this comment

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

Thanks for handling this PR @ramonjd Just one minor suggestion 🙂

src/wp-includes/block-supports/border.php Outdated Show resolved Hide resolved
audrasjb and others added 7 commits September 19, 2022 20:58
@audrasjb @costdev
Update src/wp-includes/block-supports/border.php
5646544 
better consistency for `since` mentions.

Co-authored-by: Colin Stewart <79332690+costdev@users.noreply.github.com>
@audrasjb
Update border.php
d13204d 
better consistency for `since` mentions.
@audrasjb
Update colors.php
9f2682c 
better consistency for `since` mentions.
@audrasjb
Update colors.php
e7cafa8 
better consistency for `since` mentions.
@audrasjb
Update colors.php
20e5b59 
Better consistency for `since` mentions
@audrasjb
Update elements.php
fb597d3 
Better consistency for `since` mentions
@audrasjb
Update spacing.php
bf978d4 
Better consistency for `since` mentions
@audrasjb
Copy link
Contributor

Committed in https://core.trac.wordpress.org/changeset/54211

@audrasjb audrasjb closed this Sep 19, 2022
@ramonjd
Copy link
Member Author

ramonjd commented Sep 19, 2022

Thanks for all the help, folks!

@ramonjd ramonjd deleted the update/backport-border-color-elements-spacing-block-supports-6-1 branch September 27, 2022 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@costdev costdev costdev requested changes

@ockham ockham ockham approved these changes

Assignees

@audrasjb audrasjb

Labels
None yet
Projects
No open projects
WordPress 6.1 Editor Tasks
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ramonjd @ockham @audrasjb @costdev