Opened 3 years ago
Closed 3 years ago
#54250 closed defect (bug) (fixed)
Twenty Twenty One: Editor Buttons margins incompatible with gap
Reported by: | stacimc | Owned by: | audrasjb |
---|---|---|---|
Milestone: | 5.9.1 | Priority: | normal |
Severity: | normal | Version: | 5.9 |
Component: | Bundled Theme | Keywords: | has-testing-info has-screenshots has-patch commit fixed-major |
Focuses: | Cc: |
Description
Twenty Twenty One has a few style rules in the editor that will causes issues when the Buttons block is refactored to use gap
to control vertical spacing in between buttons, rather than using margins. (https://github.com/WordPress/gutenberg/pull/34546)
It enforces a 0px margin on the Buttons container, and applies additional margin to the first and last children of the Buttons block. With margins removed from individual buttons, this causes:
- Insufficient space between Buttons container and adjacent blocks
- The first button in a Buttons group is vertically offset from its siblings
[data-block].wp-block-buttons { margin-top: 0; margin-bottom: 0; } [data-block].wp-block-buttons .wp-block-button:first-child { margin-top: var(--global--spacing-vertical); } [data-block].wp-block-buttons .wp-block-button:last-child { margin-bottom: var(--global--spacing-vertical); }
Proposed Solution:
- Remove the rule setting 0px margin on the container, allowing it to fall back to default spacing
- Remove the rules setting extra margin-top/bottom on first/last buttons as it is no longer necessary
Attachments (2)
Change History (24)
#1
@
3 years ago
- Keywords needs-testing-info added
Hello @stacimc thank you for creating the ticket.
Could you provide us with the exact step by step instructions that are used to reproduce the issue?
It'll be easier for the Test team to follow exactly the same steps as you do.
This ticket was mentioned in Slack in #core-test by boniu91. View the logs.
3 years ago
#3
@
3 years ago
Steps to reproduce and test
- Use the twentytwentyone theme.
- In
style.css
, add this:Check and verify that the image is everywhere.div { background-image: url("./assets/images/Daffodils.jpg") !important; }
- In
functions.php
add the following:add_filter( 'should_load_separate_core_block_assets', '__return_true' ); add_filter( 'styles_inline_size_limit', function( $size ) { return 1000000; // Extreme, but this is just for the purpose of a test. }); add_action( 'wp_enqueue_scripts', function() { wp_style_add_data( 'twenty-twenty-one-style', 'path', get_theme_file_path( 'style.css' ) ); }, 20 );
Without the patch: the image no longer shows as the background.
With the patch: the image should show.
#5
@
3 years ago
Since writing this ticket, the Buttons gap support was merged (https://github.com/WordPress/gutenberg/pull/34546). It refactors Buttons to use gap
for vertical spacing rather than margins, and forces all margins on Button blocks to 0
. This causes the described regressions to vertical spacing.
To test:
- Use the Twenty Twenty One theme
- Insert a Buttons block into a new post, and add several buttons to it, enough that they take up multiple rows.
- Insert a second Buttons block immediately afterward, and add several buttons to it.
- Observe that there is no margin in between the two Buttons containers.
- Inspect the first Button in either group and observe this CSS rule:
[data-block].wp-block-buttons .wp-block-button:first-child { margin-top: var(--global--spacing-vertical); }
The first-child (and corresponding last-child) rule do not cause a visual bug because they are overridden by the block, but these styles could be deleted. The lack of margin between Buttons containers does cause a visual issue.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
3 years ago
#8
@
3 years ago
- Keywords needs-patch added
- 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 a patch and broader testing.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
3 years ago
#10
@
3 years ago
- Keywords needs-testing added
Now that 5.9 was released, the next step is to see if it’s still possible to reproduce the issue, and to provide a few screenshots/steps to help reproduce the issue.
This ticket was mentioned in Slack in #core by ironprogrammer. View the logs.
3 years ago
#12
@
3 years ago
- Keywords has-screenshots added
I was able to reproduce this issue with these updated instructions.
tl;dr -- What to Check When Reproducing the Issue
It's important that the Buttons blocks be vertically adjacent to one another. To verify this, open List View to check the nesting of the Buttons blocks. If the blocks are separated by any other block type, or within different Groups, use the List View to drag the Buttons blocks so that they are adjacent. Refer to the screenshots below for comparison.
⚠️ Note that the Gap setting this ticket originally refers to appears to have been removed from 5.9.
To reproduce:
- Install WordPress 5.9.
- Activate Twenty Twenty-One theme.
- Add a new Post.
- Enter a title and press
Return
, or just advance to the first content block location (pressTab
or place the cursor in the "Type / to choose a block" field). - Type
/buttons
+Return
to insert a Buttons block, or use the "plus" button to the right of the insert block area and select "Buttons". - Insert multiple buttons into this block, for instance by typing "button" +
Return
repeatedly. Insert enough buttons that they wrap into the next line. - Use the
Down
cursor to advance to the next block insertion area, and add another Buttons block by typing/buttons
+Return
. Alternatively, click the "plus" button to the right and below the current block, and select another Buttons block. - Insert a few buttons into this block.
- Note that there is no margin between the first Buttons block and the one immediately below it.
Screenshots
WordPress 5.9 (Repro)
WordPress 5.8.3 (Regression)
@
3 years ago
This issue is happen only on the editor side it is working fine in user side. I have the solution of this issue.
This ticket was mentioned in PR #2268 on WordPress/wordpress-develop by ironprogrammer.
3 years ago
#13
- Keywords has-patch added; needs-patch removed
Removes overrides that prevent site editor updates in 5.9 from controlling vertical margins via var(--global--spacing-vertical)
.
Trac ticket: https://core.trac.wordpress.org/ticket/54250
#14
@
3 years ago
Thank you, @nidhidhandhukiya, for pointing out the margin.
Backtracking from that selector group reveals that the spacing discrepancy relates to margins between Buttons blocks themselves, rather than for individual buttons. I've opened PR 2268 to address this in the Twenty Twenty-One (TT1) theme.
This update makes TT1 behave like previous non-FSE themes, where control over editor block margins is controlled by bundled editor styles. The fix falls in line with @stacimc's proposed solution, though is technically separate from blockGap
support. Visually, the margins are consistent with legacy themes like Twenty Twenty, Twenty Nineteen, etc.
Twenty Twenty-One with Fix Applied
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
3 years ago
This ticket was mentioned in Slack in #core-test by boniu91. View the logs.
3 years ago
#17
@
3 years ago
Working as expected for me.
Test Report
Env
- WordPress 5.9
- Chrome 98.0.4758.82
- Windows 10
- Theme: Twenty Twenty One
Steps to test
- Add more than 5 buttons to one block
- Add more than 5 buttons to the block below the first one
- The view in the block editor is exactly the same as the view on the frontend
- Checked different screen sizes
Screenshot:
https://screenrec.com/share/Ev9ULJFSK8
#18
@
3 years ago
- Keywords commit added; needs-testing removed
- Owner set to audrasjb
- Status changed from new to assigned
Thanks for testing!
Self-assigning for commit
.
3 years ago
#20
Committed in https://core.trac.wordpress.org/changeset/52726
Bug: There is no margin in between Buttons containers, and the first button in each group is vertically offset