The Wayback Machine - https://web.archive.org/web/20220219160752/https://github.com/WordPress/gutenberg/issues/38875
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

Add warning for making a Cover block height smaller than embed #38875

Open
annezazu opened this issue Feb 17, 2022 · 3 comments
Open

Add warning for making a Cover block height smaller than embed #38875

annezazu opened this issue Feb 17, 2022 · 3 comments

Comments

@annezazu
Copy link
Contributor

@annezazu annezazu commented Feb 17, 2022

What problem does this address?

If you add a Cover block and include an embed within it, you cannot make the minimum height smaller than the embed. This makes sense but there's no warning in place to tell you that. This was found as part of the FSE Outreach Program's All Things Media exploration:

Insert a YouTube video or Imgur image. Click on the outer cover block. Adjusting the height on the cover doesn’t work.

What is your proposed solution?

The text is terrible :) but just to show an idea -- we could have a similar warning you might see for contrast:

Screen Shot 2022-02-16 at 8 42 13 PM

@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Feb 17, 2022

Perhaps there's a deeper question on the minimum height control. Technically it's working perfectly as intended: it's the minimum height, and whatever content is inside is free to expand the container beyond that. But of course that isn't necessarily intuitive for most people who might think of it as a plain height control, perhaps especially since the resize handle in the canvas suggests that to be the case.

I seem to vaguely recall @kjellr mentioning some issues related to height vs. min-height in context of Cover. I wonder if we should explore converting this to just height 🤔

@ntsekouras
Copy link
Contributor

@ntsekouras ntsekouras commented Feb 17, 2022

Technically it's working perfectly as intended: it's the minimum height, and whatever content is inside is free to expand the container beyond that

I agree with that and I don't think we can swap this with height. It will create a tons of issues and confusion since everything is responsive and will create scrollbars, might cause issues with the fullscreen toggle, etc...

Maybe what we need here is a help text explaining a bit what min-height is?

@annezazu
Copy link
Contributor Author

@annezazu annezazu commented Feb 17, 2022

Ohh a brief description might help. I still am strangely partial to having a warning of sorts to save the sidebar from getting even longer but that might be difficult to intelligently implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment