From 60d018a81047a5d65614d86740821869ad4b7d60 Mon Sep 17 00:00:00 2001 From: Joen Asmussen Date: Thu, 1 Nov 2018 10:34:22 +0100 Subject: [PATCH 1/5] 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 | 67 ++++---- .../src/components/block-list/style.scss | 161 +++++++++++------- 4 files changed, 155 insertions(+), 102 deletions(-) diff --git a/packages/block-library/src/columns/editor.scss b/packages/block-library/src/columns/editor.scss index a96a6dcf8c1ea3..73c668177b2bef 100644 --- a/packages/block-library/src/columns/editor.scss +++ b/packages/block-library/src/columns/editor.scss @@ -69,10 +69,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 { @@ -82,8 +82,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 2795f5330ba361..0463c005479ef4 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 8b656f0616944e..bd156cb3f79ecd 100644 --- a/packages/editor/src/components/block-list/block.js +++ b/packages/editor/src/components/block-list/block.js @@ -515,45 +515,46 @@ export class BlockListBlock extends Component { onDragEnd={ this.onDragEnd } /> ) } - { shouldShowBreadcrumb && ( - - ) } - { shouldShowContextualToolbar && } { isFirstMultiSelected && ( ) } - - - { isValid && blockEdit } - { isValid && mode === 'html' && ( - - ) } - { ! isValid && [ - , -
- { getSaveElement( blockType, block.attributes ) } -
, - ] } - - { shouldShowMobileToolbar && ( - + { shouldShowBreadcrumb && ( + ) } - { !! error && } - + { shouldShowContextualToolbar && } + + + { isValid && blockEdit } + { isValid && mode === 'html' && ( + + ) } + { ! isValid && [ + , +
+ { getSaveElement( blockType, block.attributes ) } +
, + ] } + + { shouldShowMobileToolbar && ( + + ) } + { !! error && } + + { showEmptyBlockSideInserter && (
diff --git a/packages/editor/src/components/block-list/style.scss b/packages/editor/src/components/block-list/style.scss index 28632c0eda60af..1d64c5f7cc9290 100644 --- a/packages/editor/src/components/block-list/style.scss +++ b/packages/editor/src/components/block-list/style.scss @@ -295,13 +295,23 @@ // Position toolbar better on mobile. .editor-block-contextual-toolbar { + width: auto; border-bottom: 1px solid $light-gray-500; - bottom: $block-padding; - left: auto; - right: auto; + bottom: auto; } } + // Unlike most explicit left/right alignments, this one should be flipped by the auto-RTL system. + &[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. @@ -407,8 +417,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. @@ -485,7 +495,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; } @@ -506,7 +517,7 @@ } } - // Left side UI + // Left side UI. > .editor-block-mover { padding-right: $block-side-ui-clearance; @@ -724,58 +735,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; } } @@ -815,19 +840,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="right"] & { - @include break-small() { - // RTL note: this rule should not be auto-flipped based on direction. - /*rtl:ignore*/ - float: right; - } + // RTL note: this rule should not be auto-flipped based on direction. + /*rtl:ignore*/ + float: right; + } + + .editor-block-list__block[data-align="left"] &, + .editor-block-list__block[data-align="right"] & { + // Move the block toolbar out of the flow using translate, but less for floats. + transform: translateY(-$block-padding -$border-width); } } @@ -835,19 +862,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; } } @@ -862,7 +898,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 { @@ -887,6 +923,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 { From 120fe08ad8e5eda11d8c60e9eb27daa14377d9fc Mon Sep 17 00:00:00 2001 From: Nicola Heald Date: Wed, 7 Nov 2018 13:06:19 +0000 Subject: [PATCH 2/5] Use block navigator to select the blocks --- test/e2e/specs/block-icons.test.js | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/test/e2e/specs/block-icons.test.js b/test/e2e/specs/block-icons.test.js index a02c497bbcd1bc..0e44cddb124f44 100644 --- a/test/e2e/specs/block-icons.test.js +++ b/test/e2e/specs/block-icons.test.js @@ -2,10 +2,11 @@ * Internal dependencies */ import { + ACCESS_MODIFIER_KEYS, + pressWithModifier, newPost, insertBlock, searchForBlock, - selectBlockByClientId, } from '../support/utils'; import { activatePlugin, deactivatePlugin } from '../support/plugins'; @@ -34,18 +35,17 @@ async function getFirstInserterIcon() { return await getInnerHTML( INSERTER_ICON_SELECTOR ); } +async function selectFirstBlock() { + await pressWithModifier( ACCESS_MODIFIER_KEYS, 'o' ); + const navButtons = await page.$$( '.editor-block-navigation__item-button' ); + await navButtons[ 0 ].click(); +} + describe( 'Correctly Renders Block Icons on Inserter and Inspector', () => { const dashIconRegex = /.*?<\/svg>/; const circleString = ''; const svgIcon = new RegExp( `${ circleString }` ); - const getBlockIdFromBlockName = async ( blockName ) => { - return await page.$eval( - `[data-type="${ blockName }"] > .editor-block-list__block-edit`, - ( el ) => el.getAttribute( 'data-block' ) - ); - }; - const validateSvgIcon = ( iconHtml ) => { expect( iconHtml ).toMatch( svgIcon ); }; @@ -81,7 +81,7 @@ describe( 'Correctly Renders Block Icons on Inserter and Inspector', () => { it( 'Renders correctly the icon on the inspector', async () => { await insertBlock( blockTitle ); - await selectBlockByClientId( await getBlockIdFromBlockName( blockName ) ); + await selectFirstBlock(); validateIcon( await getInnerHTML( INSPECTOR_ICON_SELECTOR ) ); } ); } @@ -105,7 +105,6 @@ describe( 'Correctly Renders Block Icons on Inserter and Inspector', () => { } ); describe( 'Block with dash icon and background and foreground colors', () => { - const blockName = 'test/test-dash-icon-colors'; const blockTitle = 'TestDashIconColors'; it( 'Renders the icon in the inserter with the correct colors', async () => { await searchForBlock( blockTitle ); @@ -116,7 +115,7 @@ describe( 'Correctly Renders Block Icons on Inserter and Inspector', () => { it( 'Renders the icon in the inspector with the correct colors', async () => { await insertBlock( blockTitle ); - await selectBlockByClientId( await getBlockIdFromBlockName( blockName ) ); + await selectFirstBlock(); validateDashIcon( await getInnerHTML( INSPECTOR_ICON_SELECTOR ) ); @@ -126,7 +125,6 @@ describe( 'Correctly Renders Block Icons on Inserter and Inspector', () => { } ); describe( 'Block with svg icon and background color', () => { - const blockName = 'test/test-svg-icon-background'; const blockTitle = 'TestSvgIconBackground'; it( 'Renders the icon in the inserter with the correct background color and an automatically compute readable foreground color', async () => { await searchForBlock( blockTitle ); @@ -137,7 +135,7 @@ describe( 'Correctly Renders Block Icons on Inserter and Inspector', () => { it( 'Renders correctly the icon on the inspector', async () => { await insertBlock( blockTitle ); - await selectBlockByClientId( await getBlockIdFromBlockName( blockName ) ); + await selectFirstBlock(); validateSvgIcon( await getInnerHTML( INSPECTOR_ICON_SELECTOR ) ); From 1980bee1031caf50a7af5e9bae3889bd6b5af530 Mon Sep 17 00:00:00 2001 From: Joen Asmussen Date: Thu, 8 Nov 2018 12:23:01 +0100 Subject: [PATCH 3/5] Remove block and inline level rules This removes the rules that were specific to inline blocks or block level blocks and levels the playing field. The benefit is no special rules. The downside is that a block following a float still might not perfectly match the user expectation. This PR also fixes issues with mobile toolbars. Finally, it makes the block toolbar not float. This rule used to be necessary, but for whatever reason it no longer appears to be. --- .../src/components/block-list/style.scss | 32 ++++--------------- 1 file changed, 6 insertions(+), 26 deletions(-) diff --git a/packages/editor/src/components/block-list/style.scss b/packages/editor/src/components/block-list/style.scss index 1d64c5f7cc9290..203f5dec761190 100644 --- a/packages/editor/src/components/block-list/style.scss +++ b/packages/editor/src/components/block-list/style.scss @@ -296,7 +296,7 @@ // Position toolbar better on mobile. .editor-block-contextual-toolbar { width: auto; - border-bottom: 1px solid $light-gray-500; + border-bottom: $border-width solid $light-gray-500; bottom: auto; } } @@ -543,15 +543,15 @@ .editor-block-list__block-mobile-toolbar { display: flex; flex-direction: row; + // Make room for the height of the block toolbar above. - margin-top: $grid-size + $block-toolbar-height - $block-padding; + transform: translateY($block-padding + $border-width); + margin-top: $block-toolbar-height; margin-right: -$block-padding; margin-left: -$block-padding; border-top: $border-width solid $light-gray-500; height: $block-toolbar-height; - transform: translateY(#{ $block-padding + $border-width }); - @include break-small() { display: none; } @@ -745,7 +745,7 @@ // Position toolbar below the block on mobile. position: absolute; - bottom: $block-toolbar-height - $block-padding - 1px; + bottom: $block-toolbar-height - $block-padding; left: -$block-padding; right: -$block-padding; @@ -769,7 +769,7 @@ &[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; + margin-top: -$block-toolbar-height; } // Make block toolbar full width on mobile. @@ -829,13 +829,6 @@ // Compensate for translate, so the sticky sticks to the top. top: $block-toolbar-height + $block-padding; } - - // This is an important one. Because the toolbar is sticky, it's part of the flow. - // It behaves as relative, in other words, until it reaches an edge and then behaves as fixed. - // But by applying a float, we take it out of this flow. The benefit is that we don't need to compensate for margins. - // In turn, this allows margins on sibling elements to collapse to parent elements. - // RTL note: this rule does need to be auto-flipped based on direction. - float: left; } } @@ -874,19 +867,6 @@ } } -// 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; - } -} - /** * Hover label From 5d4801cf767e07c9954403a4d6a6c58361e69fba Mon Sep 17 00:00:00 2001 From: Joen Asmussen Date: Fri, 9 Nov 2018 10:29:19 +0100 Subject: [PATCH 4/5] Fix rebase issue. --- packages/block-library/src/columns/editor.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/columns/editor.scss b/packages/block-library/src/columns/editor.scss index 73c668177b2bef..234fd4783f4b24 100644 --- a/packages/block-library/src/columns/editor.scss +++ b/packages/block-library/src/columns/editor.scss @@ -134,6 +134,6 @@ pointer-events: none; } -:not(.components-disabled) > .wp-block-columns > .editor-inner-blocks > .editor-block-list__layout > [data-type="core/column"] > .editor-block-list__block-edit > .editor-inner-blocks { +:not(.components-disabled) > .wp-block-columns > .editor-inner-blocks > .editor-block-list__layout > [data-type="core/column"] > .editor-block-list__block-edit .editor-inner-blocks { pointer-events: all; } From d5ff02ade8a0924d36049ed2f51d145ecebea4ca Mon Sep 17 00:00:00 2001 From: Joen Asmussen Date: Fri, 9 Nov 2018 13:12:40 +0100 Subject: [PATCH 5/5] Fix classic block. --- packages/block-library/src/classic/editor.scss | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/classic/editor.scss b/packages/block-library/src/classic/editor.scss index bc4e371702a875..0720c22d4a2c3d 100644 --- a/packages/block-library/src/classic/editor.scss +++ b/packages/block-library/src/classic/editor.scss @@ -216,9 +216,12 @@ div[data-type="core/freeform"] .editor-block-contextual-toolbar + div { // So we move it to the right, and make room for it. @include break-small() { .editor-block-list__block[data-type="core/freeform"] { + .editor-block-switcher__no-switcher-icon { + display: none; + } .editor-block-contextual-toolbar { float: right; - margin-right: -$block-side-ui-clearance - $border-width; + margin-right: $icon-button-size - $block-padding + $border-width; transform: translateY(-#{ $block-padding - $border-width }); top: $block-padding;