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

Block Editor List View: use anchor elements instead of buttons #35655

Merged
merged 8 commits into from
Nov 25, 2021
Merged

Block Editor List View: use anchor elements instead of buttons #35655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2c8eb79
Switching from using Button to an anchor tags for List View tree items.
ramonjd Oct 15, 2021
c5ec335
Let's not use the HTML anchor here as it's not reliable in the editor.
ramonjd Oct 15, 2021
6e52396
Updating e2e test selectors
ramonjd Oct 15, 2021
e3fb45b
Updating e2e test selectors
ramonjd Oct 15, 2021
a074b96
The `href` attribute triggers the browser's native HTML drag operations.
ramonjd Oct 18, 2021
e9d17d7
Using the Button's component's anchor tag, given that we're passing a…
ramonjd Oct 22, 2021
4099726
Adding key event handlers for space and enter. This is to persist but…
ramonjd Oct 23, 2021
48e288c
Reverting back to Space key since we've added a keyboard event for Sp…
ramonjd Nov 16, 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
21 changes: 20 additions & 1 deletion packages/block-editor/src/components/list-view/block-select-button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
{
Expand Down Expand Up @@ -47,6 +48,22 @@ function ListViewBlockSelectButton(
level
);

// 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();
Copy link
Member Author

Choose a reason for hiding this comment

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

The href attribute is triggering the browser's native drag + copy link. (More information on the browser's drag operations)

Oct-18-2021 12-21-19

It's populating the DataTransfer object with text/html, which is triggering a paste command in onHTMLDrop used by useBlockDrop

This effect is causing the e2e failure.

Clearing the data using event.dataTransfer.clearData(); seems to address this.

Are there better ways to handle this event?

Maybe useOnBlockDrop could take a prop that explicitly defines/overwrites the onDrop functionality so we wouldn't have to create extra functions in our components.

onDragStart( event );
};

function onKeyDownHandler( event ) {
if ( event.keyCode === ENTER || event.keyCode === SPACE ) {
event.preventDefault();
onClick( event );
}
}

return (
<>
<Button
Expand All @@ -55,13 +72,15 @@ function ListViewBlockSelectButton(
className
) }
onClick={ onClick }
onKeyDown={ onKeyDownHandler }
aria-describedby={ descriptionId }
ref={ ref }
tabIndex={ tabIndex }
onFocus={ onFocus }
onDragStart={ onDragStart }
onDragStart={ onDragStartHandler }
onDragEnd={ onDragEnd }
draggable={ draggable }
href={ `#block-${ clientId }` }
>
<ListViewExpander onClick={ onToggleExpanded } />
<BlockIcon icon={ blockInformation?.icon } showColors />
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/specs/editor/blocks/columns.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/specs/editor/blocks/cover.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
Expand Down
6 changes: 3 additions & 3 deletions packages/e2e-tests/specs/editor/various/block-hierarchy-navigation.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions packages/e2e-tests/specs/editor/various/list-view.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/specs/experiments/document-settings.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/specs/experiments/multi-entity-saving.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"]' );
Expand Down