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

Escape moves focus to editor region when in select mode #62196

Merged
merged 21 commits into from
Jun 26, 2024
Merged

Escape moves focus to editor region when in select mode #62196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
c191d4d
Escape moves focus to parent when in select mode
jeryj May 31, 2024
5b69791
Escape sends focus directly to canvas and styled with pseudoelement
jeryj Jun 3, 2024
3b7d2de
Use selected block focus style mixin
jeryj Jun 3, 2024
56b8f86
Fix test for escape keypress in select mode
jeryj Jun 3, 2024
c7f7ed3
Redo CSS for editor canvas outline.
jeryj Jun 5, 2024
0614c6f
Fix focus selector to not show if multiselection
jeryj Jun 5, 2024
34aa86a
Use aria-label to improve announcement for focusing editor canvas
jeryj Jun 5, 2024
7f3e381
Move focus to editor content region
jeryj Jun 6, 2024
dd7c1a1
Remove body focus styles
jeryj Jun 6, 2024
9712646
Focus the region of the editor canvas if it is available
jeryj Jun 13, 2024
474be49
Use focus-visible to prevent clicks on regions from showing blue outline
jeryj Jun 13, 2024
c42153e
Revert aria-labels on iframe and writing flow
jeryj Jun 14, 2024
ba6e245
Revert focus style for regions and add new focus-visible style to han…
jeryj Jun 14, 2024
ba2d1ad
Fix incorrect selector when reverting
jeryj Jun 14, 2024
7050e0b
Only show focus on editor region with focus-visible
jeryj Jun 14, 2024
e5a6d50
Revert selected outline mixin move
jeryj Jun 14, 2024
c245a89
Update test to right focus area
jeryj Jun 14, 2024
55f0f32
Convert repeated styles to localized mixins
jeryj Jun 14, 2024
801ceb9
Refactor finding focusable wrapper
jeryj Jun 14, 2024
b32c3c8
Allow for multiple iframed editors on a page
jeryj Jun 14, 2024
387ed23
Convert nodelist to array for firefox compatability
jeryj Jun 25, 2024
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
13 changes: 4 additions & 9 deletions packages/block-editor/src/components/block-tools/block-selection-button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import clsx from 'clsx';
import { dragHandle, trash } from '@wordpress/icons';
import { Button, Flex, FlexItem, ToolbarButton } from '@wordpress/components';
import { useSelect, useDispatch } from '@wordpress/data';
import { useEffect, useRef } from '@wordpress/element';
import { forwardRef, useEffect } from '@wordpress/element';
Copy link
Contributor Author

@jeryj jeryj Jun 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We shouldn't need to forward a ref here anymore. I think some unnecessary things made their way in during the rebase. I'll get this fixed.

I was wrong on this. It is necessary unless we want to check for the selection button by name, as we need to know if the target is our current navigation selection button. There may be another way to check this without passing the ref though.

