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

Migrated button markup causing rendering problems #28957

Open
fullofcaffeine opened this issue Feb 12, 2021 · 11 comments
Open

Migrated button markup causing rendering problems #28957

fullofcaffeine opened this issue Feb 12, 2021 · 11 comments
Assignees
Labels
[Block] Buttons Affects the Buttons Block Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Bug An existing feature does not function as intended

Comments

@fullofcaffeine
Copy link
Member

Description

I have a somewhat old block buttons/button markup that looks like this:

<!-- wp:buttons -->
<div class="wp-block-buttons"><!-- wp:button {"align":"center"} -->
<div class="wp-block-button aligncenter"><a class="wp-block-button__link">Contact</a></div>
<!-- /wp:button --></div>
<!-- /wp:buttons -->

Upon upgrading to the latest Gutenberg, I noticed some unexpected rendering behavior upon reloading one of my posts:

  1. The button did not respect its align property unless it was set to right;
  2. If I added new buttons, they wouldn't align horizontally with the old ones;
  3. The rendering / DOM structure looks broken (see the screencast below).

I tried to debug this and I suspect there's something fishy in one of the deprecations. I'm not entirely sure how they work, but it seems that the culprit is somewhere around here (might be wrong!).

Step-by-step reproduction instructions

  1. Create a new post or page;
  2. Change to the code editor and paste the following block markup: https://gist.github.com/fullofcaffeine/8e42e3491549c177f66f78697d5aa997
  3. Go back to the visual editor
  4. Notice that even though the "align" attribute is set to "center", the button still aligns to the left.
  5. Go back to the code editor
  6. Change the value of the "align" attribute in {"align":"center"} to "right". Exit the code editor;
  7. Notice how now it aligns to the right 🤔
  8. Change the alignment back to center or left (or paste the code over again to start from scratch);
  9. Now try adding other buttons alongside this one;
  10. Notice how they don't horizontally align :/

Expected behavior

I expect the old markup to correctly deprecate and migrate to a newer version that matches the latest and doesn't cause any rendering problems. This can be problematic as buttons stop respecting the previous align setting and end up breaking the layout.

Actual behavior

Something is wrong somewhere in the deprecation/migration pipeline that generates an inconsistent markup and ends up causing rendering issues as I described above.

Screenshots or screen recording (optional)

Peek.2021-02-11.17-55.mp4

WordPress information

  • WordPress version: 5.7-beta2-50285
  • Gutenberg version: 10.0.0-rc.1
  • Are all plugins except Gutenberg deactivated? Yes
  • Are you using a default theme (e.g. Twenty Twenty-One)? Yes

Device information

  • Device: Desktop
  • Operating system: Ubuntu Linux 18.04
  • Browser: Firefox 86.0b8 (64-bit)
@talldan
Copy link
Contributor

talldan commented Feb 12, 2021

Thanks for the report @fullofcaffeine.

I did some testing and noticed that if the inner button block is pasted on its own into the code editor it works ok:

<!-- wp:button {"align":"center"} -->
<div class="wp-block-button aligncenter"><a class="wp-block-button__link">Contact</a></div>
<!-- /wp:button -->

So I think it's rather than being caused by a deprecation, it's caused by new styling on the <div class="wp-block-buttons"> element, that element is now a flex container causing all the button block elements to shrink. (Since this changes - https://github.com/WordPress/gutenberg/pull/23168/files#diff-ba5e352fd98cd49d679e248a86f7d04077ed547c26ca9482d002d015cf93c9b4R2)

I think there's a simple fix, which would be to make any aligned blocks in a buttons container use the flex-grow: 1; css property. I'll create a PR.

This should also be added to WordPress 5.7, so adding this issue to the project.

@talldan talldan added Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta and removed Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Feb 12, 2021
@talldan talldan self-assigned this Feb 12, 2021
@talldan
Copy link
Contributor

talldan commented Feb 16, 2021

This is a bit harder than I thought, the problem with flex-grow: 1 is that multiple button blocks would appear on the same row. Will continue looking into it, hopefully it'll still just be a CSS fix.

@youknowriad
Copy link
Contributor

The issue here is that you shouldn't be able to add alignments to the inner button block unless there's no wrapper. So the question is here is how did you manage to get that markup? Did you write by hand?

@youknowriad youknowriad added [Status] Needs More Info Follow-up required in order to be actionable. and removed [Type] Regression Related to a regression in the latest release labels Mar 8, 2021
@talldan
Copy link
Contributor

talldan commented Mar 8, 2021

@youknowriad I think that's only true since #26380, but buttons created before that may have had alignment on the inner button block.

edit: I may be wrong though—would have to check out quite an old version of the code.

@youknowriad
Copy link
Contributor

youknowriad commented Mar 8, 2021

buttons created before that may have had alignment on the inner button block.

Buttons created before aren't supposed to have a wrapper and if there's a migration in place somewhere it should move the "align" to the parent wrapper instead. (The linked PR just refactors the block to disable alignments in a different way, the alignments were still disabled)

@kraftner
Copy link

I just got bitten too by this. And although I wasn't able to find the reason in code I am 99% confident it is caused by by a migration since I tracked down the change in the affected post:

<!-- wp:button {"align":"center"} -->
<div class="wp-block-button aligncenter"><a class="wp-block-button__link" href="#anmelden">Jetzt anmelden</a></div>
<!-- /wp:button --></div></div>

got transformed into

!-- wp:buttons {"align":"center"} -->
<div class="wp-block-buttons aligncenter"><!-- wp:button {"align":"center"} -->
<div class="wp-block-button aligncenter"><a class="wp-block-button__link" href="#anmelden">Jetzt anmelden</a></div>
<!-- /wp:button --></div>
<!-- /wp:buttons --></div></div>

I was then also able to reproduce it by creating the un-wrapped legacy button above and then migrating it to the wrapped new version via the UI. Which resulted in the broken markup.

@talldan
Copy link
Contributor

talldan commented Mar 15, 2021

From what I understand, there isn't any code that would automatically migrate a button block to be wrapped in a buttons block.

@youknowriad
Copy link
Contributor

From what I understand, there isn't any code that would automatically migrate a button block to be wrapped in a buttons block.

This shouldn't be a problem though, the question is how to get a button with alignment inside a button wrapper.

@kraftner
Copy link

From what I understand, there isn't any code that would automatically migrate a button block to be wrapped in a buttons block.

I manually transformed it via the "Transform to" function in the UI.

@youknowriad
Copy link
Contributor

@kraftner Makes sense, that's the issue than, the "transform" should move the "align" attribute to the wrapper instead of keeping it in the child. 👍

@youknowriad youknowriad added Good First Issue An issue that's suitable for someone looking to contribute for the first time and removed [Status] Needs More Info Follow-up required in order to be actionable. labels Mar 15, 2021
@jordesign jordesign added the [Type] Bug An existing feature does not function as intended label Aug 17, 2023
@annezazu
Copy link
Contributor

annezazu commented Sep 3, 2024

In trying to replicate this, I reused the mark up above, added it into a post via the code editor, and transformed it to a buttons block:

<div class="wp-block-button aligncenter"><a class="wp-block-button__link" href="#anmelden">Jetzt anmelden</a></div>
<!-- /wp:button --></div></div> 

Here's a video:

transform.button.mov

I think this replicates the initial problem of a button block not having alignment but I'm not quite sure. Is this issue still ongoing from everyone else's point of view? Testing with WordPress 6.6.1 and Gutenberg 19.1 using a simple WordPress Playground instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Buttons Affects the Buttons Block Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

6 participants