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

Consolidate Cover block Dimensions and Spacing panels within the Setting Sidebar #23770

Closed
richtabor opened this issue Jul 7, 2020 · 9 comments
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Decision Needs a decision to be actionable or relevant Needs Design Feedback Needs general design feedback.

Comments

@richtabor
Copy link
Member

Is your feature request related to a problem? Please describe.
The Cover block currently has one control per panel within its Settings Sidebar. That's pretty heavy UX wise, resulting in a compounding amount of heaviness to the UI.

Describe the solution you'd like
I suggest consolidating the "Minimum height of cover" and "Padding" controls into one panel.

Screen_Shot_2020-07-07_at_12 56 20_PM

@richtabor richtabor added Needs Design Feedback Needs general design feedback. [Block] Cover Affects the Cover Block - used to display content laid over a background image labels Jul 7, 2020
@gziolo gziolo added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Jul 9, 2020
@paaljoachim paaljoachim added Needs Design Needs design efforts. Needs Decision Needs a decision to be actionable or relevant labels Sep 22, 2020
@richtabor richtabor added Needs Design Feedback Needs general design feedback. and removed Needs Design Feedback Needs general design feedback. labels Sep 22, 2020
@richtabor
Copy link
Member Author

The decision we need is:

Should we move the “Padding” control to the “Dimensions” PanelBody?

@paaljoachim
Copy link
Contributor

Ahhh I believe this is an issue for Jon @ItsJonQ to take a closer look at.

@ItsJonQ
Copy link

ItsJonQ commented Oct 28, 2020

Haii! Thanks for highlighting this. Indeed, Cover is one of the "chunkier" core blocks, as far as sidebar control clusters are concerned.

I've been thinking about about how these controls should be grouped/organized. The addition and expansion of Global Styles will only add more controls.

(These are my thoughts. Hopefully the thoughts below make sense)

🎮 Types of Controls

It's become more apparent that there are 2 "types" of controls for blocks.

"Styling" and "Features" (for a lack of a better term).

Styling Controls adjust general styles for a block, mapping pretty well to the underlying CSS property. (e.g. padding, color, width, etc...)

Feature Controls adjust properties that are unique for a given block. For example, "Gallery" block has a Crop Image setting.

At the moment, these controls are blended together.

Admittedly, it's quite difficult to distinguish between the two. The general rule I fallback to is...

If it's (basically) a control for a CSS property, and can be generalized and applied to other blocks, than it's a Styling control.

🎳 Splitting/Grouping

With the Global Styles system underway (along with the new theme.json API), we're able to activate generalized Style controls for blocks. For example, the Padding controls code doesn't exist within the Cover block's edit.js.

So! For min-height. Technically yes, min-height is a CSS property. And min-height feels like it can be grouped with Padding in Dimensions. However, Cover's usage of min-height is quite unique/specific to Cover. It don't believe it exists on any other block.

Ultimately, I think min-height should be a globalized Style control (just like Padding), which Cover could activate using it's block.json.

The organization of that control would be predetermined and consistent with any other block activating the same controls. I would imagine clusters organized like this:

Size
----
* Width
* Height
* Min Width
* Min Height

Spacing
---
* Padding
* Margin

(I'm not saying we explicitly needs these control types. Just using this as a potential example)

If we had the above setup, then we can remove "Dimensions" from Cover, allowing it to use the globalized min-height control instead.

We'd still have the 2 Panel "chunky" UI experience. However, the controls would be organized consistently throughout the entire system, which I think is a better overall experience gain.

✨ Bonus

Cover also has "Overlay", which is also a "kinda unique but also kinda general" control. I would propose that we should instead of a "Backgrounds" (globalized style) Panel. So then Cover's controls would look something like this:

Backgrounds
---
* Color

Size
----
* Width
* Height
* Min Width
* Min Height

Spacing
---
* Padding
* Margin

@karmatosed karmatosed removed the Needs Design Needs design efforts. label Nov 9, 2020
@mtias mtias added the [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi label Nov 27, 2020
@paaljoachim
Copy link
Contributor

paaljoachim commented Jul 12, 2021

@aaronrobertshaw is currently working on a related feature.

@aaronrobertshaw
Copy link
Contributor

There is some related work currently under way that relates to this although the scope is a little narrower.

The work being done can be found at:

The proposed height block support was intended as a proof of concept from which we could expand upon adding features for min/max height etc. Until that is implemented or the block support panels are reworked to a slot and fill approach, the unification of the two panels in the original issue description might be a little ways off.

@ramonjd
Copy link
Member

ramonjd commented Aug 10, 2021

With the work being done in the PRs above, I think something like the following will soon be possible:

Screen Shot 2021-08-10 at 12 56 12 pm

Since cover block already has a "Dimensions" control panel, it sounds like a good idea to consolidate these as soon as we have the Dimensions panel merged.

The only question we'll need to address I think is whether we need to introduce a new block support for min-height (and width while we're at it?) similar to #32499

Probably yes, given how the cover blocks currently works.

If so, once the Tools Panel lands in #32392 we'll be able to show the minimum height by default using __experimentalDefaultControls

Screen Shot 2021-08-10 at 1 37 21 pm

@aaronrobertshaw
Copy link
Contributor

The only question we'll need to address I think is whether we need to introduce a new block support for min-height (and width while we're at it?) similar to #32499

@ramonjd there is already a PR under way for adding width block support. It is still a draft given there was a lot of iterating over the width control. It turned out that a simple unit control matching the height support wasn't suitable for width.

The idea with the height block support is that once merged, it will be a nice small PR to extend it to min or max height.

It's possible that there may need to be some discussion around the controls given they can expand in number quite rapidly. I believe that concern is reduced though given it will be neatly tucked away within the new ToolsPanel and only exposed as needed.

Below is a glimpse from the width support PR of the padding, margin, height and width block supports all within the new dimensions panel. Now the width control is no longer a simple unit control it won't fit neatly in a 50% column alongside the height control. We'll likely need some iterations on the design here but it's a start.

DimensionsPanelDemo2

@Mamaduka
Copy link
Member

@aaronrobertshaw, I think #34065 fixed this issue.

@aaronrobertshaw
Copy link
Contributor

Thanks for the ping @Mamaduka. 👋

Yes, #34065 leverages a new SlotFill approach to inject the min-height control into the dimensions block support panel as suggested by this issue.

I'll close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Decision Needs a decision to be actionable or relevant Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

No branches or pull requests

9 participants