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.

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

Add a header menu to switch between edit and select tool #18624

Merged
merged 11 commits into from
Nov 28, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -1141,16 +1141,12 @@ _Parameters_

<a name="setNavigationMode" href="#setNavigationMode">#</a> **setNavigationMode**

Returns an action object used to enable or disable the navigation mode.
Generators that triggers an action used to enable or disable the navigation mode.

_Parameters_

- _isNavigationMode_ `string`: Enable/Disable navigation mode.

_Returns_

- `Object`: Action object

<a name="setTemplateValidity" href="#setTemplateValidity">#</a> **setTemplateValidity**

Returns an action object resetting the template validity.
Expand Down
4 changes: 4 additions & 0 deletions packages/block-editor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,10 @@ _Type_

- `Object`

<a name="ToolSelector" href="#ToolSelector">#</a> **ToolSelector**

Undocumented declaration.

<a name="transformStyles" href="#transformStyles">#</a> **transformStyles**

Applies a series of CSS rule transforms to wrap selectors inside a given class and/or rewrite URLs depending on the parameters passed.
Expand Down
16 changes: 11 additions & 5 deletions packages/block-editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ function BlockListBlock( {
animateOnChange,
enableAnimation,
isNavigationMode,
enableNavigationMode,
setNavigationMode,
} ) {
// Random state used to rerender the component if needed, ideally we don't need this
const [ , updateRerenderState ] = useState( {} );
Expand Down Expand Up @@ -326,7 +326,7 @@ function BlockListBlock( {
isSelected &&
isEditMode
) {
enableNavigationMode();
setNavigationMode( true );
wrapper.current.focus();
}
break;
Expand All @@ -345,6 +345,14 @@ function BlockListBlock( {
return;
}

if (
isNavigationMode &&
isSelected &&
isInsideRootBlock( blockNodeRef.current, event.target )
) {
setNavigationMode( false );
}

if ( event.shiftKey ) {
if ( ! isSelected ) {
onShiftSelection();
Expand Down Expand Up @@ -778,9 +786,7 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, { select } ) => {
toggleSelection( selectionEnabled ) {
toggleSelection( selectionEnabled );
},
enableNavigationMode() {
setNavigationMode( true );
},
setNavigationMode,
};
} );

Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export {
RichTextToolbarButton,
__unstableRichTextInputEvent,
} from './rich-text';
export { default as ToolSelector } from './tool-selector';
export { default as URLInput } from './url-input';
export { default as URLInputButton } from './url-input/button';
export { default as URLPopover } from './url-popover';
Expand Down
12 changes: 7 additions & 5 deletions packages/block-editor/src/components/inner-blocks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import classnames from 'classnames';
/**
* WordPress dependencies
*/
import { withViewportMatch } from '@wordpress/viewport'; // Temporary click-through disable on desktop.
import { withViewportMatch } from '@wordpress/viewport';
import { Component } from '@wordpress/element';
import { withSelect, withDispatch } from '@wordpress/data';
import { synchronizeBlocksWithTemplate, withBlockContentContext } from '@wordpress/blocks';
Expand Down Expand Up @@ -102,7 +102,7 @@ class InnerBlocks extends Component {

render() {
const {
isSmallScreen, // Temporary click-through disable on desktop.
enableClickThrough,
clientId,
hasOverlay,
renderAppender,
Expand All @@ -117,7 +117,7 @@ class InnerBlocks extends Component {
const isPlaceholder = template === null && !! templateOptions;

const classes = classnames( 'editor-inner-blocks block-editor-inner-blocks', {
'has-overlay': isSmallScreen && ( hasOverlay && ! isPlaceholder ), // Temporary click-through disable on desktop.
'has-overlay': enableClickThrough && ( hasOverlay && ! isPlaceholder ),
} );

return (
Expand All @@ -141,7 +141,7 @@ class InnerBlocks extends Component {
}

InnerBlocks = compose( [
withViewportMatch( { isSmallScreen: '< medium' } ), // Temporary click-through disable on desktop.
withViewportMatch( { isSmallScreen: '< medium' } ),
withBlockEditContext( ( context ) => pick( context, [ 'clientId' ] ) ),
withSelect( ( select, ownProps ) => {
const {
Expand All @@ -151,8 +151,9 @@ InnerBlocks = compose( [
getBlockListSettings,
getBlockRootClientId,
getTemplateLock,
isNavigationMode,
} = select( 'core/block-editor' );
const { clientId } = ownProps;
const { clientId, isSmallScreen } = ownProps;
const block = getBlock( clientId );
const rootClientId = getBlockRootClientId( clientId );

Expand All @@ -161,6 +162,7 @@ InnerBlocks = compose( [
blockListSettings: getBlockListSettings( clientId ),
hasOverlay: block.name !== 'core/template' && ! isBlockSelected( clientId ) && ! hasSelectedInnerBlock( clientId, true ),
parentLock: getTemplateLock( rootClientId ),
enableClickThrough: isNavigationMode() || isSmallScreen,
gziolo marked this conversation as resolved.
Show resolved Hide resolved
};
} ),
withDispatch( ( dispatch, ownProps ) => {
Expand Down
76 changes: 76 additions & 0 deletions packages/block-editor/src/components/tool-selector/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/**
* WordPress dependencies
*/
import {
Dropdown,
IconButton,
MenuItemsChoice,
SVG,
Path,
NavigableMenu,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useSelect, useDispatch } from '@wordpress/data';
import { ifViewportMatches } from '@wordpress/viewport';

const editIcon = <SVG xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24"><Path fill="none" d="M0 0h24v24H0V0z" /><Path d="M14.06 9.02l.92.92L5.92 19H5v-.92l9.06-9.06M17.66 3c-.25 0-.51.1-.7.29l-1.83 1.83 3.75 3.75 1.83-1.83c.39-.39.39-1.02 0-1.41l-2.34-2.34c-.2-.2-.45-.29-.71-.29zm-3.6 3.19L3 17.25V21h3.75L17.81 9.94l-3.75-3.75z" /></SVG>;
const selectIcon = <SVG xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24"><Path d="M6.5 1v21.5l6-6.5H21L6.5 1zm5.1 13l-3.1 3.4V5.9l7.8 8.1h-4.7z" /></SVG>;

function ToolSelector() {
const isNavigationTool = useSelect( ( select ) => select( 'core/block-editor' ).isNavigationMode() );
const { setNavigationMode } = useDispatch( 'core/block-editor' );
const onSwitchMode = ( mode ) => {
setNavigationMode( mode === 'edit' ? false : true );
};

return (
<Dropdown
renderToggle={ ( { isOpen, onToggle } ) => (
<IconButton
icon={ isNavigationTool ? selectIcon : editIcon }
aria-expanded={ isOpen }
onClick={ onToggle }
label={ __( 'Tools' ) }
/>
) }
renderContent={ () => (
<>
<NavigableMenu
role="menu"
aria-label={ __( 'Tools' ) }
>
<MenuItemsChoice
value={ isNavigationTool ? 'select' : 'edit' }
onSelect={ onSwitchMode }
choices={ [
{
value: 'edit',
label: (
<>
{ editIcon }
{ __( 'Edit' ) }
</>
),
},
{
value: 'select',
label: (
<>
{ selectIcon }
{ __( 'Select' ) }
</>
),
},
] }
/>
</NavigableMenu>
<div className="block-editor-tool-selector__help">
{ __( 'Tools offer different interactions for block selection & editing. To select, press Escape, to go back to editing, press Enter.' ) }
</div>
</>
) }
/>
);
}

export default ifViewportMatches( 'medium' )( ToolSelector );
5 changes: 5 additions & 0 deletions packages/block-editor/src/components/tool-selector/style.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
.block-editor-tool-selector__help {
padding: $grid-size-large;
border-top: 1px solid $light-gray-500;
color: $dark-gray-300;
}
11 changes: 1 addition & 10 deletions packages/block-editor/src/components/writing-flow/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,6 @@ class WritingFlow extends Component {

onMouseDown() {
this.verticalRect = null;
this.disableNavigationMode();
}

disableNavigationMode() {
if ( this.props.isNavigationMode ) {
this.props.disableNavigationMode();
}
}

/**
Expand Down Expand Up @@ -377,7 +370,6 @@ class WritingFlow extends Component {
* Sets focus to the end of the last tabbable text field, if one exists.
*/
focusLastTextField() {
this.disableNavigationMode();
const focusableNodes = focus.focusable.find( this.container );
const target = findLast( focusableNodes, isTabbableTextField );
if ( target ) {
Expand Down Expand Up @@ -445,11 +437,10 @@ export default compose( [
};
} ),
withDispatch( ( dispatch ) => {
const { multiSelect, selectBlock, setNavigationMode, clearSelectedBlock } = dispatch( 'core/block-editor' );
const { multiSelect, selectBlock, clearSelectedBlock } = dispatch( 'core/block-editor' );
return {
onMultiSelect: multiSelect,
onSelectBlock: selectBlock,
disableNavigationMode: () => setNavigationMode( false ),
clearSelectedBlock,
};
} ),
Expand Down
16 changes: 11 additions & 5 deletions packages/block-editor/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { castArray, first, get, includes } from 'lodash';
* WordPress dependencies
*/
import { getDefaultBlockName, createBlock } from '@wordpress/blocks';
import { speak } from '@wordpress/a11y';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
Expand Down Expand Up @@ -793,15 +795,19 @@ export function __unstableMarkAutomaticChange() {
}

/**
* Returns an action object used to enable or disable the navigation mode.
* Generators that triggers an action used to enable or disable the navigation mode.
*
* @param {string} isNavigationMode Enable/Disable navigation mode.
*
* @return {Object} Action object
*/
export function setNavigationMode( isNavigationMode = true ) {
return {
export function * setNavigationMode( isNavigationMode = true ) {
yield {
type: 'SET_NAVIGATION_MODE',
isNavigationMode,
};

if ( isNavigationMode ) {
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure that it should be announced every time someone switches modes? When editing individual blocks with the keyboard it might be a common pattern to switch between modes.

Copy link
Member

Choose a reason for hiding this comment

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

For example when I enable VoiceOver (⌘F5 in MacOS, works best in Safari), I see the following message when the browser has focus:

Screenshot 2019-11-26 at 13 53 56

I see it now. We can give it a try. Someone should battle test it though :)

Copy link
Contributor

Choose a reason for hiding this comment

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

It is my understanding that yes, we do need something like this. We received helpful information from a user in #17088 (comment), who had also created #18583 which was related to the switch.

Copy link
Member

Choose a reason for hiding this comment

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

Right, my concern is mostly about the number of details included in the message. Anyway, let's collect feedback and refine only if necessary.

speak( __( 'You are currently in navigation mode. Navigate blocks using arrow keys. To exit navigation mode and edit the selected block, press Enter.' ) );
} else {
speak( __( 'You are currently in edit mode. To return to the navigation mode, press Escape.' ) );
}
}
2 changes: 1 addition & 1 deletion packages/block-editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1256,7 +1256,7 @@ export const blockListSettings = ( state = {}, action ) => {
*
* @return {string} Updated state.
*/
export function isNavigationMode( state = true, action ) {
export function isNavigationMode( state = false, action ) {
if ( action.type === 'SET_NAVIGATION_MODE' ) {
return action.isNavigationMode;
}
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
@import "./components/rich-text/format-toolbar/style.scss";
@import "./components/rich-text/style.scss";
@import "./components/skip-to-selected-block/style.scss";
@import "./components/tool-selector/style.scss";
@import "./components/url-input/style.scss";
@import "./components/url-popover/style.scss";
@import "./components/warning/style.scss";
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/menu-items-choice/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export default function MenuItemsChoice( {
icon={ isSelected && 'yes' }
isSelected={ isSelected }
shortcut={ item.shortcut }
className="components-menu-items-choice"
onClick={ () => {
if ( ! isSelected ) {
onSelect( item.value );
Expand Down
12 changes: 12 additions & 0 deletions packages/components/src/menu-items-choice/style.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
.components-menu-items-choice,
.components-menu-items-choice.components-icon-button {
padding-left: 2rem;

&.has-icon {
padding-left: 0.5rem;
}

.dashicon {
margin-right: 4px;
}
}
1 change: 1 addition & 0 deletions packages/components/src/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
@import "./icon-button/style.scss";
@import "./menu-group/style.scss";
@import "./menu-item/style.scss";
@import "./menu-items-choice/style.scss";
@import "./modal/style.scss";
@import "./notice/style.scss";
@import "./panel/style.scss";
Expand Down
6 changes: 6 additions & 0 deletions packages/e2e-test-utils/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## master

### Breaking Changes

- The disableNavigationMode utility was removed. By default, the editor is in edit mode now.

## 3.0.0 (2019-11-14)

### Breaking Changes
Expand Down
8 changes: 0 additions & 8 deletions packages/e2e-test-utils/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,6 @@ _Parameters_

- _slug_ `string`: Plugin slug.

<a name="disableNavigationMode" href="#disableNavigationMode">#</a> **disableNavigationMode**

Triggers edit mode if not already active.

_Returns_

- `Promise`: Promise resolving after enabling the keyboard edit mode.

<a name="disablePrePublishChecks" href="#disablePrePublishChecks">#</a> **disablePrePublishChecks**

Disables Pre-publish checks.
Expand Down
3 changes: 0 additions & 3 deletions packages/e2e-test-utils/src/create-new-post.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { addQueryArgs } from '@wordpress/url';
* Internal dependencies
*/
import { visitAdminPage } from './visit-admin-page';
import { disableNavigationMode } from './keyboard-mode';

/**
* Creates new post.
Expand Down Expand Up @@ -37,6 +36,4 @@ export async function createNewPost( {
if ( enableTips ) {
await page.reload();
}

await disableNavigationMode();
}
1 change: 0 additions & 1 deletion packages/e2e-test-utils/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export { selectBlockByClientId } from './select-block-by-client-id';
export { setBrowserViewport } from './set-browser-viewport';
export { setPostContent } from './set-post-content';
export { switchEditorModeTo } from './switch-editor-mode-to';
export { disableNavigationMode } from './keyboard-mode';
export { switchUserToAdmin } from './switch-user-to-admin';
export { switchUserToTest } from './switch-user-to-test';
export { toggleMoreMenu } from './toggle-more-menu';
Expand Down
13 changes: 1 addition & 12 deletions packages/e2e-test-utils/src/keyboard-mode.js
Original file line number Diff line number Diff line change
@@ -1,12 +1 @@
/**
* Triggers edit mode if not already active.
*
* @return {Promise} Promise resolving after enabling the keyboard edit mode.
*/
export async function disableNavigationMode() {
const focusedElement = await page.$( ':focus' );
await page.click( '.editor-post-title' );
if ( focusedElement ) {
await focusedElement.focus();
}
}

Copy link
Member

Choose a reason for hiding this comment

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

Should this file have been removed? It remains as an empty, unreferenced file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😅

Loading