Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#53428 closed defect (bug) (fixed)

Twenty Nineteen: Polish new blocks added in 5.8

Reported by: aleperez92's profile AlePerez92 Owned by: ryelle's profile 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 3 years ago.
Title within loop is out of viewport
Screenshot 2021-06-16 at 11.34.02.png (25.3 KB) - added by AlePerez92 3 years 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 3 years ago.
No margins around full-width blocks
bookmarkblock.PNG (21.6 KB) - added by Boniu91 3 years ago.
Cover block left-aligned problem
Screenshot 2021-06-21 at 12.22.30.png (42.8 KB) - added by AlePerez92 3 years ago.
53428-query-loop-testing.png (885.5 KB) - added by hellofromTonya 3 years ago.
Testing query block before and after applying PR 1379. Works as expected
53428-cover-margin-testing.png (824.1 KB) - added by hellofromTonya 3 years 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 3 years 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 3 years ago.
After applying patch: cover block computed layout (with left and right alignment)

Change History (22)

@AlePerez92
3 years ago

Title within loop is out of viewport

@AlePerez92
3 years ago

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

@AlePerez92
3 years ago

No margins around full-width blocks

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


3 years ago
#1

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
3 years 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.


3 years ago

@Boniu91
3 years ago

Cover block left-aligned problem

#4 @Boniu91
3 years 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
3 years 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
3 years 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
3 years ago

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

ryelle commented on PR #1379:


3 years ago
#8

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
3 years ago

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

@hellofromTonya
3 years ago

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

@hellofromTonya
3 years ago

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

@hellofromTonya
3 years ago

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

#9 @hellofromTonya
3 years 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
  • Computed layout shows 28 px top and bottom margin for the first block (left aligned) 53428-before-computed-layout.png

After applying the patch:

Version 0, edited 3 years ago by hellofromTonya (next)

AlejandroPerezMartin commented on PR #1379:


3 years ago
#10

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">

ryelle commented on PR #1379:


3 years ago
#11

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
3 years ago

  • Keywords commit added; needs-testing removed

The PR looks good to commit now.

#13 @ryelle
3 years 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.