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

Writing flow: remove arrow nav limitations #30057

Merged
merged 2 commits into from
Mar 20, 2021
Merged

Writing flow: remove arrow nav limitations #30057

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5be2cb9
Writing flow: remove arrow nav limitations
ellatrix Mar 20, 2021
05bd07d
Adjust e2e test
ellatrix Mar 20, 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
52 changes: 6 additions & 46 deletions packages/block-editor/src/components/writing-flow/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
computeCaretRect,
focus,
isHorizontalEdge,
isTextField,
isVerticalEdge,
placeCaretAtHorizontalEdge,
placeCaretAtVerticalEdge,
Expand All @@ -34,9 +33,7 @@ import { __ } from '@wordpress/i18n';
* Internal dependencies
*/
import {
isBlockFocusStop,
isInSameBlock,
hasInnerBlocksContext,
isInsideRootBlock,
getBlockDOMNode,
getBlockClientId,
Expand Down Expand Up @@ -115,12 +112,17 @@ export function getClosestTabbable(
targetRect = target.getBoundingClientRect();
}

function isTabCandidate( node, i, array ) {
function isTabCandidate( node ) {
// Not a candidate if the node is not tabbable.
if ( ! focus.tabbable.isTabbableIndex( node ) ) {
return false;
}

// Skip focusable elements such as links within content editable nodes.
if ( node.isContentEditable && node.contentEditable !== 'true' ) {
return false;
}

if ( onlyVertical ) {
const nodeRect = node.getBoundingClientRect();

Expand All @@ -132,48 +134,6 @@ export function getClosestTabbable(
}
}

// Prefer text fields...
if ( isTextField( node ) ) {
return true;
}

// ...but settle for block focus stop.
if ( ! isBlockFocusStop( node ) ) {
return false;
}

// If element contains inner blocks, stop immediately at its focus
// wrapper.
if ( hasInnerBlocksContext( node ) ) {
return true;
}

// If navigating out of a block (in reverse), don't consider its
// block focus stop.
if ( node.contains( target ) ) {
return false;
}

// In case of block focus stop, check to see if there's a better
// text field candidate within.
for (
let offset = 1, nextNode;
( nextNode = array[ i + offset ] );
offset++
) {
// Abort if no longer testing descendents of focus stop.
if ( ! node.contains( nextNode ) ) {
break;
}

// Apply same tests by recursion. This is important to consider
// nestable blocks where we don't want to settle for the inner
// block focus stop.
if ( isTabCandidate( nextNode, i + offset, array ) ) {
return false;
}
}

return true;
}

Expand Down
27 changes: 0 additions & 27 deletions packages/block-editor/src/utils/dom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,6 @@ export function getBlockPreviewContainerDOMNode( clientId, doc ) {
return domNode.firstChild || domNode;
}

/**
* Returns true if the given element is a block focus stop. Blocks without their
* own text fields rely on the focus stop to be keyboard navigable.
*
* @param {Element} element Element to test.
*
* @return {boolean} Whether element is a block focus stop.
*/
export function isBlockFocusStop( element ) {
return element.classList.contains( 'block-editor-block-list__block' );
}

/**
* Returns true if two elements are contained within the same block.
*
Expand Down Expand Up @@ -73,21 +61,6 @@ export function isInsideRootBlock( blockElement, element ) {
return parentBlock === blockElement;
}

/**
* Returns true if the given element contains inner blocks (an InnerBlocks
* element).
*
* @param {Element} element Element to test.
*
* @return {boolean} Whether element contains inner blocks.
*/
export function hasInnerBlocksContext( element ) {
return (
element.classList.contains( 'block-editor-block-list__layout' ) ||
!! element.querySelector( '.block-editor-block-list__layout' )
);
}

/**
* Finds the block client ID given any DOM node inside the block.
*
Expand Down
35 changes: 0 additions & 35 deletions packages/block-editor/src/utils/test/dom.js
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions packages/e2e-tests/specs/editor/blocks/quote.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ describe( 'Quote', () => {
await page.keyboard.press( 'ArrowLeft' );
await page.keyboard.press( 'ArrowDown' );
await page.keyboard.press( 'ArrowDown' );
await page.keyboard.press( 'ArrowDown' );
Copy link
Member Author

Choose a reason for hiding this comment

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

Extra arrow down needed because first the block wrapper will be focused, then the content. The block wrapper is an extra navigation stop as intended.

await page.keyboard.press( 'Backspace' );

expect( await getEditedPostContent() ).toMatchSnapshot();
Expand Down