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

Inconsistent treatment around reusable blocks #20687

Closed
karmatosed opened this issue Mar 6, 2020 · 8 comments
Closed

Inconsistent treatment around reusable blocks #20687

karmatosed opened this issue Mar 6, 2020 · 8 comments
Labels
[Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) Needs Design Needs design efforts.

Comments

@karmatosed
Copy link
Member

This was on the latest build, the spacing feels out of balance on the reusable block:

image

My suggestion would be to not highlight the block or use the background shade as in paragraph block. This gif shows the variations in treatment.

2020-03-06 17 03 33

@jasmussen looping you in as feels like something comes from the latest iteration styling and I don't want to break a pattern here.

@karmatosed karmatosed added the [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) label Mar 6, 2020
@karmatosed karmatosed changed the title Consider spacing around reusable blocks Inconsistent treatment around reusable blocks Mar 6, 2020
@MichaelArestad
Copy link
Contributor

I am glad you brought this up. There is a pretty interesting visual treatment on the reusable block that raises some questions for me.

  1. What led the design to have a header/action bar?
  2. Could the edit action be part of the block toolbar instead?
  3. Should the visual treatment be identical to selected template or template part blocks?

CC @shaunandrews @jasmussen

@MichaelArestad MichaelArestad added the Needs Design Needs design efforts. label Mar 6, 2020
@karmatosed
Copy link
Member Author

Should the visual treatment be identical to selected template or template part blocks?

This feels profound and a point that needs deeper debate.

@chrisvanpatten
Copy link
Contributor

Might also be related to #20558.

@jasmussen
Copy link
Contributor

jasmussen commented Mar 9, 2020

That spacing does seem to be related to #20558 at an initial glance, good catch Chris.

Definitely feels like that's worth addressing first.

As a very quick primer (forgive me for repeating myself and with things I know you already know, but I find it helpful at times): an unselected block should be a preview of the block contents, and the selected block can show additional controls.

That was also the reason for the header/action-bar, to answer your questions Michael: it did not feel appropriate to have that be part of the block toolbar itself — though a convincing mockup can overturn that thought of course! Just need to be able to accommodate a good long label.

However a reusable block is not meant to be a template part, and serves a separate, much more light-weight purpose.

@mtias wrote a comment in Slack the other day, which deserves surfacing outside of that as well, so I'll quote it in its entirety:


There are some considerable differences between the two, even if their internal architecture is mostly the same:

  • Reusable blocks are meant to coexist with regular blocks as a way for the user to make snippets of content that are globally synchronised. (A testimonial, a “this post is part of a series” block, the “all birthdays” Matt has at the bottom of his birthday posts 🙂 , etc). They are primarily user content and could be conceptualised as custom user blocks.
  • Template Parts, on the other hand, are the equivalent — in blocks — of theme template parts. They are generally defined by a theme first. They carry some semantic meaning (could be swapped between themes such as header -> header or footer -> footer) and can only be inserted in the site editor context (that is, within “templates”). They are primarily site structure and are never to be mixed with the post content editor.
  • User roles might be configured entirely different for both of these entities, even if behind the scenes they are just groups of blocks saved somewhere in the database. Access to template parts should require theme editing capabilities while reusable blocks can be just “author” content.
  • A template or template part can include a reusable block. A reusable block should not contain a template part.
  • Block patterns are an entirely different thing in behaviour and presentation to the two of them since they are saved locally upon insertion and don’t exist globally. They are just blocks you insert with starter content that is meant to be changed by the user every time.
  • Block variations, inner blocks, block templates, and so on, are developer APIs not to be exposed to users. A template in the context of the block API is just an array of blocks that can be passed to any inner block or editor instance as the way to initialize a block tree.

Given this differentiation, it feels appropriate to me that the visual treatment of a reusable block is more light-weight than that of a template part.

@dwolfe
Copy link

dwolfe commented Mar 31, 2020

I just checked this in master and it appears that reusable blocks now have padding that prevents the content butting up against the border:

Reusable-Block-Spacing

I couldn't find the work that introduced those changes, does anyone know which issue/PR that was? Maybe there's additional context there.

Also, @karmatosed - Just to clarify the issue that you're raising, is it that reusable blocks appear different than normal blocks? If that's the case, I feel like there should be some visual difference(s) to indicate that they're different types of blocks with different properties, no?

If that's not the issue, and this is just a discussion about the lack of padding between the content and the border, I agree, that needed to be addressed. The current solution isn't perfect - it does mean that there's a difference between the editor view and how it looks on your site - but it might be a step forward. All of which is to say, does this still need work, or can we close this one?

@jasmussen
Copy link
Contributor

@dwolfe There are a few things going on here.

I just checked this in master and it appears that reusable blocks now have padding that prevents the content butting up against the border

This is a regression, seems to be as a result of a recent refactor to how block margins are treated. I think this is to blame: https://github.com/WordPress/gutenberg/pull/21099/files#diff-ee2ed3adbe2578628039530c717a9a93R640

Screenshot 2020-04-01 at 09 00 06

Just to clarify the issue that you're raising, is it that reusable blocks appear different than normal blocks? If that's the case, I feel like there should be some visual difference(s) to indicate that they're different types of blocks with different properties, no?

It is my understanding that it's the fact that margins are different. Observe what happens before and after these paragraphs are turned into a reusable block:

conversion

Note that the margin doubles. This is because a sequence of paragraphs rely on margins collapsing. But as soon as the block becomes "reusable", it becomes its own block area which divs that prevent this margin collapsing, therefore doubling the margin. One idea for how a PR could fix this would be to write CSS to hook into the reusable block itself, then nullify the top and bottom margin on the first and last blocks respectively. However this is hacky, and I wish there was a way to simply allow the margin collapsing to happen organically, but without display: contents; I'm not sure this is easily possible. Would love to be proven wrong in a PR though!

It's worth noting that this margin behavior is unchanged. Here's the 5.4 gutenberg UI:

before

Here's the G2 UI:

after

@jasmussen
Copy link
Contributor

PR with fix for horizontal padding here: #21312

@karmatosed
Copy link
Member Author

As there is a padding PR in, let's close this up for now and loop back in iterations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) Needs Design Needs design efforts.
Projects
None yet
Development

No branches or pull requests

5 participants