Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cancel left margin when full width block appender used at root level of layout #33087

Closed
wants to merge 1 commit into from

Conversation

getdave
Copy link
Contributor

@getdave getdave commented Jun 30, 2021

Description

This PR ensures that when a full width block appended is used at the root level of the block list then it doesn't have any left margin applied to it.

It looks like we've been tinkering with the block appended margins quite a bit so it's obviously a thorny area.

This fixes #33085

How has this been tested?

I don't have a high degree of confidence in the test scenarios I'll need to run through but here's a start:

  • Activate TT1.
  • Go to Appearance -> Widgets
  • Look at the block list appender - check there is no additional left margin.
  • Also check full width appender that is used in the Group block.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@getdave getdave added [Type] Bug An existing feature does not function as intended [Feature] Widgets Screen The block-based screen that replaced widgets.php. CSS Styling Related to editor and front end styles, CSS-specific issues. labels Jun 30, 2021
@getdave getdave requested a review from ellatrix as a code owner June 30, 2021 08:54
@getdave getdave self-assigned this Jun 30, 2021
@getdave
Copy link
Contributor Author

getdave commented Jun 30, 2021

@jasmussen I'd really appreciate your 👀 on this one as I expect there are a number of test scenarios I'll need to run through to ensure this change doesn't introduce any regressions.

@jasmussen
Copy link
Contributor

Thanks for the PR.

I'm still investigating why the margin is there in the first place. It's also present, but cancelled out, in the block editor when inserting a Group block:

Screenshot 2021-06-30 at 10 59 33

Screenshot 2021-06-30 at 10 59 26

I feel like it's there for a reason, and if not, we can remove it entirely. But I'll dig a little deeper and get back to this one!

@github-actions

This comment has been minimized.

@jasmussen
Copy link
Contributor

Okay, so here's why the left margin is there:

Screenshot 2021-06-30 at 11 03 12

so it has to have an explicit left margin that isn't "auto", in order to work inside flex containers:

Screenshot 2021-06-30 at 11 03 52

But obviously when the big white plus button is used, that margin is useless.

Let me look at whether there's a better way to cancel out that margin. The thing is, I'd like to remove that in-canvas inserter entirely, in favor of something that's abs-positioned/popovered, for some of the reasons outlined in #31886. So I'd love to find CSS that doesn't assume that black plus will exist forever, instead of overriding it.

@getdave
Copy link
Contributor Author

getdave commented Jun 30, 2021

We really need a way to distinguish between the "plus" appended and the full width one.

@jasmussen
Copy link
Contributor

Yep, I'm looking into that now. block-list-appender__toggle is only used on the black plus, and block-editor-button-block-appender is the only remaining class on the white one.

I think there's an opportunity for a refactor, also in context of #33025, but let's find a small fix in the mean time. I shall return in a bit.

@jasmussen jasmussen mentioned this pull request Jun 30, 2021
7 tasks
@jasmussen
Copy link
Contributor

I have #33088 as an alternate fix.

@getdave
Copy link
Contributor Author

getdave commented Jun 30, 2021

Closing in favour of #33088

@getdave getdave closed this Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Feature] Widgets Screen The block-based screen that replaced widgets.php. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected margin left for block appender in widgets screen
2 participants