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

Mayland Blocks: Add support for spacing overrides #3456

Closed

Conversation

alaczek
Copy link
Contributor

@alaczek alaczek commented Mar 12, 2021

Changes proposed in this Pull Request:
Mayland and other Varia child themes have a few classes built-in that help to control the spacing. This PR introduces the same spacing override classes to Mayland Blocks.

Related issue(s):
Addresses #3424

Testing:

  1. Checkout this branch
  2. Zip the mayland-blocks theme to upload to a jurassic.ninja website.
  3. Import & activate the theme into a WordPress jurassic.ninja site.
  4. Open the Gutenberg editor and add two blocks. There should be a margin between them.
  5. Open the HTML element inspector and modify the classname of either or both blocks.
  6. Include variations of spacing overrides introduced in this PR, like margin-top-none, margin-bottom-none, margin-top-half, etc.
  7. Verify that the margins on the blocks are modified.

alaczek added 2 commits March 12, 2021 14:35
Adding a custom variable for vertical spacing that can be used in style.css.
Add spacing overrides so that we can set margin/padding to half-width/height, or remove it completely.
@alaczek alaczek requested a review from kjellr March 12, 2021 03:45
@alaczek
Copy link
Contributor Author

alaczek commented Mar 12, 2021

One thing I noticed but wasn't sure how to fix was the default top/bottom block margin (without any classes), which is 20px. I would love to bump it up to 32px.

@kjellr kjellr requested a review from a team March 15, 2021 12:48
@scruffian
Copy link
Member

I feel uncomfortable introducing these classes to a block based theme. If we need this kind of control then I think it should made available to users in the UI, but I'm curious what others think.

cc @jasmussen

@jasmussen
Copy link
Member

Yeah this is a tough one. In my own unreleased experimental theme, I've created a few "helper classes" like these myself, classes that I apply to blocks to have them do my bidding, in absence of the block editor itself supporting them. The good news is, nearly all of them I've been able to retire as the global styles project keeps gaining in features.

And ultimately I would agree with Ben, these all seem like classes that exist to be absorbed by the global styles project, so they are user configurable. The thing is, they will land there, hopefully sooner rather than later, I recall @youknowriad mentioning it just last week. And when they do, people will be confused in the cases where it doesn't work, because the !important is overriding it.

Is there a way to solve it that is less reliant on these "utility" helper classes? I know TT1 did some super opinionated stuff with style variations that might be a holdover?

@alaczek
Copy link
Contributor Author

alaczek commented Mar 18, 2021

To give some background, we're putting a few header patterns in preparation for full site editing, and manipulating the vertical space is essential with things like the visual grouping of elements (for example, site title and tagline):

rosetta_pattern

To Joen's idea, I saw this kind of thing in Eksell theme, where a few blocks offer a "No Vertical Margin" style. Seeing it in action, I'm not sure this is the right way to go. It really feels like something that should be handled via block setting (which brings us back to Ben's point).

image

I agree that having this handled in core would be ideal. If adding these classes would later interfere with settings, then it's a solid reason no to proceed with this PR.

@kjellr
Copy link
Contributor

kjellr commented Mar 18, 2021

While we could technically build in some theme-specific overrides to address this, those sorts of spacing changes should be the same across all themes. Frankly, the lack of consistent margin + padding controls is one of the most glaring issues when trying to build block-based themes (or block patterns for that matter).

This Gutenberg issue would address the issue, so I recommend we put our effort towards getting this one closed instead: WordPress/gutenberg#28356

@alaczek
Copy link
Contributor Author

alaczek commented Mar 26, 2021

Agreed, closing, and let's focus on the core issue instead.

@alaczek alaczek closed this Mar 26, 2021
@scruffian scruffian deleted the add/mayland-blocks-support-for-spacing-overrides branch March 26, 2021 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants