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

Synced Patterns: Not receiving alignment attribute in editor #8288

Closed
brettsmason opened this issue Jul 30, 2018 · 90 comments
Closed

Synced Patterns: Not receiving alignment attribute in editor #8288

brettsmason opened this issue Jul 30, 2018 · 90 comments
Assignees
Labels
[Block] Block The "Reusable Block" Block [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) Needs Dev Ready for, and needs developer efforts [Type] Bug An existing feature does not function as intended

Comments

@brettsmason
Copy link

Describe the bug
I'm attempting to use the shared blocks feature but have hit an editor specific issue with alignfull and alignwide. When converting a a wide or full width block to shared, in the editor view the block looses its width. This does not apply to the front end which works fine.

To Reproduce

  1. Create a block that supports full width/wide width (eg cover block).
  2. Save the block as a shared block.
  3. The block is no longer the width specified.

Expected behavior
I expected the shared block to keep the current data-align: full/wide that non-shared blocks receive when setting the alignment. This makes it very difficult currently to use shared blocks for aligned blocks, as its not an accurate representation of how the block actually looks.

Screenshots
shared-block

@designsimply designsimply added [Type] Bug An existing feature does not function as intended [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) Needs Testing Needs further testing to be confirmed. labels Jul 31, 2018
@brandonpayton
Copy link
Member

I am able to reproduce this.

@brandonpayton brandonpayton removed the Needs Testing Needs further testing to be confirmed. label Sep 26, 2018
@noisysocks noisysocks added this to the WordPress 5.0 milestone Nov 5, 2018
@notnownikki
Copy link
Member

Looks like getBlock in packages/editor/src/store/selectors.js doesn't apply the filters needed to add the extra props and attributes for features provided by the supports hooks. I'll see what I can figure out.

@notnownikki
Copy link
Member

notnownikki commented Nov 16, 2018

@youknowriad @noisysocks could I get a bit of feedback here? The reusable block is got with the originalContent but the features supplied by the supports hooks are only applied on save, so we'd have to use getSaveContent from the blocks serializer to make sure the reusable block has exactly the same output as the original block would have when it's saved shown in the editor. Does that sound like an acceptable solution?

@notnownikki
Copy link
Member

oook, so that's not quite the problem. I'm still investigating, but it looks like when the block is rendered, the align isn't applied because it's taking the supports from the block block, instead of the cover block.

@notnownikki
Copy link
Member

Yeah, that seems to be the problem.

When the filters are applied, they get core/block and because core/block constructs a BlockEdit element insides its render, it never gets exposed to the filters it needs for supports to work.

Not sure how to handle this one yet.

@notnownikki notnownikki self-assigned this Nov 16, 2018
@jasmussen
Copy link
Contributor

I can explain why this is happening. I don't know that we can get this fixed for 5.0, but perhaps we can for 5.0.1. But it's important to note — this is an issue isolated to the editor, the front-end view will still render correctly.

All blocks in the editor have a max-width, and are centered.

Blocks with align-wide have a wider max-width, and blocks with align-full have no max-width and therefore span the full width.

If you convert a wide image into a reusable block, it is now a wide image wrapped in a reusable block container. I.e. the image inside the container still has a wide alignment, but it is now constrained by the parent block, the reusable block container, which has the same max-width as any other normal block.

I can think of two ways to fix it:

  1. Make all reusable blocks always be fullwide. Any blocks stored inside will retain their max-widths and be correctly centered. See:

screenshot 2018-11-16 at 19 30 00

  1. Let the reusable block detect that a child block inside has a wide or fullwide block, and assign that attribute to the reusable block group itself. That way the reusable block will be wide when a child is wide, and fullwide when a child is wide.

2 is arguably the best user experience, but is probably also the most complex.

1 is relatively trivial to implement, but is arguably a little confusing. But it still might be the best way to do it. Assigning for some design feedback.

@jasmussen jasmussen added the Needs Design Feedback Needs general design feedback. label Nov 16, 2018
@notnownikki
Copy link
Member

It's not specific to align either, any feature applied with the supports block attribute does not get applied to reusable blocks in the editor. So there's the design aspect, and having supports propagate to reusable blocks too.

@jasmussen
Copy link
Contributor

If we had access to the CSS :has this would be easy to fix. But I wonder if that selector will ever come to pass or be performant.

@noisysocks
Copy link
Member

Note that a reusable block can consist of several blocks, which complicates things. What should the supports.align value be for a reusable block that has a Cover block and a Paragraph block?

oook, so that's not quite the problem. I'm still investigating, but it looks like when the block is rendered, the align isn't applied because it's taking the supports from the block block, instead of the cover block.

That's right, the editor will use the supports, description, title etc. that's defined by core/block.

We could have editor/src/hooks/align.js have a special case for core/block which looks up the referenced block type and use its supports attribute. I think that it's fine to treat core/block as a special case since it is a special block in that it implements some of the editor's core functionality.

@jasmussen
Copy link
Contributor

Note that a reusable block can consist of several blocks, which complicates things. What should the supports.align value be for a reusable block that has a Cover block and a Paragraph block?

It is not unlikely that I'm missing something, so forgive me if I am.

But what I'm suggesting in #8288 (comment), to clarify, is that _no matter the blocks inside a reusable group supports, if we make the reusable block group full width, the children will render correctly. Simply because when the reusable block group is fullwide, then there is room for the children to be constrained by their intrinsic max-widths.

Is that not right?

In other words, a "fix" could be either:

  1. Always making reusable block groups fullwide
  2. Making resuable block groups have the widest alignment of any of the children

To elaborate on 2, if you have 10 blocks in a reusable group, and one of them has align-wide, it's enough that the reusable block group receives align-wide. If one of them has full-wide, it's enough that the reusable block group receives full-wide.

@noisysocks
Copy link
Member

I moreso just wanted to drop a note to remind ourselves (myself?) that 1 reusable block ≠ 1 block 🙂

You're totally right, and either of the two fixes you described would fix the issue.

  1. Always making reusable block groups fullwide

This is easy to do, but it wouldn't be the best UX as you say 🙂

  1. Making resuable block groups have the widest alignment of any of the children

I'd much prefer this if we can get it to work. The challenge is that alignment is calculated by looking at the block's supports attribute which is completely static. We could investigate making it more dynamic or adding a special case for core/block.

@jasmussen
Copy link
Contributor

Thank you for the sanity check and clarification. I agree the 2nd option is the nicer one. But the 1st option would be one we could implement right now, if this needed an urgent fix. However given that it's "only" an visual inconsistency in the editor, and the front-end works as it should, I wouldn't call it critical enough to rush a fix for.

@codetipi
Copy link

Can I add that this also happens for left/right aligned blocks.

As a quick example - add an Image block, align it to left and make it 50% width, then turn it into a reusable block. The reusable block will now show as full-width even though on the frontend it's 50% and floated.

Is there a reason reusable blocks don't have the standard block's data-align attribute? I think adding it would fix all of it.

@mapk
Copy link
Contributor

mapk commented Mar 8, 2019

I'm removing the "Needs Design Feedback" label since it appears we're not in a rush on this, and number 2 above seems like the direction we want to go.

  1. Making resuable block groups have the widest alignment of any of the children

@getdave
Copy link
Contributor

getdave commented Apr 3, 2019

@youknowriad I started looking into this as it somewhat limits the utility of the new Section Block.

Could you advise at a high level how it might be possible to dynamically set an align attribute on the core/block. I know how to query the child Blocks and find the Block with the "widest" align setting but I can't for the life of me fathom how that could be passed to the Reusable Block.

@richtabor
Copy link
Member

What about this: If the full/wide attribute is detected, then the pattern is inserted within a group block with said alignment applied?

@ndiego
Copy link
Member

ndiego commented Jun 27, 2023

What about this: If the full/wide attribute is detected, then the pattern is inserted within a group block with said alignment applied?

This would solve the frontend issue, but not the Editor, I believe. The wrapper around the synced pattern in the Editor causes issues with alignment controls and layout.

@richtabor
Copy link
Member

The wrapper around the synced pattern in the Editor causes issues with alignment controls and layout.

This is what that would look like; not an ideal experience (either auto-applied, or manually):

CleanShot 2023-06-27 at 19 33 43

@richtabor
Copy link
Member

Why is this wrapper not treated like the Template part block, which does allow for alignment width controls when nested in Group blocks? Aren't they quite similar?

CleanShot.2023-06-27.at.19.39.52.mp4

@youknowriad
Copy link
Contributor

@richtabor What I'm hearing is that you basically want a reusable block to save "attributes" that will be applied to the instance depending on its context:

  • So if I'm inserted within a container that supports full width, apply full width
  • Potentially the same reusable block can say: if I'm inserted within a "flex" container, apply a "flex-self" or something

--

To simplify we could say that we only save a single "position" attribute, so a reusable block can't have both "align: full" and "flex-self" (which would allow us to continue using the existing storage mechanism.

Worth noting though that, A user can decide to change the "alignment" on the instance and it won't be saved again within the reusable block (it's just an instance value, and the value saved within the reusable block saves as "initial value").

I think this could work, but I also think that in order to do this properly, we need to "normalize" position attributes. In other words, the block-editor would know about these attributes and consider them "special". In face, there's a number of behaviors that could be added to these attributes:

  • Use a single name: probably "position" for this attribute: its value could be align full when used within a regular container and it could also be align-self when used within a flex container... (there are also variations within grid...)
  • Add the smart behavior on reusable block insertion: (apply the attribute to the instance parent and remove it from the content itself)
  • When moving a "regular" block between two containers, if the containers are of different types, reset that attribute. For instance when moving a block from a regular container to a flex container remove the "align: full"...

I think it's a lot of work but it's doable. First step being to unify these attributes.

@strarsis
Copy link
Contributor

strarsis commented Jul 3, 2023

