-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Allow setting the height from the placeholder state. #34706
Comments
I've had a play with this. Please ignore the layout. In my mind, we could group the background color/media options in a single row + two columns, with the height control below. Just noting down some considerations for when I, or someone else, rounds back to properly implement:
I didn't pay any attention to what's going on in the edit.native.js file of the Cover Block so some work might need to be done in its placeholder too. |
@jasmussen Any thoughts on a good approach for this, or what Ramon has posted above? A large part of me wants to remove the placeholder entirely with the more intuitive color and image controls we have now via the toolbar. |
My initial thoughts, now I've had time to think about the experiment, was that the placeholder was throwing up too much of a speed hump to editing. Without seeing the block in the editor, I don't really know which min height is appropriate for my image or whichever background and content I have. At the very least, I'd skip adding more controls to the placeholder and, as Andy says, continue with improving the block inspector controls. |
Thanks for exploring. I share the instinct of wanting to use the The cover block has a lot of age attached to it, though, and if feedback suggests the need for a min-height as part of the setup state, it seems worth exploring that. Doing so also wouldn't preclude future explorations to reduce the usage of the Would it be possible to add the dimensions panel itself to the inspector sidebar even in the setup state? Along the same lines, it feels like the best interface for setting the min-height is the in-canvas resizable box, like so: This would actually resize the bordered-box |
It might just possible! Regardless, I think it's a great compromise and worth a shot. Thanks for the suggestion and the design concept image. 🙇 |
Very cool! One gotcha on this design, though, is that it doesn't work if you want to make the cover smaller 🙈 That speaks to the limitation of the placeholder component itself, though, and is why it's good to use it sparingly. It's likely fine, though, as the height is meant as a min-height and will always be affected by the content inside, and once you pick a color or add an image, you can adjust the size again with the same interface. |
No description provided.
The text was updated successfully, but these errors were encountered: