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

Jump to bottom

Try: Restore block mover for floats. #12758

Merged
merged 3 commits into from
Feb 6, 2019
Merged

Try: Restore block mover for floats. #12758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d7f155e
Try: Restore block mover for floats.
Dec 10, 2018
f45ae27
Address feedback, and improve dragging edgecase.
Jan 30, 2019
bf6d1f3
Add borders around floats.
Feb 6, 2019
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
8 changes: 5 additions & 3 deletions assets/stylesheets/_z-index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
$z-layers: (
".editor-block-list__block-edit::before": 0,
".editor-block-switcher__arrow": 1,
".editor-block-list__block {core/image aligned left or right}": 20,
".editor-block-list__block {core/image aligned wide or fullwide}": 20,
".block-library-classic__toolbar": 10,
".editor-block-list__layout .reusable-block-indicator": 1,
Expand Down Expand Up @@ -48,9 +47,12 @@ $z-layers: (
".components-drop-zone": 100,
".components-drop-zone__content": 110,

// Block controls, particularly in nested contexts, floats aside block and
// The block mover, particularly in nested contexts,
// should overlap most block content.
".editor-block-list__block.is-{selected,hovered} .editor-block-{settings-menu,mover}": 80,
".editor-block-list__block.is-{selected,hovered} .editor-block-mover": 80,

// The block mover for floats should overlap the controls of adjacent blocks.
".editor-block-list__block {core/image aligned left or right}": 81,

// Small screen inner blocks overlay must be displayed above drop zone,
// settings menu, and movers.
Expand Down
30 changes: 15 additions & 15 deletions packages/editor/src/components/block-list/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,25 +519,25 @@ export class BlockListBlock extends Component {
clientId={ clientId }
rootClientId={ rootClientId }
/>
{ shouldRenderMovers && (
<BlockMover
clientIds={ clientId }
blockElementId={ blockElementId }
isFirst={ isFirst }
isLast={ isLast }
isHidden={ ! ( isHovered || isSelected ) || hoverArea !== 'left' }
isDraggable={
isDraggable !== false &&
( ! isPartOfMultiSelection && isMovable )
}
onDragStart={ this.onDragStart }
onDragEnd={ this.onDragEnd }
/>
) }
{ isFirstMultiSelected && (
<BlockMultiControls rootClientId={ rootClientId } />
) }
<div className="editor-block-list__block-edit">
{ shouldRenderMovers && (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the block mover to be able to work for a right-floated block, it has to be inside .editor-block-list__block-edit, or the control will show up way on the left of the main column. Same case as when we refactored the floats, essentially.

<BlockMover
clientIds={ clientId }
blockElementId={ blockElementId }
isFirst={ isFirst }
isLast={ isLast }
isHidden={ ! ( isHovered || isSelected ) || hoverArea !== 'left' }
isDraggable={
isDraggable !== false &&
( ! isPartOfMultiSelection && isMovable )
}
onDragStart={ this.onDragStart }
onDragEnd={ this.onDragEnd }
/>
) }
{ shouldShowBreadcrumb && (
<BlockBreadcrumb
clientId={ clientId }
Expand Down
74 changes: 53 additions & 21 deletions packages/editor/src/components/block-list/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
.editor-block-list__layout .components-draggable__clone {
// Hide the Block UI when dragging the block
// This ensures the page scroll properly (no sticky elements)
// Hide the Block UI when dragging the block.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a few comment cleanups while I'm in here.

// This ensures the page scroll properly (no sticky elements).
.editor-block-contextual-toolbar {
// I think important is fine here to avoid over complexing the selector
// It's probably okay to use !important here to avoid over-complicating the selector.
display: none !important;
}
}
Expand Down Expand Up @@ -127,7 +127,7 @@
}
}

// Selected style
// Selected style.
&.is-selected > .editor-block-list__block-edit::before {
// Use opacity to work in various editor styles.
outline: $border-width solid $dark-opacity-light-500;
Expand All @@ -137,12 +137,12 @@
}
}

// Hover style
// Hover style.
&.is-hovered > .editor-block-list__block-edit::before {
outline: $border-width solid theme(outlines);
}

// Spotlight mode
// Spotlight mode.
&.is-focus-mode:not(.is-multi-selected) {
opacity: 0.5;
transition: opacity 0.1s linear;
Expand All @@ -168,7 +168,7 @@
background-color: $blue-medium-highlight;
}

// selection style for multiple blocks
// Selection style for multiple blocks.
&.is-multi-selected *::selection {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
&.is-multi-selected *::selection {
&.is-multi-selected ::selection {

I think that asterisk is redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good thought, but I'd prefer not actually making this change in this PR. It is already sort of big surgery and I'd like to minimize the changes as much as possible. Given that code did not change in this PR, only the comment, I'd prefer to look at this separately.

background-color: transparent;
}
Expand All @@ -179,7 +179,7 @@
// Use opacity to work in various editor styles.
mix-blend-mode: multiply;

// Collapse extra vertical padding on selection
// Collapse extra vertical padding on selection.
top: -$block-padding;
bottom: -$block-padding;

Expand Down Expand Up @@ -293,12 +293,6 @@
margin-bottom: $border-width;
}

// Hide all additional UI on floats.
.editor-block-mover,
.editor-block-list__block-mobile-toolbar {
display: none;
}

// Position toolbar better on mobile.
.editor-block-contextual-toolbar {
width: auto;
Expand Down Expand Up @@ -506,7 +500,8 @@
.editor-block-list__block {

// Left and right block settings and mover.
> .editor-block-mover {
&.is-multi-selected > .editor-block-mover,
> .editor-block-list__block-edit > .editor-block-mover {
position: absolute;
width: $block-side-ui-width + $block-side-ui-clearance;

Expand All @@ -516,7 +511,8 @@
}

// Position depending on whether selected or not.
> .editor-block-mover {
&.is-multi-selected > .editor-block-mover,
> .editor-block-list__block-edit > .editor-block-mover {
top: -$block-padding - $border-width;
}

Expand All @@ -526,24 +522,60 @@
&.is-selected,
&.is-hovered {
.editor-block-mover {
z-index: z-index(".editor-block-list__block.is-{selected,hovered} .editor-block-{settings-menu,mover}");
z-index: z-index(".editor-block-list__block.is-{selected,hovered} .editor-block-mover");
}
}
}

// Left side UI.
> .editor-block-mover {
&.is-multi-selected > .editor-block-mover,
> .editor-block-list__block-edit > .editor-block-mover {
padding-right: $block-side-ui-clearance;

// Position for top level blocks
left: -$block-side-ui-width - $block-side-ui-clearance;
// Position for top level blocks.
left: -$block-side-ui-width - $block-side-ui-clearance - $block-padding - $border-width;

// Mobile
// Hide on mobile, as mobile has a separate solution.
display: none;
@include break-small() {
display: block;
}
}

&.is-multi-selected > .editor-block-mover {
left: -$block-side-ui-width - $block-side-ui-clearance;
}

// For floats, show block mover when block is selected, and never on hover.
&[data-align="left"],
&[data-align="right"] {
// Show always when the block is selected.
&.is-selected > .editor-block-list__block-edit > .editor-block-mover {
// Don't show on mobile, allow the special mobile toolbar to work there.
display: none;
@include break-small() {
display: block;
opacity: 1;
animation: none;

// Make wider and taller to make "safe" hover area bigger.
// The intent is to make it less likely that you hover float-adjacent
// blocks that visually appear below the block.
width: $block-side-ui-width + $block-side-ui-clearance + $block-padding + $border-width;
height: auto;
padding-bottom: $block-padding;

// Unset the negative top margin, or it might overlap the block toolbar.
margin-top: 0;
}
}

// Don't show on hover, or on the "ghost" when dragging.
&.is-hovered > .editor-block-list__block-edit > .editor-block-mover,
&.is-dragging > .editor-block-list__block-edit > .editor-block-mover {
display: none;
}
}
}


Expand Down
2 changes: 2 additions & 0 deletions packages/editor/src/components/block-mover/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@
.editor-block-mover__control-drag-handle:not(:disabled):not([aria-disabled="true"]):not(.is-default),
.editor-block-mover__control {
@include break-small() {
.editor-block-list__layout [data-align="right"] &,
.editor-block-list__layout [data-align="left"] &,
.editor-block-list__layout .editor-block-list__layout & {
background: $white;
box-shadow: inset 0 0 0 1px $light-gray-500;
Expand Down