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

Jump to bottom

Fix: Blocks: Alt+F10 should navigate to block toolbar regardless of visibility #11607

Merged
merged 1 commit into from
Nov 9, 2018
Merged

Fix: Blocks: Alt+F10 should navigate to block toolbar regardless of visibility #11607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions packages/editor/CHANGELOG.md
View file
Open in desktop
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
3 changes: 2 additions & 1 deletion packages/editor/src/components/block-list/block-contextual-toolbar.js
View file
Open in desktop
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
View file
Open in desktop
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( () => ( {} ) );
}

render() {
return (
<HoverArea container={ this.wrapperNode }>
Expand Down Expand Up @@ -540,7 +553,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
View file
Open in desktop
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 ) {
Copy link
Member

Choose a reason for hiding this comment

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

If it's part of the public interface, we should include a note in the CHANGELOG.md file.

Ideally also in README.md for the component, but it doesn't exist (until #10699).

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a note to the CHANGELOG.md.

Regarding the README.md given that it is already being added in #10699 my plan is to add a commit in #10699 after merging this PR updating its description to include the new property (provided it is not merged before if that is the case I will add the commit here).

this.focusToolbar();
}
}

render() {
const { children, ...props } = this.props;
return (
Expand Down
7 changes: 0 additions & 7 deletions test/e2e/specs/navigable-toolbar.test.js
View file
Open in desktop
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