Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#54251 closed defect (bug) (reported-upstream)

Twenty Twenty: Vertical buttons spacing on frontend dependent on margins

Reported by: stacimc's profile stacimc Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.9
Component: Bundled Theme Keywords: has-patch needs-testing has-testing-info
Focuses: Cc:

Description

The vertical spacing of buttons in Twenty Twenty relies on overriding margins. This is not compatible with efforts to refactor the Buttons block to use gap to control vertical spacing in between buttons, rather than using margins https://github.com/WordPress/gutenberg/pull/34546.

.wp-block-button:not(.alignleft):not(.alignright) {
	margin-bottom: 30px;
	margin-top: 30px;
}

.wp-block-button {
	margin: 3rem 0;
}

These styles will break with the linked gap refactor; the margins are overridden and the vertical spacing between buttons shrinks considerably.

Proposed Solution: Remove the margin-based styling in Twenty Twenty and instead add a rule increasing the size of the row-gap.

Attachments (3)

Screen Shot 2021-10-12 at 4.07.40 PM.png (29.4 KB) - added by stacimc 2 years ago.
Before: Vertical spacing of buttons before refactoring the block to use gap
Screen Shot 2021-10-12 at 4.07.51 PM.png (26.0 KB) - added by stacimc 2 years ago.
After: Significantly reduced vertical spacing when the block is refactored to use gap
54251.patch (527 bytes) - added by umesh84 2 years ago.
extra margin remove from style.css

Download all attachments as: .zip

Change History (15)

@stacimc
2 years ago

Before: Vertical spacing of buttons before refactoring the block to use gap

@stacimc
2 years ago

After: Significantly reduced vertical spacing when the block is refactored to use gap

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


2 years ago

#2 @stacimc
2 years ago

The Buttons gap PR has been merged so this can be tested on trunk. To test:

  1. Use the Twenty Twenty theme
  2. Insert a Buttons block into a new post and add several buttons, enough so that they wrap onto multiple rows.
  3. View the post on the frontend.
  4. Observe that the row-gap between buttons has decreased considerably. If you inspect the buttons, you will see that this is because the margins applied by the theme styles are overridden in favor of the new gap-based spacing.

@umesh84
2 years ago

extra margin remove from style.css

#3 @umesh84
2 years ago

  • Keywords has-patch added
  • Resolution set to invalid
  • Status changed from new to closed

#4 @sabernhardt
2 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

#5 @audrasjb
2 years ago

  • Milestone changed from Awaiting Review to 5.9

Moving to milestone 5.9 as there is indeed an issue introduced in 5.9.

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


2 years ago

#7 @audrasjb
2 years ago

  • Milestone changed from 5.9 to 5.9.1

Moving to 5.9.1 as WP 5.9 RC 1 is today and this still needs broader testing.

#8 @Boniu91
2 years ago

  • Keywords needs-testing has-testing-info added

#9 @hellofromTonya
2 years ago

  • Milestone changed from 5.9.1 to 5.9.2

5.9.1 is happening in about 30 minutes. As broader testing activity hasn't yet happened since being put in the milestone, moving to 5.9.2 to give it more time.

#10 @ironprogrammer
2 years ago

tl;dr
I'd like to propose that this ticket be closed, as the concern was resolved with @stacimc's PR 34546.

Discovery

A few days after this ticket was originally created, PR 34546 introduced a highly-specific override that prevents the reported margins in "Twenty Twenty" from taking effect, resetting them and resulting in:

.wp-block-buttons > .wp-block-button.wp-block-button.wp-block-button.wp-block-button.wp-block-button {
    margin: 0;
}

The above ensures that the gap setting of the wrapping .wp-block-buttons controls spacing, which was the original concern voiced in this ticket 👍🏻.

Screenshots

Here are current screenshots of the editor and front end (WordPress 6.0-alpha-52448-src), which render consistently. Buttons within the same block adhere to gap: 0.5em set by the editor.

Additionally, a new row with a button block has been added to show that spacing between button blocks is not being set by the theme, but defaulting to editor classic styles.

https://cldup.com/g9_vvCIR5g.png
Consecutive Buttons blocks rendered in post editor.

https://cldup.com/09D3FTdXJl.png
Consecutive Buttons blocks rendered on front end post page.

#11 @nidhidhandhukiya
2 years ago

I Think this issue is resolved with the latest update 5.9.1
Because i'm checking it in my local and this is not happen.

#12 @audrasjb
2 years ago

  • Milestone 5.9.2 deleted
  • Resolution set to reported-upstream
  • Status changed from reopened to closed

Let's close this ticket as reported-upstream, as it was fixed in https://github.com/WordPress/gutenberg/pull/34546

Note: See TracTickets for help on using tickets.