Embed Block: Border support added #66386

benazeer-ben · 2024-10-23T19:44:56Z

What?

Adding border support for Embed block.
Fixes #43247

Why?

Embed block is missing border support

How?

Adds the border support via block.json

Testing Instructions

Go to the Global Styles setting.
Make sure that the Embed block's border is configurable via Global Styles.
Verify that Global Styles are applied correctly in the editor and frontend.
Navigate to post/page edit.
Then add Embed block and apply the border styles.
Verify that block border styles take precedence over global block colour settings.
Verify that block borders display correctly in both the editor and frontend.
Border will get applied only to the embeds, not to captions.

NOTE:

WP embeds are handling in bit different way, so make sure all embeds are working well. You can try with all possible embeds available like YouTube, X(Twitter), Spotify, SoundCloud and WordPress posts.
This PR does not handle border-radius since it requires padding support as well. Adding padding support would involve reconsidering several scenarios, which could complicate the implementation. To keep this PR straightforward, border-radius and padding are not addressed at this time.

Sample URLs for Testing:

WordPress Post:
https://wordpress.org/documentation/article/embeds/

YouTube Video:
https://www.youtube.com/watch?v=sYNqeahE2rg

Tweet:
https://x.com/WordPress

Spotify Playlist:
https://open.spotify.com/playlist/37i9dQZF1DXcBWIGoYBM5M

SoundCloud Track:
https://soundcloud.com/forss/flickermood

Screenshots or screencast

Screenshare.-.2024-11-26.3_47_53.PM.mp4

github-actions · 2024-10-23T19:45:09Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @benazeerhassan1909.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: benazeerhassan1909.

Co-authored-by: benazeer-ben <[email protected]>
Co-authored-by: akasunil <[email protected]>
Co-authored-by: aaronrobertshaw <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

github-actions · 2024-10-23T19:45:31Z

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @benazeer-ben! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

…tenberg into add/embed-border

This reverts commit 5437114.

aaronrobertshaw

Thank you for putting this together @benazeer-ben 🙇

I get the feeling this one is going to have to wrangle several edge cases given the various types of content the block can embed e.g. youtube videos, tweets, spotify and audio links etc.

Testing this PR as is, I've run into a couple of issues:

To support border radius we should probably also support padding on the block. This might need design feedback as discussed on Video: Add border support.
The current approach in this PR applies the border to the block's wrapper. The embed block can also include a caption. Other blocks such as the image block, apply the border to the actual element that needs the border leaving the caption outside it. I think this would be the expected behaviour here as well.
When adding border or padding support we need to ensure the block has box-sizing: border-box styles so that it will respect any width its parent might be trying to apply to it.

Box sizing issue:

Embed blocks with different content and captions:

Next Steps

To move this PR forward while waiting on some design input regarding border-radius adoption, I think we can:

Skip serialization of borders on the block wrapper and apply it to the embed's inner wrapper so the caption falls outside it. See Video: Add border block support #63777 for some clues on this approach.
Add box-sizing: border-box to the block. (there's also a separate PR aimed at generically adding this style to blocks but it might be slow to land given the potential to cause visual regressions).

Thanks again for all your work on adopting block supports, it's appreciated 👍

benazeer-ben · 2024-11-15T15:43:53Z

Thanks @aaronrobertshaw for the detailed feedback and details.

Implemented skip serialization of borders on the block wrapper and applied it to the embed's inner wrapper.

For now padding support also added, if we get some design feedback we can change it accordingly.

Please let me know if we need more tweak on this.

Screenshare.-.2024-11-15.9_02_39.PM.mp4

aaronrobertshaw · 2024-11-18T08:31:38Z

packages/block-library/src/embed/block.json

 		}
 	},
