WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 4 months ago

#53428 closed defect (bug) (fixed)

Twenty Nineteen: Polish new blocks added in 5.8

Reported by: AlePerez92 Owned by: ryelle
Milestone: 5.8 Priority: normal
Severity: normal Version: 5.8
Component: Bundled Theme Keywords: has-patch has-screenshots commit
Focuses: Cc:

Description

This is a continuation of #53389 for Twenty Nineteen issues only.

Attachments (9)

Screenshot 2021-06-16 at 15.10.57.png (217.6 KB) - added by AlePerez92 4 months ago.
Title within loop is out of viewport
Screenshot 2021-06-16 at 11.34.02.png (25.3 KB) - added by AlePerez92 4 months ago.
No separation with the blocks below a left/right aligned block
Screenshot 2021-06-16 at 13.55.03.png (69.1 KB) - added by AlePerez92 4 months ago.
No margins around full-width blocks
bookmarkblock.PNG (21.6 KB) - added by Boniu91 4 months ago.
Cover block left-aligned problem
Screenshot 2021-06-21 at 12.22.30.png (42.8 KB) - added by AlePerez92 4 months ago.
53428-query-loop-testing.png (885.5 KB) - added by hellofromTonya 4 months ago.
Testing query block before and after applying PR 1379. Works as expected
53428-cover-margin-testing.png (824.1 KB) - added by hellofromTonya 4 months ago.
Cover block margin testing before (in Firefox) and after (in Chrome) applying PR 1379
53428-before-computed-layout.png (586.9 KB) - added by hellofromTonya 4 months ago.
Before applying patch: cover block computed layout (with left and right alignment)
53428-cover-after-computed-layout.png (393.2 KB) - added by hellofromTonya 4 months ago.
After applying patch: cover block computed layout (with left and right alignment)

Change History (22)

@AlePerez92
4 months ago

Title within loop is out of viewport

@AlePerez92
4 months ago

No separation with the blocks below a left/right aligned block

@AlePerez92
4 months ago

No margins around full-width blocks

This ticket was mentioned in PR #1379 on WordPress/wordpress-develop by AlejandroPerezMartin.


4 months ago

Hello!

This PR aims to fix margins for several blocks in Twenty Nineteen theme:

## No margin around full-width blocks
https://i0.wp.com/user-images.githubusercontent.com/5501685/122239481-436a9300-cec1-11eb-9b8e-1ea6d8f541f2.png

### Steps to reproduce

  1. Activate _Twenty Nineteen_ theme
  2. Create a new page.
  3. Add a cover block.
  4. Change the block alignment to 'Full-width'.
  5. You'll now notice there are no margins around.

## No margin bottom on left and right aligned blocks
https://i0.wp.com/user-images.githubusercontent.com/5501685/122239352-26ce5b00-cec1-11eb-9091-beda45e5babf.png

### Steps to reproduce

  1. Activate _Twenty Nineteen_ theme
  2. Create a new page.
  3. Add a cover block.
  4. Change the block alignment to 'Left align' or 'Right align'.
  5. Add any other block below (e.g. heading as in the screenshot above).
  6. You'll now notice there are no margins below thus no separation from the block below.

## Full-width blocks within a query loop block
https://i0.wp.com/user-images.githubusercontent.com/5501685/122239531-4bc2ce00-cec1-11eb-90d5-faadd743c759.png

### Steps to reproduce

  1. Activate _Twenty Nineteen_ theme
  2. Create a new page.
  3. Add a query loop block.
  4. Change alignment to full-width.
  5. Insert blocks inside the loop block and set its alignment to full-width as well.
  6. This last block will be cropped and be placed out of the viewport.

Trac ticket: https://core.trac.wordpress.org/ticket/53428

#2 @JeffPaul
4 months ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 5.8

Thanks for the PR @AlePerez92, I'll look to get this reviewed quickly ahead of Beta 3 next week!

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


4 months ago

@Boniu91
4 months ago

Cover block left-aligned problem

#4 @Boniu91
4 months ago

@AlePerez92 I can see that the problem with margins still happens when:

  1. Adding a new page
  2. Adding Cover block and aligning it to the left
  3. Adding Cover block below and setting it to full-width

