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

Fix sibling block inserter displaying at end of block list. #29920

Merged
merged 5 commits into from
Mar 19, 2021
Merged

Fix sibling block inserter displaying at end of block list. #29920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aebae77
Only show inserter version of insertion point if there is a next block
talldan Mar 17, 2021
a7e4298
Clarify comment
talldan Mar 17, 2021
4aebf8d
Improve boolean logic
talldan Mar 18, 2021
1955412
Update e2e test
talldan Mar 19, 2021
0d8c064
Update tests
talldan Mar 19, 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
23 changes: 16 additions & 7 deletions packages/block-editor/src/components/block-list/insertion-point.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,18 @@ function InsertionPointPopover( {
}
}

// Only show the inserter when there's a `nextElement` (a block after the
// insertion point). At the end of the block list the trailing appender
// should serve the purpose of inserting blocks.
Copy link
Member

Choose a reason for hiding this comment

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

What if there's no appender available?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that's an issue, from what I understand this inserter was never intended to show at the end of a block list. Its appearance there is a bug from #28418.

const showInsertionPointInserter =
! isHidden && nextElement && ( isInserterShown || isInserterForced );

// Show the indicator if the insertion point inserter is visible, or if
// the `showInsertionPoint` state is `true`. The latter is generally true
// when hovering blocks for insertion in the block library.
const showInsertionPointIndicator =
showInsertionPointInserter || ( ! isHidden && showInsertionPoint );

/* eslint-disable jsx-a11y/no-static-element-interactions, jsx-a11y/click-events-have-key-events */
// While ideally it would be enough to capture the
// bubbling focus event from the Inserter, due to the
Expand All @@ -224,13 +236,10 @@ function InsertionPointPopover( {
className={ className }
style={ style }
>
{ ! isHidden &&
( showInsertionPoint ||
isInserterShown ||
isInserterForced ) && (
<div className="block-editor-block-list__insertion-point-indicator" />
) }
{ ! isHidden && ( isInserterShown || isInserterForced ) && (
{ showInsertionPointIndicator && (
<div className="block-editor-block-list__insertion-point-indicator" />
) }
{ showInsertionPointInserter && (
<InsertionPointInserter
rootClientId={ rootClientId }
clientId={ nextClientId }
Expand Down
18 changes: 10 additions & 8 deletions packages/e2e-tests/specs/experiments/document-settings.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
/**
* WordPress dependencies
*/
import {
canvas,
trashAllPosts,
activateTheme,
} from '@wordpress/e2e-test-utils';
import { trashAllPosts, activateTheme } from '@wordpress/e2e-test-utils';

/**
* Internal dependencies
Expand Down Expand Up @@ -58,9 +54,15 @@ describe( 'Document Settings', () => {

describe( 'and a template part is clicked in the template', () => {
it( "should display the selected template part's name in the document header", async () => {
// Click on a template part in the template
const header = await canvas().$( '.site-header' );
await header.click();
// Select the header template part via list view.
await page.click( 'button[aria-label="List View"]' );
const headerTemplatePartListViewButton = await page.waitForXPath(
'//button[contains(@class, "block-editor-block-navigation-block-select-button")][contains(., "Header")]'
);
headerTemplatePartListViewButton.click();
await page.click(
'button[aria-label="Close list view sidebar"]'
);

// Evaluate the document settings secondary title
const actual = await getDocumentSettingsSecondaryTitle();
Expand Down
11 changes: 7 additions & 4 deletions 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 @@ -7,7 +7,6 @@ import {
publishPost,
trashAllPosts,
activateTheme,
canvas,
} from '@wordpress/e2e-test-utils';

/**
Expand Down Expand Up @@ -189,9 +188,13 @@ describe( 'Multi-entity save flow', () => {
await navigationPanel.clickItemByText( 'Index' );
await navigationPanel.close();

// Click the first block so that the template part inserts in the right place.
const firstBlock = await canvas().$( '.wp-block' );
await firstBlock.click();
// Select the header template part via list view.
await page.click( 'button[aria-label="List View"]' );
const headerTemplatePartListViewButton = await page.waitForXPath(
'//button[contains(@class, "block-editor-block-navigation-block-select-button")][contains(., "Header")]'
);
headerTemplatePartListViewButton.click();
await page.click( 'button[aria-label="Close list view sidebar"]' );

// Insert something to dirty the editor.
await insertBlock( 'Paragraph' );
Expand Down