+	"selectors": {
+		"border": ".wp-block-embed__wrapper "


The convention with these selectors is to explicitly include the block's class. In this case, .wp-block-embed.

I'm not 100% whether it is required but it does make it clearer the intended target for the styles. That block class is also tweaked when generating block style variation selectors for the block.

One last nit, there's an unnecessary space at the end of that selector. Better safe than sorry, we should clean that up in case theres some theme.json processing somewhere that wasn't expecting it.

aaronrobertshaw · 2024-11-18T08:39:47Z

packages/block-library/src/embed/block.json

@@ -43,12 +43,29 @@
 	"supports": {
 		"align": true,
 		"spacing": {
-			"margin": true
+			"margin": true,
+			"padding": true


This applies the padding to the Embed block's wrapper.

The justification for adding the padding support was so that it could be used to counter border radius and prevent the content from sticking out beyond the border.

This might need to be applied to the inner embed wrapper as well.

aaronrobertshaw

Thank you for iterating on this 👍

I've given it a test and there are still a few issues. Most notably:

Padding is applied to the wrapper so doesn't help with the border radius problem
The custom selector for borders should probably also have the block class in it to ensure compatibility with block style variations.
PR description and test instructions no longer match what the PR does

On that last point, I find it handy each time I iterate on a PR to re-test it in full asking myself some questions like:

Have I changed what the PR does?
If it does something new, how do I test that?
If the PR is related to a block, are there different ways to use the block? e.g. testing the embed block with not just a video but all the other embed types.

Answering those questions then means you already have everything you need to update the PR description in a way that ensures things don't get missed and it's easier for reviewers.

Here's what I see when applying both padding and borders to various embeds. I'd expect the padding within the border, not outside it otherwise it's basically just a second margin.

benazeer-ben · 2024-11-20T06:20:31Z

Hi @aaronrobertshaw

Thank you for your detailed feedback and for testing the PR! 🙏

To keep things straightforward and manageable, how about we focus only on the border support for this PR and hold off on padding and border radius for now? This approach should help us avoid any complications and keep the scope of this PR more focused.

Apologies for the confusion caused by the current PR description and test instructions—I’ll make a note to clarify these aspects in future PRs. Your suggestions on updating the PR description through iterative testing are very helpful, and I'll incorporate this approach going forward to ensure clarity and completeness.

Thank you again for your patience and insights!

aaronrobertshaw · 2024-11-21T04:55:55Z

To keep things straightforward and manageable, how about we focus only on the border support for this PR and hold off on padding and border radius for now?

As the initial intent was to adopt border support including border-radius (which I think is expected by users), padding is a natural inclusion here. I think there is some benefit to introducing both at the same time where testing can cover the whole picture and how the supports interact together.

This approach should help us avoid any complications and keep the scope of this PR more focused.

In some ways I agree, however, there are some curly problems around deprecations and block supports, especially if the application of block supports might require skipping serialization. To me, that risk means it's better to work through the whole process here holistically.

Additionally, you've already gone to the efforts of skipping serialization of border styles to apply them to the inner embed wrapper. It's not much more to do the same for padding support. The __experimentalSkipSerialization supports flag also supports arrays so that individual features can be skipped while the rest are still applied to the wrapper (in this case, margin would still go on the outer wrapper).

Some examples of individual features skipping serialization are:

Cover gradients
Navigation text-decoration
Gallery block gap
Calendar text & background colors

Lastly, adding borders to the Embed block is a bit of a niche use case. This means there isn't a pressing timeline to deliver the feature. We have the luxury to take our time, dot our i's and cross out t's.

Are you onboard with that?

benazeer-ben · 2024-11-21T06:26:50Z

@aaronrobertshaw Thank you for your thoughtful response and for outlining the considerations so clearly.

I agree that addressing border and padding together provides a more complete solution, especially given how they interact. The examples of __experimentalSkipSerialization for individual features are very helpful and will guide the implementation.

I'll proceed with implementing the feature, ensuring deprecations and block supports are handled properly while keeping the scope clear and test instructions comprehensive.

Thanks again for guidance— I’ll keep updated as I progress!

benazeer-ben · 2024-11-25T08:46:47Z

I’ve started implementing the padding skip serialisation, but I’ve encountered an issue when working with this. Since the embed block includes position absolute style settings for the iframe, skipping padding serialisation and applying manual settings to the inner wrapper does not have any effect.

I’d appreciate any suggestions on how to handle these edge cases effectively. Should we consider an alternative approach for blocks with absolute positioning?

Thanks in advance for your insights!

aaronrobertshaw

Thanks for highlighting that issue and the detailed info @benazeer-ben 👍

This is a tricky one!

There could be a few options to explore to allow supporting padding e.g. an extra wrapper, using flex/grid positioning, transforms for the iframe. All of those options have their own pros and cons whether that is backward compatibility, needing deprecations, or impacting the aspect ratio support. This would be better suited in a follow-up PR.

Given the complexity uncovered here I think your earlier suggestion to avoid padding support and focus on border styles is a good one. We'll need to omit support for border-radius also.

Border styles will still need to be applied to the inner wrapper and I don't think that will change. In a follow-up that pursues border radius support, I'd expect that to also be applied to the same element, so we shouldn't be painting ourselves into a corner here.

Let's go with the plan to omit padding and border-radius support so we can at least offer some border support in the short term.

How's that sound to you?

benazeer-ben · 2024-11-26T06:02:21Z

Thank you for the detailed breakdown!

I agree with your assessment—introducing padding support, especially given the iframe's positioning complexities, could indeed lead to challenges with backward compatibility and aspect ratio handling.

Focusing on border styles while omitting padding and border-radius support for now sounds like a practical approach to deliver incremental improvements without overcomplicating this PR.

I’m on board with this plan and will proceed accordingly, ensuring border styles are applied to the inner wrapper as discussed. For border-radius and padding, I agree these can be revisited in a follow-up PR with a more comprehensive approach.

aaronrobertshaw

We're slowly narrowing in on something we can land. Thanks for your patience @benazeer-ben 🙇

I've left a few comments inline on some issues I spotted after the latest updates.

aaronrobertshaw · 2024-11-26T23:47:00Z

packages/block-library/src/embed/block.json

@@ -47,8 +47,24 @@
 		},
 		"interactivity": {
 			"clientNavigation": true
+		},
+		"__experimentalBorder": {
+			"radius": false,


The border support is opt-in by default so we shouldn't have to explicitly set it to false. That said, this does communicate that it was a conscious decision to omit border-radius support so we can leave it..

packages/block-library/src/embed/block.json

aaronrobertshaw · 2024-11-27T00:45:32Z

packages/block-library/src/embed/embed-preview.js

+				className={ borderProps.className }
+				style={ { ...borderProps.style } }


I'm not sure about these new props. Would it be more forward-compatible for them to be combined into something like wrapperProps? It might also help communicate the intended use for the class name and style.

It might also help get other reviewers if we can update the test instructions to cover this scenario as well as others (e.g. explaining to test with a WordPress post as an embed, youtube video, tweet, spotify link etc.) Even better if you could include some test URLs 🙏

aaronrobertshaw · 2024-11-27T00:48:46Z

packages/block-library/src/embed/wp-embed-preview.js

@@ -13,7 +13,7 @@ const attributeMap = {
 	marginwidth: 'marginWidth',
 };

-export default function WpEmbedPreview( { html } ) {
+export default function WpEmbedPreview( { html, className, style } ) {


As noted in my earlier comment, these new props might be better combined into a single wrapperProps prop or something.

Co-authored-by: Aaron Robertshaw <[email protected]>

Border support added for Embed block

1398e28

benazeer-ben requested a review from ajitbohra as a code owner October 23, 2024 19:44

github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Oct 23, 2024

aaronrobertshaw mentioned this pull request Oct 24, 2024

Tracking: Border Design Tools Consistency #43247

Open

benazeerhassan1909 added 2 commits October 24, 2024 12:21

Border support added for Embed block

99b4f17

Merge branch 'add/embed-border' of https://github.com/benazeer-ben/gu…

519f7b7

…tenberg into add/embed-border

benazeer-ben requested review from spacedmonkey and ellatrix as code owners October 24, 2024 06:52

akasunil added [Type] Enhancement A suggestion for improvement. [Block] Embed Affects the Embed Block labels Oct 24, 2024

benazeer-ben and others added 4 commits October 25, 2024 16:14

Revert "Style Book: Add color tab (WordPress#65692)"

4184b4b

This reverts commit 5437114.

Merge branch 'trunk' into add/embed-border

03a5108

Synced with trunk and resolved conflicts

920cf53

Revert unneccessory changes

a2e68b4

aaronrobertshaw reviewed Nov 11, 2024

View reviewed changes

Feedback changes - Skip serialization & border box

e267978

benazeer-ben requested review from juanmaguitar, fabiankaegy and ndiego as code owners November 15, 2024 15:39

aaronrobertshaw reviewed Nov 18, 2024

View reviewed changes

Skip serialization for padding

38ca863

aaronrobertshaw reviewed Nov 26, 2024

View reviewed changes

benazeer-ben added 2 commits November 26, 2024 11:40

Doc Build

cc18957

Update for wordpress embed case border styling

b0e2645

aaronrobertshaw reviewed Nov 27, 2024

View reviewed changes

benazeer-ben and others added 2 commits November 27, 2024 11:14

Update packages/block-library/src/embed/block.json

3be64f4

Co-authored-by: Aaron Robertshaw <[email protected]>

Feedback changes - added wrapperProps

cc10820

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Embed Block: Border support added #66386

Embed Block: Border support added #66386

benazeer-ben commented Oct 23, 2024 •

edited

Loading

github-actions bot commented Oct 23, 2024 •

edited

Loading

github-actions bot commented Oct 23, 2024

aaronrobertshaw left a comment

benazeer-ben commented Nov 15, 2024

aaronrobertshaw Nov 18, 2024

aaronrobertshaw Nov 18, 2024

aaronrobertshaw left a comment

benazeer-ben commented Nov 20, 2024

aaronrobertshaw commented Nov 21, 2024

benazeer-ben commented Nov 21, 2024

benazeer-ben commented Nov 25, 2024

aaronrobertshaw left a comment

benazeer-ben commented Nov 26, 2024 •

edited

Loading

aaronrobertshaw left a comment

aaronrobertshaw Nov 26, 2024

aaronrobertshaw Nov 27, 2024

aaronrobertshaw Nov 27, 2024

		className={ borderProps.className }
		style={ { ...borderProps.style } }

Embed Block: Border support added #66386

Are you sure you want to change the base?

Embed Block: Border support added #66386

Conversation

benazeer-ben commented Oct 23, 2024 • edited Loading

What?

Why?

How?

Testing Instructions

Sample URLs for Testing:

Screenshots or screencast

github-actions bot commented Oct 23, 2024 • edited Loading

Unlinked Accounts

github-actions bot commented Oct 23, 2024

aaronrobertshaw left a comment

Choose a reason for hiding this comment

Box sizing issue:

Embed blocks with different content and captions:

Next Steps

benazeer-ben commented Nov 15, 2024

aaronrobertshaw Nov 18, 2024

Choose a reason for hiding this comment

aaronrobertshaw Nov 18, 2024

Choose a reason for hiding this comment

aaronrobertshaw left a comment

Choose a reason for hiding this comment

benazeer-ben commented Nov 20, 2024

aaronrobertshaw commented Nov 21, 2024

benazeer-ben commented Nov 21, 2024

benazeer-ben commented Nov 25, 2024

aaronrobertshaw left a comment

Choose a reason for hiding this comment

benazeer-ben commented Nov 26, 2024 • edited Loading

aaronrobertshaw left a comment

Choose a reason for hiding this comment

aaronrobertshaw Nov 26, 2024

Choose a reason for hiding this comment

aaronrobertshaw Nov 27, 2024

Choose a reason for hiding this comment

aaronrobertshaw Nov 27, 2024

Choose a reason for hiding this comment

benazeer-ben commented Oct 23, 2024 •

edited

Loading

github-actions bot commented Oct 23, 2024 •

edited

Loading

benazeer-ben commented Nov 26, 2024 •

edited

Loading