import {
BACKSPACE,
DELETE,
Expand Down Expand Up @@ -48,10 +48,11 @@ import Shuffle from '../block-toolbar/shuffle';
*
* @param {string} props Component props.
* @param {string} props.clientId Client ID of block.
* @param {Object} ref Reference to the component.
*
* @return {Component} The component to be rendered.
*/
function BlockSelectionButton( { clientId, rootClientId } ) {
function BlockSelectionButton( { clientId, rootClientId }, ref ) {
const selected = useSelect(
( select ) => {
const {
Expand Down Expand Up @@ -125,7 +126,6 @@ function BlockSelectionButton( { clientId, rootClientId } ) {
canMove,
} = selected;
const { setNavigationMode, removeBlock } = useDispatch( blockEditorStore );
const ref = useRef();

// Focus the breadcrumb in navigation mode.
useEffect( () => {
Expand Down Expand Up @@ -164,11 +164,6 @@ function BlockSelectionButton( { clientId, rootClientId } ) {
const isEnter = keyCode === ENTER;
const isSpace = keyCode === SPACE;
const isShift = event.shiftKey;
if ( isEscape && editorMode === 'navigation' ) {
setNavigationMode( false );
event.preventDefault();
return;
}

if ( keyCode === BACKSPACE || keyCode === DELETE ) {
removeBlock( clientId );
Expand Down Expand Up @@ -368,4 +363,4 @@ function BlockSelectionButton( { clientId, rootClientId } ) {
);
}

export default BlockSelectionButton;
export default forwardRef( BlockSelectionButton );
13 changes: 9 additions & 4 deletions packages/block-editor/src/components/block-tools/block-toolbar-breadcrumb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
*/
import clsx from 'clsx';

/**
* WordPress dependencies
*/
import { forwardRef } from '@wordpress/element';

/**
* Internal dependencies
*/
Expand All @@ -11,10 +16,7 @@ import { PrivateBlockPopover } from '../block-popover';
import useBlockToolbarPopoverProps from './use-block-toolbar-popover-props';
import useSelectedBlockToolProps from './use-selected-block-tool-props';

export default function BlockToolbarBreadcrumb( {
clientId,
__unstableContentRef,
} ) {
function BlockToolbarBreadcrumb( { clientId, __unstableContentRef }, ref ) {
const {
capturingClientId,
isInsertionPointVisible,
Expand All @@ -38,9 +40,12 @@ export default function BlockToolbarBreadcrumb( {
{ ...popoverProps }
>
<BlockSelectionButton
ref={ ref }
clientId={ clientId }
rootClientId={ rootClientId }
/>
</PrivateBlockPopover>
);
}

export default forwardRef( BlockToolbarBreadcrumb );
38 changes: 37 additions & 1 deletion packages/block-editor/src/components/block-tools/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export default function BlockTools( {
} = useShowBlockTools();

const {
clearSelectedBlock,
duplicateBlocks,
removeBlocks,
replaceBlocks,
Expand All @@ -92,6 +93,8 @@ export default function BlockTools( {
expandBlock,
} = unlock( useDispatch( blockEditorStore ) );

const blockSelectionButtonRef = useRef();

function onKeyDown( event ) {
if ( event.defaultPrevented ) {
return;
Expand Down Expand Up @@ -152,6 +155,39 @@ export default function BlockTools( {
// block so that focus is directed back to the beginning of the selection.
// In effect, to the user this feels like deselecting the multi-selection.
selectBlock( clientIds[ 0 ] );
} else if (
clientIds.length === 1 &&
event.target === blockSelectionButtonRef?.current
) {
event.preventDefault();
clearSelectedBlock();
// If there are multiple editors, we need to find the iframe that contains our contentRef to make sure
// we're focusing the region that contains this editor.
const editorCanvas =
Array.from(
document
.querySelectorAll( 'iframe[name="editor-canvas"]' )
.values()
).find( ( iframe ) => {
// Find the iframe that contains our contentRef
const iframeDocument =
iframe.contentDocument ||
iframe.contentWindow.document;

return (
iframeDocument ===
__unstableContentRef.current.ownerDocument
);
} ) ?? __unstableContentRef.current;

// The region is provivided by the editor, not the block-editor.
// We should send focus to the region if one is available to reuse the
// same interface for navigating landmarks. If no region is available,
// use the canvas instead.
const focusableWrapper =
editorCanvas?.closest( '[role="region"]' ) ?? editorCanvas;

focusableWrapper.focus();
}
} else if ( isMatch( 'core/block-editor/collapse-list-view', event ) ) {
// If focus is currently within a text field, such as a rich text block or other editable field,
Expand Down Expand Up @@ -182,7 +218,6 @@ export default function BlockTools( {
}
}
}

const blockToolbarRef = usePopoverScroll( __unstableContentRef );
const blockToolbarAfterRef = usePopoverScroll( __unstableContentRef );

Expand Down Expand Up @@ -213,6 +248,7 @@ export default function BlockTools( {

{ showBreadcrumb && (
<BlockToolbarBreadcrumb
ref={ blockSelectionButtonRef }
__unstableContentRef={ __unstableContentRef }
clientId={ clientId }
/>
Expand Down
38 changes: 25 additions & 13 deletions packages/components/src/higher-order/navigate-regions/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,35 @@
// Allow the position to be easily overridden to e.g. fixed.

@mixin region-selection-outline {
outline: 4px solid $components-color-accent;
outline-offset: -4px;
}

@mixin region-selection-focus {
position: absolute;
top: 0;
left: 0;
right: 0;
bottom: 0;
content: "";
pointer-events: none;
@include region-selection-outline;
z-index: z-index(".is-focusing-regions {region} :focus::after");
}

[role="region"] {
position: relative;

// Handles the focus when we programatically send focus to this region
&.interface-interface-skeleton__content:focus-visible::after {
@include region-selection-focus;
}
}

.is-focusing-regions {
[role="region"]:focus::after {
position: absolute;
top: 0;
left: 0;
right: 0;
bottom: 0;
content: "";
pointer-events: none;
outline: 4px solid $components-color-accent;
outline-offset: -4px;
z-index: z-index(".is-focusing-regions {region} :focus::after");
@include region-selection-focus;
}

// Fixes for edge cases.
// Some of the regions are currently used for layout purposes as 'interface skeleton'
// items. When they're absolutely positioned or when they contain absolutely
Expand All @@ -33,7 +46,6 @@
.interface-interface-skeleton__actions .editor-layout__toggle-publish-panel,
.interface-interface-skeleton__actions .editor-layout__toggle-entities-saved-states-panel,
.editor-post-publish-panel {
outline: 4px solid $components-color-accent;
outline-offset: -4px;
@include region-selection-outline;
}
}
14 changes: 6 additions & 8 deletions test/e2e/specs/editor/various/writing-flow.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ test.describe( 'Writing Flow (@firefox, @webkit)', () => {
<!-- /wp:table -->` );
} );

test( 'escape should toggle between edit and navigation modes', async ( {
test( 'escape should set select mode and then focus the canvas', async ( {
page,
writingFlowUtils,
} ) => {
Expand All @@ -975,15 +975,13 @@ test.describe( 'Writing Flow (@firefox, @webkit)', () => {
.poll( writingFlowUtils.getActiveBlockName )
.toBe( 'core/paragraph' );

// Second escape Toggles back to Edit Mode
// Second escape should send focus to the canvas
await page.keyboard.press( 'Escape' );
// The navigation button should be hidden.
await expect( navigationButton ).toBeHidden();
const blockToolbar = page.getByLabel( 'Block tools' );

await expect( blockToolbar ).toBeVisible();
await expect
.poll( writingFlowUtils.getActiveBlockName )
.toBe( 'core/paragraph' );
await expect(
page.getByRole( 'region', { name: 'Editor content' } )
).toBeFocused();
} );

// Checks for regressions of https://github.com/WordPress/gutenberg/issues/40091.
Expand Down
Loading