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 margin support to paragraph blocks. #37300

Closed

Conversation

ndiego
Copy link
Member

@ndiego ndiego commented Dec 10, 2021

Description

Fixes #37299

As noted in the linked issue, paragraph blocks are fundamental to the design of any website. As we move closer towards Full Site Editing, the need for full control over paragraph margins has become increasingly apparent. In fact, theme developers (myself included) are resorting to "magic classes" to zero out, or modify, margins on blocks. Allowing themes to opt-in to paragraph margin support will provide greater flexibility and decrease the reliance on custom CSS in the theme's stylesheet and/or crude "hacks".

As a side benefit, margin support works in tandem with blockGap. With blockGap enabled, margin-top is added to every block. While this is great 90% of the time, there are instances where you would want to remove/modify this margin. Margin support for paragraphs enables this functionality. Again, no need for "magic classes" or alternative solutions.

I have chosen to focus solely on the paragraph block in this PR, but separate PRs will be created for other block types that equally need greater dimensions support.

How has this been tested?

  1. Activate the latest version of the Twenty Twenty-Two theme.
  2. Make sure that "appearanceTools": true is set in theme.json
  3. Add a new page and add a paragraph block.
  4. Check that the Dimensions panel is visible and you can modify top and bottom margin on the paragraph.
  5. Add a custom margin and make sure it is reflected in the Editor and on the Frontend.
  6. Go back to the theme.json and add settings.blocks.core/paragraph.spacing.margin.false.
  7. Navigate to a paragraph block in the Editor and the Dimensions panel should be gone.

Screenshots

image

Types of changes

New Feature

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@andrewserong
Copy link
Contributor

Thanks for opening up the PR to opt-in to margin support! It works nicely for me in manual testing:

Kapture 2021-12-14 at 15 42 59

In the past, there's been some hesitancy in us opting in for the margin support due to the potential complexity for users in using margin controls. You raise an excellent point, though, both here and in #28356 (comment) that we'll need to allow individual overrides for blocks that are children of blocks that opt in to the layout block support. As they have a margin applied directly to them, how else do we offer the design flexibility to make overrides?

I know @youknowriad has been exploring tweaking the behaviour of the Layout block support and how it renders margins (#36521 (comment)) — by rendering the blockGap value directly rather than via the CSS variable. Riad, does opting-in for top/bottom margin support like this now make sense for individual overrides?

Also, CC: @jasmussen as we've discussed margin in the past, and potentially de-emphasising it in the UI. So I wonder if we do opt-in for the support, if we should explore it not being a default control?

Overall, personally I quite like the idea of opting in to the vertical margin support for blocks like this, Group, and possibly Heading. I believe @aaronrobertshaw has also made the point that if pretty much every block that could be a child of a layout block support needs to have individual control over margin, that perhaps margin support is something we could enable programmatically, too. That's a bit outside the scope of this PR, of course.

Thanks again for pushing this forward @ndiego!

@aaronrobertshaw
Copy link
Contributor

Thanks from me as well @ndiego for putting this together. It tests well 👍

@andrewserong's write up I think sums up the current state of affairs around spacing, along with questions we need to answer to proceed further.

There's another aspect to the margin support that is worth mentioning here. That is, we currently don't have margin visualization. @jasmussen explains this issue in his comment on a closed PR aimed at adding margin support to the Group block.

@jasmussen
Copy link
Contributor

Thanks all for the work and careful thought and attention! Aaron captured well my concern with margins, which is all about the user experience. As developers we take the behavior of margin for granted, know what it does. But in a visually flat canvas, with all the complexities of margin collapsing, padding and gap, it's incredibly hard to visualize what, precisely, happens when a margin is being adjusted. Depending on the context, changing the margin might not do anything at all (due to collapsing).

It's for that reason I've wanted to hold off on surfacing the UI controls until we have a better margin visualization, and have recommended the spacer block as a more intuitive alternative. If we had to add it, Andrew makes an excellent point: it could be de-emphasized in the UI, be a non-default control that you explicitly add to the tools panel.

Because of the conversation on the group block, I'd love more thoughts on this one.

@ndiego
Copy link
Member Author

ndiego commented Dec 14, 2021

The concerns around margin (and padding for that matter) are well warranted, which is why I think it's useful to focus on one block at time when weighing the merits of additional dimension UI. Understanding that I am operating from the standpoint of trying to design fully in the Editor with as little CSS as possible, allowing users to control margin at the block level is imperative to achieve certain designs. And of course this becomes more important with blockGap enabled.

That said, I agree that this should be completely "opt-in". In many cases, this functionality provides more control than is needed, especially for newer users or for those building websites for clients that need a very locked down experience.

But I feel theme.json is the answer here. Yes, the margin UI could be improved in the Editor to make it more intuitive, but theme developers can easily toggle on/off margin as they need. Some themes need all the bells and whistles in the Editor, and others need a more restrictive environment with style.css doing the majority of the heavy lifting. Making margin UI configurable in theme.json provides a very flexible environment for theme developers to operate in, which I believe is the beauty of the Editor.

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Aug 21, 2022
@ndiego
Copy link
Member Author

ndiego commented Aug 21, 2022

I am closing this PR in favor of #43455.

@ndiego ndiego closed this Aug 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Paragraph Affects the Paragraph Block [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Margin support for Paragraph blocks is desperately needed.
4 participants