Skip to content

Embed Block: Set width for embed in group block #65841

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

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

brbrr
Copy link

@brbrr brbrr commented Oct 2, 2024

What?

This PR sets a width: 100% CSS rule for embed blocks within group block.

Why?

Fixes: #65741
Video embeds (or even all embeds responsive: true attribute) are collapsing in both the editor and on the frontend when embedded (😉 ) into Row/Stack blocks.

While I haven't been able to identify the root cause precisely, it seems that because the embed block, when in responsive mode, rely on a container to define its height/width and the group block does not provide size related styles, embed collapses.

How?

By setting width: 100% for .wp-block-group .wp-block-embed.

Testing Instructions

  1. In a block editor, add a Stack block
  2. add a youtube or vimeo embed block into the Stack block
  3. Check if the video preview is rendered correctly
  4. Publish, check the content
  5. Repeat 1-4 for Post, Page, and Side editor

Testing Instructions for Keyboard

N/A

Screenshots or screencast

Before:
Screenshot 2024-10-02 at 19 53 27
Screenshot 2024-10-02 at 19 53 01

After:
Screenshot 2024-10-02 at 20 21 53
Screenshot 2024-10-02 at 20 22 26

@brbrr brbrr requested a review from ajitbohra as a code owner October 2, 2024 18:24
Copy link

github-actions bot commented Oct 2, 2024

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: @supernovia.

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: supernovia.

Co-authored-by: brbrr <[email protected]>
Co-authored-by: youknowriad <[email protected]>

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

Copy link

github-actions bot commented Oct 2, 2024

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @brbrr! 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.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Oct 2, 2024
@brbrr brbrr changed the title Set width for embed in group block Embed Block: Set width for embed in group block Oct 2, 2024
@skorasaurus skorasaurus added the [Block] Embed Affects the Embed Block label Oct 4, 2024
@youknowriad youknowriad requested a review from jasmussen October 21, 2024 14:01
@youknowriad
Copy link
Contributor

Thanks for creating this PR. Difficult problem indeed.

The root issue is that "flex" alignment (stack or row) doesn't define "widths" for its children "divs". They just use whatever width they like.

So when you insert a "responsive" embed (like YouTube embed), the width is supposed to be computed dynamically from the parent div, but since the parent div is width "0" by default (it adapts to its content in flex containers), the video collapses.

That said, I'm not really sure what's the right approach here. Setting 100% width doesn't seem like the right solution to me because when using a "row" you might want multiple items in a row. and even in a stack, you might to "center" the items or something instead of expanding the width.

If feels like the "Resize for smaller devices" toggle, shouldn't have an impact in this case maybe? I'm not sure though :)

cc @jasmussen any thoughts.

@cbravobernal cbravobernal added the [Type] Bug An existing feature does not function as intended label Oct 21, 2024
@jasmussen
Copy link
Contributor

Yes, this comes up a lot. A past solution we explored was to define a min-width for embeds. I think that was even merged at one point, has this since been removed? One reason to potentially remove it is that some embeds are truly tiny. So a min-width also has to be tiny, and since we can't provide, and even if we could we couldn't maintain, specific widths for arbitrary embeds, it's really hard to find a universal min-width that works.

A min-width still does seem the best solution to me, at least for the responsive embeds. It just has to be really small, just enough to not collapse, let you select it, and then using the block inspector, set an explicit width.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Embed Affects the Embed Block First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stack/Row block prevents YouTube Embeds from showing when "Resize for smaller devices" is active
5 participants