From a pragmatic perspective, currently using reusable blocks means that the block inside will always be constrained to content width, regardless whether it is aligned widely or fully. This requires additional editor styles for forcing the reusable block wrapper to be wide enough.

@ndiego
Copy link
Member

ndiego commented Jul 5, 2023

Folks testing Beta 3 are stumbling onto this issue: #52312.

@mikemcalister
Copy link

Hey folks, I hit this width/alignment issue on the very first synced pattern I tried to create. I'm also working with a group of testers for my upcoming theme, and they've all encountered this quirk independently as well. Based on the many wide-width patterns folks are creating, unfortunately it's going to be an issue they run into right away.

I don't have a proposed solution outside of what's already being discussed here (great thoughts so far), but for the sake of optics around this powerful new feature, I'm hoping we can get some priority because of how visible this quirk is. Happy to throw around ideas and test POCs.

Synced patterns are the talk of the town in my circles! People seem to be more excited about them than they were about regular ol' patterns. 😅

@markhowellsmead
Copy link

markhowellsmead commented Jul 24, 2023

This issue has been happening in our projects since the beginning of 2019, which makes reusable blocks (and now synced patterns) unusable for every block-based project we've ever created. (As reported in #52893, the issue is always present in every client block theme we've ever built, as well as other public themes.)

@richtabor
Copy link
Member

How did folks get around this previously? Or has this always been an issue — but now that synced patterns are more front-and-center in 6.3, it's become bigger?

@ndiego
Copy link
Member

ndiego commented Jul 24, 2023

How did folks get around this previously? Or has this always been an issue — but now that synced patterns are more front-and-center in 6.3, it's become bigger?

The only solution is to place a synced pattern inside of a Group block with the layout toggle disabled. This will make the synced pattern fill the Group, and then you can set alignment on the Group. It's a fairly convoluted workaround that is not intuitive to most users.

My sense is that folks who ran into this issue with reusable blocks just moved on, perhaps to just normal (unsynced) patterns. I know I did. But now that synced patterns are font-and-center, as well as the talk about partially-synced patterns, this issue becomes far more problematic.

@brettsmason
Copy link
Author

I personally simply stopped using reusable blocks since I opened this issue because their use was greatly diminished for my requirements. Fingers crossed something can be worked out with synced patterns becoming a thing.

@JiveDig
Copy link

JiveDig commented Jul 24, 2023

We use them for ourselves and understand the quirk, but we don't teach clients or theme customers about it. We sort of pretend it's not a thing because it's been too confusing for the ones we show.

@mikemcalister
Copy link

How did folks get around this previously?

The only workarounds I could find previously involved extra grouping as @ndiego mentions. However, I was hesitant to share this with users as a fix because it seemed a bit too hacky and folks were already having a hard enough time with the group/alignment/content width setting.

@Mamaduka Mamaduka removed this from the WordPress 5.x milestone Jul 27, 2023
@richtabor
Copy link
Member

This is from one of the 6.3 release videos; shows the issue occurring right off:

CleanShot.2023-08-08.at.14.38.27.mp4

@bgardner
Copy link

I am experiencing this as well and created an issue (#53500). I closed that issue as a duplicate b/c originally my search for synced didn't show this issue in the results.

@Olein-jp
Copy link
Contributor

I encountered this problem like everyone else.

And while the hack of wrapping a synched pattern block with a group block improves things somewhat, as others have said, I too think it is too difficult for consumer users.

Since version 6.3, when Pattern was featured, many users will be challenged with site editing using synchronised and asynchronous patterns. I am looking forward to a solution that will not confuse users at that time, with respect to your great contribution.

@annezazu annezazu changed the title Reusable Blocks: Not receiving alignment attribute in editor Synced Patterns: Not receiving alignment attribute in editor Aug 22, 2023
@annezazu
Copy link
Contributor

Noting that I've renamed this issue to "Synced Patterns" in light of WordPress 6.3's release and the overall reusable blocks rename to improve discoverability and, ideally, increase feedback.

@AceMedia
Copy link

Also experiencing this issue. It would be nice to have the Synced Patterns retain their width settings, or perhaps even have an initial synced setting with some basic layout overrides (width, padding etc.) without changing the content. Maybe with some sort of reset/ovverride (of the defaults)?

I can work around this myself but I know clients and regular folks are going to be stumped by this one and the issue is present in the 6.3 promotional video so it's highlighted right there.

@aaronrobertshaw
Copy link
Contributor

The primary issue here of synced patterns (formerly reusable blocks) "losing" their alignment has been addressed via #54416 so I'll close this issue for now.

For some additional context, several other approaches to solving this problem were explored via:

The final approach that has been merged via #54416 effectively applies layout styles and alignments only within the editor without adopting layout block supports in a hacky way. Long term, we might wish to pursue a new core/block-v2 that can apply a wrapper to both the editor and frontend without causing BC issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Block The "Reusable Block" Block [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) Needs Dev Ready for, and needs developer efforts [Type] Bug An existing feature does not function as intended
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.