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

Consider adding bottom margin to some blocks #15668

Closed
kjellr opened this issue May 15, 2019 · 19 comments
Closed

Consider adding bottom margin to some blocks #15668

kjellr opened this issue May 15, 2019 · 19 comments
Labels
[Block] Media & Text Affects the Media & Text Block [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes.

Comments

@kjellr
Copy link
Contributor

kjellr commented May 15, 2019

Currently, if you're using a theme that does not provide front-end styles for the Media & Text block, you'll see this in the editor:

Screen Shot 2019-05-15 at 3 09 13 PM

... but something like this in the front end:

Screen Shot 2019-05-15 at 3 07 48 PM

It seems a bit jarring to completely remove the bottom margin on the front end here. Something similar happens with a number of other blocks: Cover, Group, etc.

This isn't just an issue We used to provide some bottom margin for the Cover block, but we don’t anymore in the latest release (@jasmussen can correct me if I'm wrong, but I believe this happened in #14407). I don’t believe the Media and Text block did ever ship with bottom margins.

In any case, I know we’re purposefully trying to avoid adding styles like that bottom margin so we don’t provide too much “bloat” to for theme authors to have to override… but this actually does a disservice to pre-Gutenberg themes, since they obviously wouldn’t include margin styles for these sorts of elements that didn’t exist yet. Hopefully there's some sort of middle ground.

So — I'd like to reopen the discussion around providing front-end bottom margins to blocks like these which may not come with them by default.

@kjellr kjellr added [Type] Bug An existing feature does not function as intended Needs Design Feedback Needs general design feedback. labels May 15, 2019
@swissspidy swissspidy changed the title Consider adding bottom margin to blocks some blocks Consider adding bottom margin to some blocks May 15, 2019
@swissspidy swissspidy added the [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes. label May 15, 2019
@jasmussen
Copy link
Contributor

Great ticket, thank you.

Looking over #14407 again, it doesn't look like it specifically removed any margins from the Media & Text block. In fact I haven't updated my own site yet, it's still 5.1 which I'm pretty sure doesn't ship with that change. And things look the same as you describe here. Editor:

Screenshot 2019-05-16 at 08 46 46

Frontend:

Screenshot 2019-05-16 at 08 46 52

However this ticket is still valid, and I happen to agree with you, Media & Text deserves to be born with an intrinsic bottom margin (i.e. in style.scss, not just theme.scss). The reason being: it's an entirely new block.

There's also precedence for doing it in such cases, here, an explicit bottom margin is applied to the columns block:
https://github.com/WordPress/gutenberg/pull/14407/files#diff-d0807e2d5cf91a52ec75864e689a9d14R3

So long as this margin is trivial to override and customize for themers, I would suggest it's good to still add. Next step could be a PR that does the same as is referenced just above for the columns block.

@idea--list
Copy link

Honestly, i do not think adding any automatic padding/margin would be a good idea.
GB did so in the past with columns block before and guess what... it has been removed as it caused unpredictable behaviour.

If you still can not give up this idea, please set the margins/paddings to 0 by default and implement sliders on the right sidebar where users can choose the size and even the unit of the top/bottom/left/right margins. Otherwise all that would cause more harm than joy.

@kjellr
Copy link
Contributor Author

kjellr commented May 16, 2019

Looking over #14407 again, it doesn't look like it specifically removed any margins from the Media & Text block.

Ah sorry — I'm talking about a lot of blocks here. 😄 I was referring to the Cover block, which used to have a bottom margin. That was definitely removed in #14407, so now the Cover block suffers from the same problem described in this PR.

GB did so in the past with columns block before and guess what... it has been removed as it caused unpredictable behaviour.

Just to be clear: we're only talking about the bottom margin, which still does exist for the columns block today. The goal here is to ensure that things don't look broken on themes that don't supply styles to these blocks by default.

Adding block margin/padding controls are still a possibility, and tracked in #11824.

@idea--list
Copy link

idea--list commented May 16, 2019

The goal here is to ensure that things don't look broken on themes that don't supply styles to these blocks by default.

I understand what the goal is, however hardcoding margins/paddings in GB without clearly informing the user just screws things up (not mention what may happen if GB hardcodes another unit type than theme coder particularly in case of paddings/margins)

In #12640 i was really frustrated just because of the hard coded margins/paddings i did not expect at all while coding my CSS for a client. Others reported the same issue in #12199 and #13035. Please take a look at the bottom of this image and notice how an unexpected bottom margin can screw up a real design.

So a i am not against the idea but please do it with settable sliders and unit types, deafaulting to 0. That way newbies and experts can set those with the same ease and neither user type gets frustrated. I also understand that designers of GB want to keep everything minimalistic, but computers need to be instructed as they still can not read our minds :-)

@kjellr
Copy link
Contributor Author

kjellr commented May 16, 2019

In #12640 i was really frustrated just because of the hard coded margins/paddings i did not expect at all while coding my CSS for a client.

Thanks for sharing, @idea--list. I can definitely relate to that sort of thing being frustrating. I'm a theme developer too, and I've had to write theme styles to override that sort of quirk too.

It's definitely something that Gutenberg could address through issues like #11824: that approach would turn those margins from a theme developer's decision into a user decision.

But regardless of whether or not that lands in a PR, the core of the conflict here is just that the Media & Text block comes with a bottom margin by default in the editor, but does not in the front end. I'd like for us to even that out one way or another, so users aren't surprised when their front end doesn't look like what they built in the editor.

@m-e-h
Copy link
Member

m-e-h commented May 16, 2019

@idea--list Pretty sure those column issues were because of an error in the CSS. So that actually needed "fixed".
I do agree though that opinionated styles shouldn't be added to the front-end by default.

intrinsic bottom margin (i.e. in style.scss, not just theme.scss). The reason being: it's an entirely new block.

I disagree a little. This seems like a perfect candidate for theme.scss.
A bottom margin on blocks is an opinionated style. There are surely some that would prefer the second image in the first post here.
And the age of the block shouldn't matter for old themes. They're all new blocks right? 😉

As asides to the margin issue @kjellr brings up here:

  • If we're thinking it's too difficult for older themes to opt-in to the theme.css file, perhaps we should look into making it easier for them to do so.
  • Has it ever been clearly defined what belongs in style.css and what belongs in theme.css?

@jasmussen
Copy link
Contributor

Has it ever been clearly defined what belongs in style.css and what belongs in theme.css?

I think it sort of has, but I also think it may be worth revisiting, because it's slightly complicated. For example, theme.scss gets loaded into the editor regardless of whether the theme has opted in, which is why moving the margin there won't bring the frontend and backend in sync unless the theme opts in. At the same time, it's probably not as simple as just not loading theme.scss into the editor unless the theme has opted in, because then numerous aspects will simply look completely unstyled in the editor for old themes, and it is still an editor after all.

But it should boil down to this:

  • style.scss: fundamental block styles. Loaded in editor an frontend.
  • editor.scss: editor-only block styles. Loaded only in editor.
  • theme.scss: opinionated block styles. Loaded in editor always, and frontend if theme opts in.

Stepping back for a minute, and looking at the current state of things, how would we change things? Keeping in mind, and correct me if I'm wrong, the two high level goals we want to solve are these:

  1. Make sure old WordPress themes that never gets updated to be Gutenberg aware, and with 30+% of the web this will be a high percentage, have a good experience in the editor.
  2. Ensure a clear and simple path forward for blocks to provide styles to the editor and theme alike, while still giving control to themers.

Strawman question: themes can opt in to opinionated styles. Should themes also be able to opt out of opinionated styles in the editor?

@idea--list
Copy link

idea--list commented May 17, 2019

I am definitely for a possibility to opt out. Even nicer would be to implement a notification if GB does opt in automatically and tell people how to opt out in case the opinionated styles just do not have the desired effect.

This might be a very different approach, but what if:
There was a new section in the theme customizer called "support for old themes" (i assume most people would and up looking there). That would be only visible if GB is the editor. Under that section there would be a list of different options to try to fix the looks of older themes in GB. "Set a top and bottom margin for blocks" could be a toggable option.
This way there would not be a whole css file with dozens of rules applied at once and while maybe fixing some graphical issues introducing other ones... but people could try to apply fixes in a granulated way

@benlk
Copy link
Contributor

benlk commented Jun 21, 2019

As a theme dev, I'd like for the ability to opt out of theme.scss styles entirely, and style.scss styles on a per-block basis. We end up having to write complicated selectors to override Gutenberg's highly-specific-selector opinionated styles (things like "caption should overlay image in gallery block") which would be much more easily accomplished by simply dequeueing gallery block styles.

If themes can opt out of opinionated styles in the frontend, and if the block editor is supposed to be WYSIWYG as it has been marketed, then themes should be able to opt out of opinionated styles in the editor in order to preserve WYSIWYG.

@mapk
Copy link
Contributor

mapk commented Jan 14, 2020

This was discussed by the Design Team in today's slack triage.

Can this be tested further with themes that don't have Gutenberg styling to verify this is still an issue?
The recommendation is to keep this consistent across blocks.

@mapk mapk added Needs Testing Needs further testing to be confirmed. and removed Needs Design Feedback Needs general design feedback. labels Jan 14, 2020
@youknowriad
Copy link
Contributor

Not sure if it helps, but just tested with a very old theme and there's no margin between two consecutive media and text blocks.

@youknowriad youknowriad added [Block] Media & Text Affects the Media & Text Block [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. and removed Needs Testing Needs further testing to be confirmed. [Type] Bug An existing feature does not function as intended labels Sep 16, 2020
@aristath
Copy link
Member

Perhaps instead of adding margins on the frontend we should remove the extra spacing from the editor?
Adding frontend styles to add margins could very well break a lot of things... We can't assume that they want them

@jasmussen
Copy link
Contributor

Perhaps instead of adding margins on the frontend we should remove the extra spacing from the editor?
Adding frontend styles to add margins could very well break a lot of things... We can't assume that they want them

That's what this PR did, and there's some great conversation there: #22208 — as with all things, it's not as simple as it might seem.

@youknowriad
Copy link
Contributor

@jasmussen so what do you think the path forward here, should we just close this issue for now?

@jasmussen
Copy link
Contributor

Ultimately I think no margins is the best approach, but I also don't think we can do that as things exist today. I would hope that perhaps Global Styles could help enable the status quote to continue existing, but for themes to still zero it out.

In the mean time, if Media & Text is a challenge, we could make an exception like we did for groups in #22209 (review).

What do you think?

@youknowriad
Copy link
Contributor

I'm not sure I have the full picture here personally to give the best answer. Having control over this in theme.json seems like a good option (the margin needs to become a style of all blocks for that) but I can't speak about Media & Text.

@kjellr
Copy link
Contributor Author

kjellr commented Sep 16, 2020

In the mean time, if Media & Text is a challenge, we could make an exception like we did for groups in #22209 (review).

Just to confirm, the suggestion here is to zero out the margins for Media & Text blocks in the editor, so that they match the front end by default? (If so, I think that makes sense as an initial step — we might as well make things consistent for now.)

@carolinan
Copy link
Contributor

Block themes: The spacing is now equally large in the editor and front thanks to the block gap.

Classic themes without theme.json: The difference is still visible. I think we may run into backwards compatibility problems with classic themes that have made up for this by adding their own styles. Is changing this margin still something that we want to do?

@carolinan
Copy link
Contributor

Let's close this, there has not been any discussions for over a year.

@carolinan carolinan closed this as not planned Won't fix, can't repro, duplicate, stale Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Media & Text Affects the Media & Text Block [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes.
Projects
None yet
Development

No branches or pull requests

10 participants