From 2c8eb79d6d3bb87c517fcbcaf42cffaf8a37edf3 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Fri, 15 Oct 2021 14:27:36 +1100 Subject: [PATCH 1/8] Switching from using Button to an anchor tags for List View tree items. --- .../src/components/list-view/block-select-button.js | 13 +++++++++---- .../src/components/list-view/style.scss | 11 +++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/list-view/block-select-button.js b/packages/block-editor/src/components/list-view/block-select-button.js index ce51d1f2a099..04783bf7d18c 100644 --- a/packages/block-editor/src/components/list-view/block-select-button.js +++ b/packages/block-editor/src/components/list-view/block-select-button.js @@ -6,7 +6,7 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { Button, VisuallyHidden } from '@wordpress/components'; +import { VisuallyHidden } from '@wordpress/components'; import { useInstanceId } from '@wordpress/compose'; import { forwardRef } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; @@ -46,15 +46,19 @@ function ListViewBlockSelectButton( siblingBlockCount, level ); + const onClickHandler = ( event ) => { + onClick(); + event.preventDefault(); + }; return ( <> - +
Date: Fri, 15 Oct 2021 14:36:18 +1100 Subject: [PATCH 2/8] Let's not use the HTML anchor here as it's not reliable in the editor. --- .../src/components/list-view/block-select-button.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/list-view/block-select-button.js b/packages/block-editor/src/components/list-view/block-select-button.js index 04783bf7d18c..a3fb99582ede 100644 --- a/packages/block-editor/src/components/list-view/block-select-button.js +++ b/packages/block-editor/src/components/list-view/block-select-button.js @@ -66,7 +66,7 @@ function ListViewBlockSelectButton( onDragStart={ onDragStart } onDragEnd={ onDragEnd } draggable={ draggable } - href={ `#block-${ blockInformation?.anchor ?? clientId }` } + href={ `#block-${ clientId }` } > From 6e5239616654c239343e90f111e0fe4bf39fb1c3 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Fri, 15 Oct 2021 15:34:30 +1100 Subject: [PATCH 3/8] Updating e2e test selectors --- packages/e2e-tests/specs/editor/blocks/cover.test.js | 2 +- .../specs/editor/various/block-hierarchy-navigation.test.js | 6 +++--- packages/e2e-tests/specs/editor/various/list-view.test.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/e2e-tests/specs/editor/blocks/cover.test.js b/packages/e2e-tests/specs/editor/blocks/cover.test.js index 4cfbcdc955cd..e00bfb8ac513 100644 --- a/packages/e2e-tests/specs/editor/blocks/cover.test.js +++ b/packages/e2e-tests/specs/editor/blocks/cover.test.js @@ -131,7 +131,7 @@ describe( 'Cover', () => { // Select the cover block.By default the child paragraph gets selected. await page.click( '.edit-post-header-toolbar__list-view-toggle' ); await page.click( - '.block-editor-list-view-block__contents-container button' + '.block-editor-list-view-block__contents-container a' ); const heightInput = ( diff --git a/packages/e2e-tests/specs/editor/various/block-hierarchy-navigation.test.js b/packages/e2e-tests/specs/editor/various/block-hierarchy-navigation.test.js index 0eae0977afcb..fd4de7ba31d3 100644 --- a/packages/e2e-tests/specs/editor/various/block-hierarchy-navigation.test.js +++ b/packages/e2e-tests/specs/editor/various/block-hierarchy-navigation.test.js @@ -51,7 +51,7 @@ describe( 'Navigating the block hierarchy', () => { await page.click( '.edit-post-header-toolbar__list-view-toggle' ); const columnsBlockMenuItem = ( await page.$x( - "//button[contains(@class,'block-editor-list-view-block-select-button') and contains(text(), 'Columns')]" + "//a[contains(@class,'block-editor-list-view-block-select-button') and contains(text(), 'Columns')]" ) )[ 0 ]; await columnsBlockMenuItem.click(); @@ -75,7 +75,7 @@ describe( 'Navigating the block hierarchy', () => { // Navigate to the last column block. const lastColumnsBlockMenuItem = ( await page.$x( - "//button[contains(@class,'block-editor-list-view-block-select-button') and contains(text(), 'Column')]" + "//a[contains(@class,'block-editor-list-view-block-select-button') and contains(text(), 'Column')]" ) )[ 3 ]; await lastColumnsBlockMenuItem.click(); @@ -188,7 +188,7 @@ describe( 'Navigating the block hierarchy', () => { await page.click( '.edit-post-header-toolbar__list-view-toggle' ); const groupMenuItem = ( await page.$x( - "//button[contains(@class,'block-editor-list-view-block-select-button') and contains(text(), 'Group')]" + "//a[contains(@class,'block-editor-list-view-block-select-button') and contains(text(), 'Group')]" ) )[ 0 ]; await groupMenuItem.click(); diff --git a/packages/e2e-tests/specs/editor/various/list-view.test.js b/packages/e2e-tests/specs/editor/various/list-view.test.js index 24ffb7082107..85b27f2139fa 100644 --- a/packages/e2e-tests/specs/editor/various/list-view.test.js +++ b/packages/e2e-tests/specs/editor/various/list-view.test.js @@ -42,12 +42,12 @@ describe( 'List view', () => { await pressKeyWithModifier( 'access', 'o' ); const paragraphBlock = await page.waitForXPath( - '//button[contains(., "Paragraph")][@draggable="true"]' + '//a[contains(., "Paragraph")][@draggable="true"]' ); // Drag above the heading block const headingBlock = await page.waitForXPath( - '//button[contains(., "Heading")][@draggable="true"]' + '//a[contains(., "Heading")][@draggable="true"]' ); await dragAndDrop( paragraphBlock, headingBlock, -5 ); From e3fb45bca39a1fab9049486472aec812db615659 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Fri, 15 Oct 2021 15:59:03 +1100 Subject: [PATCH 4/8] Updating e2e test selectors --- packages/e2e-tests/specs/editor/blocks/columns.test.js | 2 +- packages/e2e-tests/specs/experiments/document-settings.test.js | 2 +- .../e2e-tests/specs/experiments/multi-entity-saving.test.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/e2e-tests/specs/editor/blocks/columns.test.js b/packages/e2e-tests/specs/editor/blocks/columns.test.js index da30710f5c43..f593800cd097 100644 --- a/packages/e2e-tests/specs/editor/blocks/columns.test.js +++ b/packages/e2e-tests/specs/editor/blocks/columns.test.js @@ -21,7 +21,7 @@ describe( 'Columns', () => { await page.click( '.edit-post-header-toolbar__list-view-toggle' ); const columnBlockMenuItem = ( await page.$x( - '//button[contains(concat(" ", @class, " "), " block-editor-list-view-block-select-button ")][text()="Column"]' + '//a[contains(concat(" ", @class, " "), " block-editor-list-view-block-select-button ")][text()="Column"]' ) )[ 0 ]; await columnBlockMenuItem.click(); diff --git a/packages/e2e-tests/specs/experiments/document-settings.test.js b/packages/e2e-tests/specs/experiments/document-settings.test.js index 0d764a3e3dda..f79331d5183d 100644 --- a/packages/e2e-tests/specs/experiments/document-settings.test.js +++ b/packages/e2e-tests/specs/experiments/document-settings.test.js @@ -60,7 +60,7 @@ describe( 'Document Settings', () => { '.edit-post-header-toolbar__list-view-toggle' ); const headerTemplatePartListViewButton = await page.waitForXPath( - '//button[contains(@class, "block-editor-list-view-block-select-button")][contains(., "Header")]' + '//a[contains(@class, "block-editor-list-view-block-select-button")][contains(., "Header")]' ); headerTemplatePartListViewButton.click(); await page.click( diff --git a/packages/e2e-tests/specs/experiments/multi-entity-saving.test.js b/packages/e2e-tests/specs/experiments/multi-entity-saving.test.js index a8429cc24a47..fb8a2bd19b8b 100644 --- a/packages/e2e-tests/specs/experiments/multi-entity-saving.test.js +++ b/packages/e2e-tests/specs/experiments/multi-entity-saving.test.js @@ -249,7 +249,7 @@ describe( 'Multi-entity save flow', () => { // Select the header template part via list view. await page.click( '.edit-site-header-toolbar__list-view-toggle' ); const headerTemplatePartListViewButton = await page.waitForXPath( - '//button[contains(@class, "block-editor-list-view-block-select-button")][contains(., "Header")]' + '//a[contains(@class, "block-editor-list-view-block-select-button")][contains(., "Header")]' ); headerTemplatePartListViewButton.click(); await page.click( 'button[aria-label="Close list view sidebar"]' ); From a074b96fc9c55292fab525742f654160ca02241a Mon Sep 17 00:00:00 2001 From: ramonjd Date: Mon, 18 Oct 2021 13:18:46 +1100 Subject: [PATCH 5/8] The `href` attribute triggers the browser's native HTML drag operations. When the link is dragged, the element's outerHTML is set in DataTransfer object as text/html. We need to clear any HTML drag data to prevent `pasteHandler` from calling inside `useOnBlockDrop`. --- .../components/list-view/block-select-button.js | 15 ++++++++++----- .../various/block-hierarchy-navigation.test.js | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/list-view/block-select-button.js b/packages/block-editor/src/components/list-view/block-select-button.js index a3fb99582ede..303c1ddc4a70 100644 --- a/packages/block-editor/src/components/list-view/block-select-button.js +++ b/packages/block-editor/src/components/list-view/block-select-button.js @@ -46,9 +46,14 @@ function ListViewBlockSelectButton( siblingBlockCount, level ); - const onClickHandler = ( event ) => { - onClick(); - event.preventDefault(); + + // The `href` attribute triggers the browser's native HTML drag operations. + // When the link is dragged, the element's outerHTML is set in DataTransfer object as text/html. + // We need to clear any HTML drag data to prevent `pasteHandler` from firing + // inside the `useOnBlockDrop` hook. + const onDragStartHandler = ( event ) => { + event.dataTransfer.clearData(); + onDragStart( event ); }; return ( @@ -58,12 +63,12 @@ function ListViewBlockSelectButton( 'block-editor-list-view-block-select-button', className ) } - onClick={ onClickHandler } + onClick={ onClick } aria-describedby={ descriptionId } ref={ ref } tabIndex={ tabIndex } onFocus={ onFocus } - onDragStart={ onDragStart } + onDragStart={ onDragStartHandler } onDragEnd={ onDragEnd } draggable={ draggable } href={ `#block-${ clientId }` } diff --git a/packages/e2e-tests/specs/editor/various/block-hierarchy-navigation.test.js b/packages/e2e-tests/specs/editor/various/block-hierarchy-navigation.test.js index fd4de7ba31d3..69611a0eebfd 100644 --- a/packages/e2e-tests/specs/editor/various/block-hierarchy-navigation.test.js +++ b/packages/e2e-tests/specs/editor/various/block-hierarchy-navigation.test.js @@ -155,7 +155,7 @@ describe( 'Navigating the block hierarchy', () => { // Return to first block. await openListViewSidebar(); await page.keyboard.press( 'ArrowUp' ); - await page.keyboard.press( 'Space' ); + await page.keyboard.press( 'Enter' ); // Replace its content. await pressKeyWithModifier( 'primary', 'a' ); From e9d17d711ed0fd7f715781bb07e31e1aee8b5baa Mon Sep 17 00:00:00 2001 From: ramonjd Date: Fri, 22 Oct 2021 11:07:50 +1100 Subject: [PATCH 6/8] Using the Button's component's anchor tag, given that we're passing an href. This allows us to remove the custom CSS for the native anchor element. --- .../src/components/list-view/block-select-button.js | 6 +++--- .../block-editor/src/components/list-view/style.scss | 11 ----------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/packages/block-editor/src/components/list-view/block-select-button.js b/packages/block-editor/src/components/list-view/block-select-button.js index 303c1ddc4a70..99fd51b9ebb4 100644 --- a/packages/block-editor/src/components/list-view/block-select-button.js +++ b/packages/block-editor/src/components/list-view/block-select-button.js @@ -6,7 +6,7 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { VisuallyHidden } from '@wordpress/components'; +import { Button, VisuallyHidden } from '@wordpress/components'; import { useInstanceId } from '@wordpress/compose'; import { forwardRef } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; @@ -58,7 +58,7 @@ function ListViewBlockSelectButton( return ( <> - ) } - +
Date: Sat, 23 Oct 2021 20:16:22 +1100 Subject: [PATCH 7/8] Adding key event handlers for space and enter. This is to persist button element behaviour, even though we're switching to an anchor element. Forcing a tabIndex of `0` so the anchor elements are tabbable. --- .../src/components/list-view/block-select-button.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/list-view/block-select-button.js b/packages/block-editor/src/components/list-view/block-select-button.js index 99fd51b9ebb4..5c20bc37b114 100644 --- a/packages/block-editor/src/components/list-view/block-select-button.js +++ b/packages/block-editor/src/components/list-view/block-select-button.js @@ -19,6 +19,7 @@ import useBlockDisplayInformation from '../use-block-display-information'; import { getBlockPositionDescription } from './utils'; import BlockTitle from '../block-title'; import ListViewExpander from './expander'; +import { SPACE, ENTER } from '@wordpress/keycodes'; function ListViewBlockSelectButton( { @@ -30,7 +31,6 @@ function ListViewBlockSelectButton( position, siblingBlockCount, level, - tabIndex, onFocus, onDragStart, onDragEnd, @@ -56,6 +56,13 @@ function ListViewBlockSelectButton( onDragStart( event ); }; + function onKeyDownHandler( event ) { + if ( event.keyCode === ENTER || event.keyCode === SPACE ) { + event.preventDefault(); + onClick( event ); + } + } + return ( <>