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

Site Editor Keyboard Navigation Flow #50296

Merged
merged 7 commits into from
May 5, 2023
Merged

Site Editor Keyboard Navigation Flow #50296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c508da9
Add focus ring and keydown event to enter site editor
jeryj May 3, 2023
08e1571
Set W logo as a Go back to dashboard link when site editor navigation…
jeryj May 3, 2023
be4fe46
Use CSS var for border width focus and calc
jeryj May 4, 2023
68f6f2d
Fix missing - (negative) in the offset
jeryj May 4, 2023
1a035f3
refactor to remove many conditionals
scruffian May 5, 2023
39a9db6
Refactor how to pass props to Iframe
jeryj May 5, 2023
781bdd7
Lint js
jeryj May 5, 2023
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
44 changes: 36 additions & 8 deletions packages/edit-site/src/components/block-editor/editor-canvas.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* External dependencies
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
Expand All @@ -9,6 +14,9 @@ import {
store as blockEditorStore,
} from '@wordpress/block-editor';
import { useSelect, useDispatch } from '@wordpress/data';
import { ENTER, SPACE } from '@wordpress/keycodes';
import { useState, useEffect } from '@wordpress/element';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
Expand All @@ -31,6 +39,30 @@ function EditorCanvas( { enableResizing, settings, children, ...props } ) {
const { setCanvasMode } = unlock( useDispatch( editSiteStore ) );
const deviceStyles = useResizeCanvas( deviceType );
const mouseMoveTypingRef = useMouseMoveTypingReset();
const [ isFocused, setIsFocused ] = useState( false );

useEffect( () => {
if ( canvasMode === 'edit' ) {
setIsFocused( false );
}
}, [ canvasMode ] );
Comment on lines +44 to +48
Copy link
Member

Choose a reason for hiding this comment

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

Hi, @jeryj

Sorry, odd question: I came across this code and wondered how to reproduce the state when this effect is needed.

Copy link
Contributor Author

@jeryj jeryj Jul 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It's a good question! I wasn't sure off the top of my head either 😅 I took a look, and without it, the is-focused classname isn't removed when pressing enter on the editor frame to switch to edit mode. When you return back to the sidebar view, the is-focused class is still there.

To see this behavior, remove this code, then:

  • Start at the site editor with sidebar open
  • Using the tab key to move focus to the editor frame. The is-focused class is what adds the blue focus ring on the iframe since that element doesn't want to deal with focus events or :focus selector 😒
  • Press enter to switch to edit mode
  • Press the W logo to return to the sidebar view
  • The focus is on the W logo, and the is-focus class is incorrectly still on the editor iframe
Screen.Recording.2023-07-13.at.2.02.52.PM.mov

Copy link
Member

Choose a reason for hiding this comment

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

Thank you, @jeryj 🙇


const viewModeProps = {
'aria-label': __( 'Editor Canvas' ),
role: 'button',
tabIndex: 0,
onFocus: () => setIsFocused( true ),
onBlur: () => setIsFocused( false ),
onKeyDown: ( event ) => {
const { keyCode } = event;
if ( keyCode === ENTER || keyCode === SPACE ) {
event.preventDefault();
setCanvasMode( 'edit' );
}
},
onClick: () => setCanvasMode( 'edit' ),
readonly: true,
};

return (
<Iframe
Expand All @@ -40,15 +72,11 @@ function EditorCanvas( { enableResizing, settings, children, ...props } ) {
style={ enableResizing ? {} : deviceStyles }
ref={ mouseMoveTypingRef }
name="editor-canvas"
className="edit-site-visual-editor__editor-canvas"
className={ classnames( 'edit-site-visual-editor__editor-canvas', {
'is-focused': isFocused && canvasMode === 'view',
} ) }
{ ...props }
role={ canvasMode === 'view' ? 'button' : undefined }
onClick={
canvasMode === 'view'
? () => setCanvasMode( 'edit' )
: undefined
}
readonly={ canvasMode === 'view' }
{ ...( canvasMode === 'view' ? viewModeProps : {} ) }
>
<EditorStyles styles={ settings.styles } />
<style>{
Expand Down
6 changes: 6 additions & 0 deletions packages/edit-site/src/components/block-editor/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,14 @@

.edit-site-visual-editor__editor-canvas {
height: 100%;

&.is-focused {
outline: calc(2 * var(--wp-admin-border-width-focus)) solid var(--wp-admin-theme-color);
outline-offset: calc(-2 * var(--wp-admin-border-width-focus));
}
}


&.is-focus-mode {
.edit-site-layout.is-full-canvas & {
padding: $grid-unit-60;
Expand Down
35 changes: 24 additions & 11 deletions packages/edit-site/src/components/site-hub/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,27 +30,40 @@ import { unlock } from '../../private-apis';
const HUB_ANIMATION_DURATION = 0.3;

const SiteHub = forwardRef( ( props, ref ) => {
const { canvasMode } = useSelect( ( select ) => {
const { canvasMode, dashboardLink } = useSelect( ( select ) => {
const { getCanvasMode, getSettings } = unlock(
select( editSiteStore )
);

return {
canvasMode: getCanvasMode(),
dashboardLink: getSettings().__experimentalDashboardLink,
dashboardLink:
getSettings().__experimentalDashboardLink || 'index.php',
};
}, [] );

const disableMotion = useReducedMotion();
const { setCanvasMode } = unlock( useDispatch( editSiteStore ) );
const { clearSelectedBlock } = useDispatch( blockEditorStore );
const siteIconButtonProps = {
label: __( 'Open Navigation' ),
onClick: () => {
if ( canvasMode === 'edit' ) {
clearSelectedBlock();
setCanvasMode( 'view' );
}
},
};
const isBackToDashboardButton = canvasMode === 'view';
const siteIconButtonProps = isBackToDashboardButton
? {
href: dashboardLink,
label: __( 'Go back to the Dashboard' ),
}
: {
href: dashboardLink, // We need to keep the `href` here so the component doesn't remount as a `<button>` and break the animation.
role: 'button',
label: __( 'Open Navigation' ),
onClick: ( event ) => {
event.preventDefault();
if ( canvasMode === 'edit' ) {
clearSelectedBlock();
setCanvasMode( 'view' );
}
},
};

const siteTitle = useSelect(
( select ) =>
select( coreStore ).getEntityRecord( 'root', 'site' )?.title,
Expand Down