Skip to content

Commit

Permalink
Fix: Blocks: Alt+F10 should navigate to block toolbar regardless of v…
Browse files Browse the repository at this point in the history
…isibility (#11607)

## Description
Closes: #10907
When pressing alt + f10 if the contextual toolbar was not visible because the user was typing, we did not focus the block contextual toolbar, and we focused the header instead.

This PR addresses this problem.
To solve the problem, the PR performs the following actions:
Adds a prop to NavigableToolbar that enables it to focus when mounting.
Adds an event handler for pressing alt+f10 key combination on BlockListBlock and state flag the indicates that the handling is in progress.

## How has this been tested?
I checked the unified toolbar mode was not enabled.
I added a paragraph, I wrote something until the toolbar disappears I pressed alt+f10, and I verified the block toolbar appeared with the first item focused.
I repeated the test with other blocks, e.g., writing on the image caption.
  • Loading branch information
jorgefilipecosta committed Nov 9, 2018
1 parent 52a3375 commit a2fee33
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 10 deletions.
4 changes: 4 additions & 0 deletions packages/editor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

## 6.2.1 (2018-11-09)

### New Features

- In `NavigableToolbar`, a property focusOnMount was added, if true, the toolbar will get focus as soon as it mounted. Defaults to false.

### Polish

- Reactive block styles.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import { __ } from '@wordpress/i18n';
import NavigableToolbar from '../navigable-toolbar';
import { BlockToolbar } from '../';

function BlockContextualToolbar() {
function BlockContextualToolbar( { focusOnMount } ) {
return (
<NavigableToolbar
focusOnMount={ focusOnMount }
className="editor-block-contextual-toolbar"
aria-label={ __( 'Block Toolbar' ) }
>
Expand Down
40 changes: 38 additions & 2 deletions packages/editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
isUnmodifiedDefaultBlock,
getUnregisteredTypeHandlerName,
} from '@wordpress/blocks';
import { withFilters } from '@wordpress/components';
import { KeyboardShortcuts, withFilters } from '@wordpress/components';
import { __, sprintf } from '@wordpress/i18n';
import { withDispatch, withSelect } from '@wordpress/data';
import { withViewportMatch } from '@wordpress/viewport';
Expand Down Expand Up @@ -57,6 +57,7 @@ export class BlockListBlock extends Component {
this.bindBlockNode = this.bindBlockNode.bind( this );
this.setAttributes = this.setAttributes.bind( this );
this.maybeHover = this.maybeHover.bind( this );
this.forceFocusedContextualToolbar = this.forceFocusedContextualToolbar.bind( this );
this.hideHoverEffects = this.hideHoverEffects.bind( this );
this.mergeBlocks = this.mergeBlocks.bind( this );
this.insertBlocksAfter = this.insertBlocksAfter.bind( this );
Expand All @@ -78,6 +79,7 @@ export class BlockListBlock extends Component {
dragging: false,
isHovered: false,
};
this.isForcingContextualToolbar = false;
}

componentDidMount() {
Expand All @@ -87,6 +89,11 @@ export class BlockListBlock extends Component {
}

componentDidUpdate( prevProps ) {
if ( this.isForcingContextualToolbar ) {
// The forcing of contextual toolbar should only be true during one update,
// after the first update normal conditions should apply.
this.isForcingContextualToolbar = false;
}
if ( this.props.isTypingWithinBlock || this.props.isSelected ) {
this.hideHoverEffects();
}
Expand Down Expand Up @@ -372,6 +379,12 @@ export class BlockListBlock extends Component {
}
}

forceFocusedContextualToolbar() {
this.isForcingContextualToolbar = true;
// trigger a re-render
this.setState( () => ( {} ) );

This comment has been minimized.

Copy link
@youknowriad

youknowriad Apr 15, 2019

Contributor

If we need to rerender after changing the value of isForcingContextualToolbar, any reason isForcingContextualToolbar is not a state value?

}

render() {
return (
<HoverArea container={ this.wrapperNode }>
Expand Down Expand Up @@ -541,7 +554,30 @@ export class BlockListBlock extends Component {
isHidden={ ! ( isHovered || isSelected ) || hoverArea !== 'left' }
/>
) }
{ shouldShowContextualToolbar && <BlockContextualToolbar /> }
{ (
shouldShowContextualToolbar ||
this.isForcingContextualToolbar
) && (
<BlockContextualToolbar
// If the toolbar is being shown because of being forced
// it should focus the toolbar right after the mount.
focusOnMount={ this.isForcingContextualToolbar }
/>
) }
{ (
! shouldShowContextualToolbar &&
isSelected &&
! hasFixedToolbar &&
! isEmptyDefaultBlock
) && (
<KeyboardShortcuts
bindGlobal
eventName="keydown"
shortcuts={ {
'alt+f10': this.forceFocusedContextualToolbar,
} }
/>
) }
<IgnoreNestedEvents
ref={ this.bindBlockNode }
onDragStart={ this.preventDrag }
Expand Down
6 changes: 6 additions & 0 deletions packages/editor/src/components/navigable-toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ class NavigableToolbar extends Component {
}
}

componentDidMount() {
if ( this.props.focusOnMount ) {
this.focusToolbar();
}
}

render() {
const { children, ...props } = this.props;
return (
Expand Down
7 changes: 0 additions & 7 deletions test/e2e/specs/navigable-toolbar.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,6 @@ describe( 'block toolbar', () => {
// until starting to type within it.
await page.keyboard.type( 'Example' );

// [TEMPORARY]: With non-unified toolbar, the toolbar will not
// be visible since the user has entered a "typing" mode.
// Future iterations should ensure Alt+F10 works in a block
// to focus the toolbar regardless of whether it is presently
// visible.
await page.keyboard.press( 'Escape' );

// Upward
await pressWithModifier( 'Alt', 'F10' );
expect( await isInBlockToolbar() ).toBe( true );
Expand Down

0 comments on commit a2fee33

Please sign in to comment.