From 56d8735bd595939da45aff8d83f51852f6fbb2b5 Mon Sep 17 00:00:00 2001
From: Joen Asmussen <asmussen@gmail.com>
Date: Thu, 1 Nov 2018 10:34:22 +0100
Subject: [PATCH] Try: Refactor contextual toolbar to work better with floats

This is rather big surgery this late in the phase. There may be dragons, and it may be necessary to punt this to phase 2.

This PR seeks to fix #4764 by doing a number of things:

- Move the contextual toolbar into the block edit div, which is the one we float. This helps make it _connected_ to the content.
- Add some complex clearing rules so we avoid many of the gnarly situations where a selected block after a floated block has a weirdly tall size.
- Fixes so a paragraph block that follows a float does behave as it will on the frontend (i.e. won't clear), but also has a toolbar that is correctly positioned.

This moving around of things caused subsequent issues, which means this PR also:

- Fixes the toolbar appearance on mobile.
- Improves upon the appearance of the toolbar on floated items on mobile.
- Fixes hover label positioning, not only so they work with floats, but are positioned correctly as a result of this refactor.
- Fixes issues with wide and fullwide toolbar positioning.
---
 .../block-library/src/columns/editor.scss     |   9 +-
 .../src/components/visual-editor/style.scss   |  20 ++-
 .../editor/src/components/block-list/block.js |  14 +-
 .../src/components/block-list/style.scss      | 161 +++++++++++-------
 4 files changed, 128 insertions(+), 76 deletions(-)

diff --git a/packages/block-library/src/columns/editor.scss b/packages/block-library/src/columns/editor.scss
index ebe614483c4a5..40ba94cbdbc21 100644
--- a/packages/block-library/src/columns/editor.scss
+++ b/packages/block-library/src/columns/editor.scss
@@ -73,10 +73,10 @@
 			}
 
 			@include break-small() {
-				> .editor-block-contextual-toolbar {
+				> .editor-block-list__block-edit > .editor-block-contextual-toolbar {
 					top: $block-toolbar-height - $border-width;
 					transform: translateY(-$block-toolbar-height - $border-width);
-					margin-left: -$block-padding - $block-padding - $border-width;
+					margin-left: -$grid-size-large - $border-width;
 				}
 
 				> .editor-block-list__block-edit::before {
@@ -86,8 +86,9 @@
 					left: 0;
 				}
 
-				> .editor-block-list__breadcrumb {
-					margin-right: -$block-padding - $border-width;
+				> .editor-block-list__block-edit > .editor-block-list__breadcrumb {
+					top: 0;
+					right: 0;
 				}
 			}
 
diff --git a/packages/edit-post/src/components/visual-editor/style.scss b/packages/edit-post/src/components/visual-editor/style.scss
index 2795f5330ba36..0463c005479ef 100644
--- a/packages/edit-post/src/components/visual-editor/style.scss
+++ b/packages/edit-post/src/components/visual-editor/style.scss
@@ -29,9 +29,9 @@
 
 		// Center the block toolbar on wide and full-wide blocks.
 		// Use specific selector to not affect nested block toolbars.
-		&[data-align="wide"] > .editor-block-contextual-toolbar,
-		&[data-align="full"] > .editor-block-contextual-toolbar {
-			width: calc(100% + #{ $block-side-ui-width * 2 + $block-padding * 2 + $border-width * 2 }); // Matches the negative margins applied to parent blocks.
+		&[data-align="wide"] > .editor-block-list__block-edit > .editor-block-contextual-toolbar,
+		&[data-align="full"] > .editor-block-list__block-edit > .editor-block-contextual-toolbar {
+			width: calc(100% + #{ $block-padding * 2 + $border-width * 2 }); // Matches the negative margins applied to parent blocks.
 			height: 0; // This collapses the container to an invisible element without margin.
 			text-align: center;
 
@@ -42,9 +42,11 @@
 			}
 		}
 
-		// The centering math changes when a fullwide image doesn't have block padding.
-		&[data-align="full"] > .editor-block-contextual-toolbar {
-			width: calc(100% + #{ $block-side-ui-width * 2 + $block-padding * 2 }); // Matches the negative margins applied to non-parent blocks, except for borders which are gone in fullwide.
+		// The centering math is simpler for a fullwide block, which doesn't have any block padding.
+		&[data-align="full"] > .editor-block-list__block-edit > .editor-block-contextual-toolbar {
+			width: 100%;
+			margin-left: 0;
+			margin-right: 0;
 
 			.editor-block-toolbar {
 				max-width: $content-width - $border-width - $border-width;
@@ -84,6 +86,12 @@
 }
 
 .edit-post-visual-editor {
+	// If the first block is floated, it needs top margin, unlike the rule in line 69.
+	.editor-block-list__layout > .editor-block-list__block[data-align="left"]:first-child,
+	.editor-block-list__layout > .editor-block-list__block[data-align="right"]:first-child {
+		margin-top: $block-padding + $block-spacing + $border-width + $border-width + $block-padding;
+	}
+
 	.editor-default-block-appender {
 		// Default to centered and content-width, like blocks
 		margin-left: auto;
diff --git a/packages/editor/src/components/block-list/block.js b/packages/editor/src/components/block-list/block.js
index 5858f8731fb7a..c33bcc7129c5c 100644
--- a/packages/editor/src/components/block-list/block.js
+++ b/packages/editor/src/components/block-list/block.js
@@ -519,13 +519,6 @@ export class BlockListBlock extends Component {
 						onDragEnd={ this.onDragEnd }
 					/>
 				) }
-				{ shouldShowBreadcrumb && (
-					<BlockBreadcrumb
-						clientId={ clientId }
-						isHidden={ ! ( isHovered || isSelected ) || hoverArea !== 'left' }
-					/>
-				) }
-				{ shouldShowContextualToolbar && <BlockContextualToolbar /> }
 				{ isFirstMultiSelected && (
 					<BlockMultiControls rootClientId={ rootClientId } />
 				) }
@@ -536,6 +529,13 @@ export class BlockListBlock extends Component {
 					className="editor-block-list__block-edit"
 					data-block={ clientId }
 				>
+					{ shouldShowBreadcrumb && (
+						<BlockBreadcrumb
+							clientId={ clientId }
+							isHidden={ ! ( isHovered || isSelected ) || hoverArea !== 'left' }
+						/>
+					) }
+					{ shouldShowContextualToolbar && <BlockContextualToolbar /> }
 					<BlockCrashBoundary onError={ this.onBlockError }>
 						{ isValid && blockEdit }
 						{ isValid && mode === 'html' && (
diff --git a/packages/editor/src/components/block-list/style.scss b/packages/editor/src/components/block-list/style.scss
index c66a202741175..ca541718cd239 100644
--- a/packages/editor/src/components/block-list/style.scss
+++ b/packages/editor/src/components/block-list/style.scss
@@ -300,13 +300,23 @@
 
 		// Position toolbar better on mobile.
 		.editor-block-contextual-toolbar {
+			//position: relative;
+			width: auto;
 			border-bottom: 1px solid $light-gray-500;
-			bottom: $block-padding;
-			left: auto;
-			right: auto;
+			bottom: auto;
 		}
 	}
 
+	&[data-align="left"] .editor-block-contextual-toolbar {
+		left: 0;
+		right: auto;
+	}
+
+	&[data-align="right"] .editor-block-contextual-toolbar {
+		left: auto;
+		right: 0;
+	}
+
 	// Left
 	&[data-align="left"] {
 		// This is in the editor only; the image should be floated on the frontend.
@@ -412,8 +422,8 @@
 	// Full-wide
 	&[data-align="full"] {
 		// Position hover label on the right
-		> .editor-block-list__breadcrumb {
-			right: -$border-width;
+		> .editor-block-list__block-edit .editor-block-list__breadcrumb {
+			right: 0;
 		}
 
 		// Compensate for main container padding and subtract border.
@@ -490,7 +500,8 @@
 	> .editor-block-mover {
 		position: absolute;
 		width: $block-side-ui-width + $block-side-ui-clearance;
-		// stretch to fill half of the available space to increase hoverable area
+
+		// Stretch to fill half of the available space to increase hoverable area.
 		height: 100%;
 		max-height: $block-side-ui-width * 4;
 	}
@@ -511,7 +522,7 @@
 		}
 	}
 
-	// Left side UI
+	// Left side UI.
 	> .editor-block-mover {
 		padding-right: $block-side-ui-clearance;
 
@@ -729,58 +740,72 @@
  * Block Toolbar when contextual.
  */
 
-.editor-block-list__block .editor-block-contextual-toolbar {
-	position: sticky;
-	z-index: z-index(".editor-block-contextual-toolbar");
-	white-space: nowrap;
-	text-align: left;
-	pointer-events: none;
-
-	// Position toolbar below the block on mobile.
-	position: absolute;
-	bottom: $block-toolbar-height - $block-padding - 1px;
-	left: 0;
-	right: 0;
+.editor-block-list__block {
+	.editor-block-contextual-toolbar {
+		position: sticky;
+		z-index: z-index(".editor-block-contextual-toolbar");
+		white-space: nowrap;
+		text-align: left;
+		pointer-events: none;
 
-	// Paint the borders on the toolbar itself on mobile.
-	border-top: $border-width solid $light-gray-500;
-	.components-toolbar {
-		border-top: none;
-		border-bottom: none;
-	}
+		// Position toolbar below the block on mobile.
+		position: absolute;
+		bottom: $block-toolbar-height - $block-padding - 1px;
+		left: -$block-padding;
+		right: -$block-padding;
 
-	@include break-small() {
-		border-top: none;
+		// Paint the borders on the toolbar itself on mobile.
+		border-top: $border-width solid $light-gray-500;
 		.components-toolbar {
-			border-top: $border-width solid $light-gray-500;
-			border-bottom: $border-width solid $light-gray-500;
+			border-top: none;
+			border-bottom: none;
+		}
+
+		@include break-small() {
+			border-top: none;
+			.components-toolbar {
+				border-top: $border-width solid $light-gray-500;
+				border-bottom: $border-width solid $light-gray-500;
+			}
 		}
 	}
 
 	// Floated items have special needs for the contextual toolbar position.
-	.editor-block-list__block[data-align="left"] &,
-	.editor-block-list__block[data-align="right"] & {
+	&[data-align="left"] .editor-block-contextual-toolbar,
+	&[data-align="right"] .editor-block-contextual-toolbar {
 		margin-bottom: $border-width;
 		margin-top: -$block-toolbar-height - $border-width;
 	}
 
 	// Make block toolbar full width on mobile.
-	margin-left: 0;
-	margin-right: 0;
+	.editor-block-contextual-toolbar {
+		margin-left: 0;
+		margin-right: 0;
+		@include break-small() {
+			margin-left: -$block-padding - $border-width;
+			margin-right: -$block-padding - $border-width;
+		}
+	}
 
-	@include break-small() {
-		margin-left: -$block-side-ui-width - $block-padding - $border-width;
-		margin-right: -$block-side-ui-width - $block-padding - $border-width;
+	// For floats, compensate for this so content doesn't grow smaller.
+	&[data-align="left"] .editor-block-contextual-toolbar {
+		/*rtl:ignore*/
+		margin-right: $block-padding + $border-width;
+	}
 
-		// Except for wide elements, this causes a horizontal scrollbar.
-		[data-align="full"] & {
-			margin-left: -$block-padding - $block-side-ui-width;
-			margin-right: -$block-padding - $block-side-ui-width;
-		}
+	&[data-align="right"] .editor-block-contextual-toolbar {
+		/*rtl:ignore*/
+		margin-left: $block-padding + $border-width;
+	}
+
+	// Don't do it for wide elements, this causes a horizontal scrollbar.
+	&[data-align="full"] .editor-block-contextual-toolbar {
+		margin-left: -$block-padding - $block-side-ui-width;
+		margin-right: -$block-padding - $block-side-ui-width;
 	}
 
 	// Reset pointer-events on children.
-	& > * {
+	.editor-block-contextual-toolbar > * {
 		pointer-events: auto;
 	}
 }
@@ -820,19 +845,21 @@
 	}
 
 	.editor-block-list__block[data-align="left"] & {
-		@include break-small() {
-			// RTL note: this rule should not be auto-flipped based on direction.
-			/*rtl:ignore*/
-			float: left;
-		}
+		// RTL note: this rule should not be auto-flipped based on direction.
+		/*rtl:ignore*/
+		float: left;
 	}
 
+	.editor-block-list__block[data-align="left"] &,
 	.editor-block-list__block[data-align="right"] & {
-		@include break-small() {
-			// RTL note: this rule should not be auto-flipped based on direction.
-			/*rtl:ignore*/
-			float: right;
-		}
+		// Move the block toolbar out of the flow using translate, but less for floats.
+		transform: translateY(-$block-padding -$border-width);
+	}
+
+	.editor-block-list__block[data-align="right"] & {
+		// RTL note: this rule should not be auto-flipped based on direction.
+		/*rtl:ignore*/
+		float: right;
 	}
 }
 
@@ -840,19 +867,28 @@
 .editor-block-contextual-toolbar .editor-block-toolbar {
 	width: 100%;
 
-	// Hide right border on desktop, where the .components-toolbar instead has a right border.
 	@include break-small() {
+		width: auto;
+
+		// Hide right border on desktop, where the .components-toolbar instead has a right border.
 		border-right: none;
-	}
 
-	// This prevents floats from messing up the position.
-	@include break-small() {
+		// This prevents floats from messing up the position.
 		position: absolute;
 		left: 0;
 	}
+}
 
-	@include break-small() {
-		width: auto;
+// This rule ensures that any blocks that are not a Paragraph, that follows any aligned block, clears them.
+// This is necessary to ensure the selected block outlines and toolbar are correctly positioned.
+.editor-block-list__block[data-align] + .editor-block-list__block:not([data-align="right"]):not([data-type="core/paragraph"]) {
+	clear: both;
+}
+
+// This rule ensures that Paragraphs, which do not clear preceeding floats, have a correctly aligned contextual toolbar.
+.editor-block-list__block[data-align] + .editor-block-list__block[data-type="core/paragraph"] {
+	.editor-block-contextual-toolbar {
+		float: none;
 	}
 }
 
@@ -867,7 +903,7 @@
 	z-index: z-index(".editor-block-list__breadcrumb");
 
 	// Position in the top right of the border.
-	right: 0;
+	right: -$block-padding;
 	top: -$block-padding - $border-width;
 
 	.components-toolbar {
@@ -892,6 +928,13 @@
 		background: rgba($white, 0.5);
 		color: $dark-gray-700;
 	}
+
+	// Position the breadcrumb closer on mobile.
+	[data-align="left"] &,
+	[data-align="right"] & {
+		right: 0;
+		top: 0;
+	}
 }
 
 .editor-block-list__descendant-arrow::before {