It's overlapping the built-in title then and also the next block below.

#5 @AlePerez92
4 months ago

Hi @Boniu91, thanks for testing this!

Unfortunately I can't reproduce that error, here is a screenshot of what I see.


Can you let me know which version of WP and browser are you using? Thanks!

#6 @onemaggie
4 months ago

This PR looks good to me. The issue with nested full width blocks inside the query block extending to the right is an editor/block issue and I'm going to submit a PR for that separately, otherwise this looks good to me in terms of the theme

#7 @onemaggie
4 months ago

The PR for the related Query block issue: https://github.com/WordPress/gutenberg/pull/32892

#8 @prbot
4 months ago

ryelle commented on PR #1379:

Thanks for the PR, @AlejandroPerezMartin ! I see there's a lot of whitespace changes, could you remove those from the PR? Cleanup like that usually happens in separate patches, unless you're touching those lines.

I think the addition of the .wp-block-post-template styles is enough to fix the issue, what do you think?

@hellofromTonya
4 months ago

Testing query block before and after applying PR 1379. Works as expected

@hellofromTonya
4 months ago

Cover block margin testing before (in Firefox) and after (in Chrome) applying PR 1379

@hellofromTonya
4 months ago

Before applying patch: cover block computed layout (with left and right alignment)

@hellofromTonya
4 months ago

After applying patch: cover block computed layout (with left and right alignment)

#9 @hellofromTonya
4 months ago

Testing Results

Env:

  • OS: macOS Big Sur
  • Browsers: Firefox 89.0.1 (for before) and Chrome 91.0.4472.106 (for after)
  • WordPress: latest trunk version

Query Loop Block

Before applying patch:

After applying patch:

Cover block: top and bottom margin

Page structure:

  • Cover block set to "left" alignment
  • Cover block set to "right" alignment
  • Cover block set to "full width"

Before applying the patch:

  • wasn't able to reproduce the problem
  • Blocks did not overlap
  • No space separation between blocks
  • Computed layout shows 28 px top and bottom margin for the first block (left aligned) 53428-before-computed-layout.png

After applying the patch:

Last edited 4 months ago by hellofromTonya (previous) (diff)

#10 @prbot
4 months ago

AlejandroPerezMartin commented on PR #1379:

Thanks for the PR, @AlejandroPerezMartin ! I see there's a lot of whitespace changes, could you remove those from the PR? Cleanup like that usually happens in separate patches, unless you're touching those lines.

I think the addition of the .wp-block-post-template styles is enough to fix the issue, what do you think?

Hello @ryelle! I undo the whitespace changes, my editor was configured to remove the trailing whitespaces, sorry about that.

Regarding the other changes, there's some overflow and the horizontal scrollbar appears on the editor, there's a wrapper element that has a negative margin of 8px, so having those values at 20px was causing this:

<img width="1160" alt="Screenshot 2021-06-22 at 23 24 23" src="https://user-images.githubusercontent.com/5501685/123001725-52fe4600-d3b1-11eb-942a-daaa5075dfcc.png">

#11 @prbot
4 months ago

ryelle commented on PR #1379:

Thanks!

there's some overflow and the horizontal scrollbar appears on the editor

Ah yeah, I see that now. The 16px change makes sense then 👍🏻

I pushed up a commit to remove the margins from nested blocks, otherwise I was seeing the titles cut off still:

<img width="481" alt="Screen Shot 2021-06-22 at 5 41 09 PM" src="https://user-images.githubusercontent.com/541093/123005066-82965980-d383-11eb-870b-7ac75bc14677.png">

#12 @ryelle
4 months ago

  • Keywords commit added; needs-testing removed

The PR looks good to commit now.

#13 @ryelle
4 months ago

  • Owner set to ryelle
  • Resolution set to fixed
  • Status changed from new to closed

In 51209:

Twenty Nineteen: Update margins on full- and wide-aligned blocks in the editor.

Previously, full width blocks would cause a horizontal scrollbar, and nesting full width blocks would cause the content to be pulled off the screen. Now wide and full width blocks can be nested without any visual issues.

Props aleperez92, Boniu91, onemaggie, hellofromTonya.
Fixes #53428.

Note: See TracTickets for help on using tickets.