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: fix vertical arrow nav in table (and generally grid) #22105

Merged
merged 3 commits into from
May 5, 2020
Merged

Writing flow: fix vertical arrow nav in table (and generally grid) #22105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e061cae
Writing flow: fix arrow nav in table (and generally gird)
ellatrix May 5, 2020
b0b9515
Add e2e test
ellatrix May 5, 2020
2ca1e8e
fix columns e2e test
ellatrix May 5, 2020
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
30 changes: 28 additions & 2 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 @@ -93,10 +93,18 @@ export function isNavigationCandidate( element, keyCode, hasModifier ) {
* @param {Element} target Currently focused text field.
* @param {boolean} isReverse True if considering as the first field.
* @param {Element} containerElement Element containing all blocks.
* @param {boolean} onlyVertical Wether to only consider tabbable elements
* that are visually above or under the
* target.
*
* @return {?Element} Optimal tab target, if one exists.
*/
export function getClosestTabbable( target, isReverse, containerElement ) {
export function getClosestTabbable(
target,
isReverse,
containerElement,
onlyVertical
) {
// Since the current focus target is not guaranteed to be a text field,
// find all focusables. Tabbability is considered later.
let focusableNodes = focus.focusable.find( containerElement );
Expand All @@ -112,12 +120,29 @@ export function getClosestTabbable( target, isReverse, containerElement ) {
focusableNodes.indexOf( target ) + 1
);

let targetRect;

if ( onlyVertical ) {
targetRect = target.getBoundingClientRect();
}

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

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

if (
nodeRect.left >= targetRect.right ||
nodeRect.right <= targetRect.left
) {
return false;
}
}

// Prefer text fields...
if ( isTextField( node ) ) {
return true;
Expand Down Expand Up @@ -517,7 +542,8 @@ export default function WritingFlow( { children } ) {
const closestTabbable = getClosestTabbable(
target,
isReverse,
container.current
container.current,
true
);

if ( closestTabbable ) {
Expand Down
6 changes: 6 additions & 0 deletions packages/e2e-tests/specs/editor/blocks/__snapshots__/table.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,9 @@ exports[`Table displays a form for choosing the row and column count of the tabl
<figure class=\\"wp-block-table\\"><table><tbody><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr></tbody></table></figure>
<!-- /wp:table -->"
`;

exports[`Table up and down arrow navigation 1`] = `
"<!-- wp:table -->
<figure class=\\"wp-block-table\\"><table><tbody><tr><td>1</td><td>4</td></tr><tr><td>2</td><td>3</td></tr></tbody></table></figure>
<!-- /wp:table -->"
`;
18 changes: 18 additions & 0 deletions packages/e2e-tests/specs/editor/blocks/table.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,4 +267,22 @@ describe( 'Table', () => {

expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'up and down arrow navigation', async () => {
await insertBlock( 'Table' );

// Create the table.
await clickButton( createButtonLabel );

await page.keyboard.press( 'Tab' );
await page.keyboard.type( '1' );
await page.keyboard.press( 'ArrowDown' );
await page.keyboard.type( '2' );
await page.keyboard.press( 'ArrowRight' );
await page.keyboard.type( '3' );
await page.keyboard.press( 'ArrowUp' );
await page.keyboard.type( '4' );

expect( await getEditedPostContent() ).toMatchSnapshot();
} );
} );
20 changes: 1 addition & 19 deletions packages/e2e-tests/specs/editor/various/writing-flow.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,25 +80,7 @@ describe( 'Writing Flow', () => {
activeBlockName = await getActiveBlockName();
expect( activeBlockName ).toBe( 'core/column' );
await page.keyboard.press( 'ArrowUp' );
activeBlockName = await getActiveBlockName();
Copy link
Member Author

Choose a reason for hiding this comment

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

This is expected. Arrow up shouldn't go through all the previous columns.

expect( activeBlockName ).toBe( 'core/paragraph' );
activeElementText = await page.evaluate(
() => document.activeElement.textContent
);
expect( activeElementText ).toBe( '1st col' );

// Arrow up from first text field in nested context focuses column and
// columns wrappers before escaping out.
let activeElementBlockType;
await page.keyboard.press( 'ArrowUp' );
activeElementBlockType = await page.evaluate( () =>
document.activeElement.getAttribute( 'data-type' )
);
expect( activeElementBlockType ).toBe( 'core/column' );
activeBlockName = await getActiveBlockName();
expect( activeBlockName ).toBe( 'core/column' );
await page.keyboard.press( 'ArrowUp' );
activeElementBlockType = await page.evaluate( () =>
const activeElementBlockType = await page.evaluate( () =>
document.activeElement.getAttribute( 'data-type' )
);
expect( activeElementBlockType ).toBe( 'core/columns' );
Expand Down