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.

Sign up for GitHub

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

Try: Refactor appender margin. #33088

Merged
merged 3 commits into from
Jul 1, 2021
Merged

Try: Refactor appender margin. #33088

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
86364cc
Try: Refactor appender margin.
jasmussen Jun 30, 2021
d099fe3
Expand comment.
jasmussen Jun 30, 2021
64c2cbb
Add clarifying comment.
jasmussen Jul 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions packages/block-editor/src/components/block-list-appender/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,35 @@
max-width: none;
}

// Add a little left margin when used horizontally.
// The right margin should be set to auto, so as to not shift layout in flex containers.
margin: 0 auto 0 $grid-unit-10;

// ... unless it's the only child.
&:first-child {
margin-left: 0;
}

.block-editor-default-block-appender {
margin: $grid-unit-10 0;
}

// Animate appearance.
// Add an explicit left margin of zero and auto right margin to work in horizontal
// flex containers. Without it, a "space-between"-like effect from two auto margins
// will cause the black plus to sit in the center of what space is left.
margin: 0 auto 0 0;

// Black square plus appender.
.block-list-appender__toggle {
padding: 0;

// Animate appearance.
opacity: 1;
transform: scale(1);
transition: all 0.1s ease;
@include reduce-motion("transition");

// The black square button should have a little left margin in horizontal containers.
margin-left: $grid-unit-10;
}

// Cancel any left margin if the black plus sits alone in the container.
// `first-of-type` is used instead of `first-child` as the element is not always the only
// element in the "empty" container. For example the empty navigation block state has a
// zero-width placeholder state that is meant to help correctly size the dimensions.
&:first-of-type .block-list-appender__toggle {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand this part, I might be missing some context. Why will it be the only child if it matches :first-of-type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent question, thanks for calling it out.

first-of-type is used because the appender is not the first-child in an otherwise "empty" navigation block:

Screenshot 2021-07-01 at 11 14 48

Shown here with zero width is an actual placeholder state for the navigation block. That placeholder state exists here but has zero with so that it can ensure the height of the navigation block is the same between the empty state, and when there are menu items inside, ensuring no layout shift when you insert.

Observe here in the top right corner that the margin-left: 0; rule is correctly applied to the appender when the first-of-type rule hits the target:

Screenshot 2021-07-01 at 11 17 31

Here, it's no longer the first-of-type, after a menu item has been inserted, and the 8px left margin applies:

Screenshot 2021-07-01 at 11 21 13

That same rule applies here as well:

Screenshot 2021-07-01 at 11 22 41

As noted in #33087 (comment), I think ultimately we need to remove the black plus from the editing canvas entirely, make it a popover that exists outside the canvas/iframe and is positioned in a way that doesn't cause layout shifts. That will be an opportunity to tear out all these complex rules.

In the mean time, I will add a comment to elaborate on why first-of-type is used. Sound good?

margin-left: 0;
}
}

Expand Down