Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#49435 closed defect (bug) (fixed)

Twenty Twenty: inconsistent top and bottom margins for .alignwide and .alignfull on Chrome vs Safari (cross browser issue)

Reported by: kthmd's profile kthmd Owned by: ianbelanger's profile ianbelanger
Milestone: 5.4.2 Priority: normal
Severity: normal Version: 5.3
Component: Bundled Theme Keywords: good-first-bug has-patch fixed-major
Focuses: css Cc:

Description

I've noticed on the latest WordPress and twentytwenty theme that there is an inconsistency in top and bottom margins, notably Chrome ignores lines style.css:5296 and style.css:5942 - so Chrome does not have the margins, but Safari does. This seems to be quite a large layout issue cross browser.

For example, use default twentytwenty theme, go to sample page and edit, create a cover block, set to fullwidth, and notice the top and bottom margin difference in Chrome (v80) and Safari (v13)

style.css:5296

	.entry-content .alignwide:not(.wp-block-group.has-background),
	.entry-content .alignfull:not(.wp-block-group.has-background) {
		margin-bottom: 6rem;
		margin-top: 6rem;
	}

style.css:5942

	.entry-content > .alignwide:not(.wp-block-group.has-background),
	.entry-content > .alignfull:not(.wp-block-group.has-background) {
		margin-bottom: 8rem;
		margin-top: 8rem;
	}

Attachments (6)

chrome.png (3.7 MB) - added by kthmd 4 years ago.
Chrome
safari.png (3.7 MB) - added by kthmd 4 years ago.
Safari
Firefox.png (1.2 MB) - added by utz119 4 years ago.
Firefox
49435.diff (934 bytes) - added by nikhilbhansi 4 years ago.
49435.2.diff (973 bytes) - added by nikhilbhansi 4 years ago.
49435.3.diff (1.6 KB) - added by ianbelanger 4 years ago.
Updates patch with RTL styles

Change History (21)

@kthmd
4 years ago

Chrome

@kthmd
4 years ago

Safari

#1 @SergeyBiryukov
4 years ago

  • Component changed from Themes to Bundled Theme
  • Summary changed from twentytwenty theme: inconsistent top and bottom margins for .alignwide and .alignfull on Chrome vs Safari (cross browser issue) to Twenty Twenty: inconsistent top and bottom margins for .alignwide and .alignfull on Chrome vs Safari (cross browser issue)

#2 @ianbelanger
4 years ago

  • Keywords good-first-bug needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Version changed from 5.3.2 to 5.3

#3 @utz119
4 years ago

I can confirm kthmd that style.css:5296 and style.css:5942 are not firing on Chrome. Firefox also has the same issue.
It seems the problem is when @media (min-width: 1000px) or @media (min-width: 700px).

@utz119
4 years ago

Firefox

@nikhilbhansi
4 years ago

#4 @nikhilbhansi
4 years ago

  • Keywords has-patch added; needs-patch removed

The cover block was looking good in between the post but not on the top because of the 8rem padding of div.post-inner element and an additional margin of the cover block.
Solved the issue by giving margin-top: 0; when it will be on the top of the post, also taken care of bottom margin.

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


4 years ago

#6 @SergeyBiryukov
4 years ago

  • Milestone changed from Future Release to 5.5

#7 @ianbelanger
4 years ago

  • Keywords needs-refresh added

Thanks for your patch @nikhilbhansi, however I would prefer that we avoid using !important if possible. Could yo see if there is a solution that does not need the !important flag.

@nikhilbhansi
4 years ago

#8 @nikhilbhansi
4 years ago

@ianbelanger avoided !important by adding specificity.
Cover Blocks which are in the middle of posts does not have any negative effects.

#9 @ianbelanger
4 years ago

  • Keywords needs-refresh removed
  • Owner set to ianbelanger
  • Status changed from new to reviewing

Thanks @nikhilbhansi for the updated patch. I am reviewing now and if all goes well I will commit it.

Last edited 4 years ago by ianbelanger (previous) (diff)

@ianbelanger
4 years ago

Updates patch with RTL styles

#10 @ianbelanger
4 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 47820:

Bundled Themes: Twenty Twenty inconsistent top and bottom margins for .alignwide and .alignfull on Chrome vs Safari.

Fixes the inconsistent margins for alignwide and alignfull classes used on Cover Block when it is the first block on a page.

Props kthmd, utz119, nikhilbhansi.
Fixes #49435.

#11 @ianbelanger
4 years ago

  • Keywords fixed-major added
  • Milestone changed from 5.5 to 5.4.2
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport.

#12 @whyisjake
4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 47823:

Bundled Themes: Twenty Twenty inconsistent top and bottom margins for .alignwide and .alignfull on Chrome vs Safari.

Fixes the inconsistent margins for alignwide and alignfull classes used on Cover Block when it is the first block on a page.

Brings the changes from [47820] to the 5.4 branch.

Props kthmd, utz119, nikhilbhansi.
Fixes #49435.

#13 @sabernhardt
4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

It seems the RTL stylesheet was missed in [47820] (and [47823]).

#14 @whyisjake
4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 47846:

Bundled Themes: Add missing RTL styles for to fix inconsistent margins on .alignwide and .alignfull items.

Fixes the inconsistent margins for alignwide and alignfull classes used on Cover Block when it is the first block on a page.

Props kthmd, utz119, nikhilbhansi, and sabernhardt.
Fixes #49435.

#15 @whyisjake
4 years ago

In 47847:

Bundled Themes: Add missing RTL styles for to fix inconsistent margins on .alignwide and .alignfull items.

Fixes the inconsistent margins for alignwide and alignfull classes used on Cover Block when it is the first block on a page.

This commit brings the changes from [47846] to the 5.4 branch.

Props kthmd, utz119, nikhilbhansi, and sabernhardt.
Fixes #49435.

Note: See TracTickets for help on using tickets.