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

Implement roving tabindex on the header toolbar #22354

Merged
merged 9 commits into from
Jun 8, 2020
Merged

Implement roving tabindex on the header toolbar #22354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fdbba33
Implement roving tabindex on the top toolbar
diegohaz May 14, 2020
8ca8540
Update header toolbar styles
diegohaz May 26, 2020
2cd9f8c
Refactor block navigation dropdown code
diegohaz May 26, 2020
f4aabc9
Refactor header toolbar code
diegohaz May 26, 2020
ffa948b
Update inserter is-pressed styles
diegohaz May 26, 2020
9ae1761
Make tool selector unavailable on the header toolbar component
diegohaz May 26, 2020
d18eeaf
Merge branch 'master' into add/top-toolbar-roving-tabindex
diegohaz May 26, 2020
a759470
Merge branch 'master' into add/top-toolbar-roving-tabindex
diegohaz Jun 4, 2020
47a9bcb
Support as prop on ToolbarItem
diegohaz Jun 4, 2020
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
26 changes: 20 additions & 6 deletions packages/block-editor/src/components/block-navigation/dropdown.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Button, Dropdown, SVG, Path } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useSelect } from '@wordpress/data';
import { useShortcut } from '@wordpress/keyboard-shortcuts';
import { useCallback } from '@wordpress/element';
import { useCallback, forwardRef } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -23,7 +23,13 @@ const MenuIcon = (
</SVG>
);

function BlockNavigationDropdownToggle( { isEnabled, onToggle, isOpen } ) {
function BlockNavigationDropdownToggle( {
isEnabled,
onToggle,
isOpen,
innerRef,
...props
} ) {
useShortcut(
'core/edit-post/toggle-block-navigation',
useCallback( onToggle, [ onToggle ] ),
Expand All @@ -42,6 +48,8 @@ function BlockNavigationDropdownToggle( { isEnabled, onToggle, isOpen } ) {

return (
<Button
{ ...props }
ref={ innerRef }
icon={ MenuIcon }
aria-expanded={ isOpen }
onClick={ isEnabled ? onToggle : undefined }
Expand All @@ -53,7 +61,10 @@ function BlockNavigationDropdownToggle( { isEnabled, onToggle, isOpen } ) {
);
}

function BlockNavigationDropdown( { isDisabled, __experimentalFeatures } ) {
function BlockNavigationDropdown(
{ isDisabled, __experimentalFeatures, ...props },
ref
) {
const hasBlocks = useSelect(
( select ) => !! select( 'core/block-editor' ).getBlockCount(),
[]
Expand All @@ -64,9 +75,12 @@ function BlockNavigationDropdown( { isDisabled, __experimentalFeatures } ) {
<Dropdown
contentClassName="block-editor-block-navigation__popover"
position="bottom right"
renderToggle={ ( toggleProps ) => (
renderToggle={ ( { isOpen, onToggle } ) => (
<BlockNavigationDropdownToggle
{ ...toggleProps }
{ ...props }
innerRef={ ref }
isOpen={ isOpen }
onToggle={ onToggle }
isEnabled={ isEnabled }
/>
) }
Expand All @@ -80,4 +94,4 @@ function BlockNavigationDropdown( { isDisabled, __experimentalFeatures } ) {
);
}

export default BlockNavigationDropdown;
export default forwardRef( BlockNavigationDropdown );
12 changes: 5 additions & 7 deletions packages/block-editor/src/components/tool-selector/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useSelect, useDispatch } from '@wordpress/data';
import { useViewportMatch } from '@wordpress/compose';
import { forwardRef } from '@wordpress/element';

const editIcon = (
<SVG
Expand All @@ -34,16 +34,12 @@ const selectIcon = (
</SVG>
);

function ToolSelector() {
function ToolSelector( props, ref ) {
const isNavigationTool = useSelect(
( select ) => select( 'core/block-editor' ).isNavigationMode(),
[]
);
const { setNavigationMode } = useDispatch( 'core/block-editor' );
const isMediumViewport = useViewportMatch( 'medium' );
if ( ! isMediumViewport ) {
return null;
}

const onSwitchMode = ( mode ) => {
setNavigationMode( mode === 'edit' ? false : true );
Expand All @@ -53,6 +49,8 @@ function ToolSelector() {
<Dropdown
renderToggle={ ( { isOpen, onToggle } ) => (
<Button
{ ...props }
ref={ ref }
icon={ isNavigationTool ? selectIcon : editIcon }
aria-expanded={ isOpen }
onClick={ onToggle }
Expand Down Expand Up @@ -99,4 +97,4 @@ function ToolSelector() {
);
}

export default ToolSelector;
export default forwardRef( ToolSelector );
15 changes: 11 additions & 4 deletions packages/components/src/toolbar-item/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,31 @@ import warning from '@wordpress/warning';
*/
import ToolbarContext from '../toolbar-context';

function ToolbarItem( { children, ...props }, ref ) {
function ToolbarItem( { children, as: Component, ...props }, ref ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think at some point we need to introduce consistency to our components. Some of them use "tagName", others "as". We kind maybe move to "as" as a bit nicer while maintaining fallbacks for BC.

const accessibleToolbarState = useContext( ToolbarContext );

if ( typeof children !== 'function' ) {
if ( typeof children !== 'function' && ! Component ) {
warning(
'`ToolbarItem` is a generic headless component that accepts only function children props'
'`ToolbarItem` is a generic headless component. You must pass either a `children` prop as a function or an `as` prop as a component.'
);
return null;
}

const allProps = { ...props, ref, 'data-experimental-toolbar-item': true };

if ( ! accessibleToolbarState ) {
if ( Component ) {
return <Component { ...allProps }>{ children }</Component>;
}
return children( allProps );
}

return (
<BaseToolbarItem { ...accessibleToolbarState } { ...allProps }>
<BaseToolbarItem
{ ...accessibleToolbarState }
{ ...allProps }
as={ Component }
>
{ children }
</BaseToolbarItem>
);
Expand Down
24 changes: 17 additions & 7 deletions packages/edit-post/src/components/header/header-toolbar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ import {
EditorHistoryRedo,
EditorHistoryUndo,
} from '@wordpress/editor';
import { Button } from '@wordpress/components';
import {
Button,
__experimentalToolbarItem as ToolbarItem,
} from '@wordpress/components';
import { plus } from '@wordpress/icons';

function HeaderToolbar( { onToggleInserter, isInserterOpen } ) {
Expand Down Expand Up @@ -65,7 +68,8 @@ function HeaderToolbar( { onToggleInserter, isInserterOpen } ) {
className="edit-post-header-toolbar"
aria-label={ toolbarAriaLabel }
>
<Button
<ToolbarItem
as={ Button }
className="edit-post-header-toolbar__inserter-toggle"
isPrimary
isPressed={ isInserterOpen }
Expand All @@ -77,11 +81,17 @@ function HeaderToolbar( { onToggleInserter, isInserterOpen } ) {
'Generic label for block inserter button'
) }
/>
<ToolSelector />
<EditorHistoryUndo />
<EditorHistoryRedo />
<TableOfContents hasOutlineItemsDisabled={ isTextModeEnabled } />
<BlockNavigationDropdown isDisabled={ isTextModeEnabled } />
{ isLargeViewport && <ToolbarItem as={ ToolSelector } /> }
<ToolbarItem as={ EditorHistoryUndo } />
<ToolbarItem as={ EditorHistoryRedo } />
<ToolbarItem
as={ TableOfContents }
hasOutlineItemsDisabled={ isTextModeEnabled }
/>
<ToolbarItem
as={ BlockNavigationDropdown }
isDisabled={ isTextModeEnabled }
/>
{ displayBlockToolbar && (
<div className="edit-post-header-toolbar__block-toolbar">
<BlockToolbar hideDragHandle />
Expand Down
25 changes: 24 additions & 1 deletion packages/edit-post/src/components/header/header-toolbar/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
.edit-post-header-toolbar {
display: inline-flex;
align-items: center;
border: none;

// Hide all action buttons except the inserter on mobile.
> .components-button {
Expand All @@ -23,6 +24,28 @@
display: flex;
}
}

// The Toolbar component adds different styles to buttons, so we reset them
// here to the original button styles
> .components-button.has-icon,
> .components-dropdown > .components-button.has-icon {
height: $button-size;
min-width: $button-size;
padding: 6px;

&.is-pressed {
background: $dark-gray-primary;
}

&:focus:not(:disabled) {
box-shadow: 0 0 0 $border-width-focus $theme-color;
outline: 1px solid transparent;
}

&::before {
display: none;
}
}
}

// Block toolbar when fixed to the top of the screen.
Expand Down Expand Up @@ -82,7 +105,7 @@
}
}

.edit-post-header-toolbar .edit-post-header-toolbar__inserter-toggle {
.edit-post-header-toolbar .edit-post-header-toolbar__inserter-toggle.has-icon {
margin-right: $grid-unit-10;
// Special dimensions for this button.
min-width: 32px;
Expand Down
11 changes: 9 additions & 2 deletions packages/editor/src/components/editor-history/redo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ import { withSelect, withDispatch } from '@wordpress/data';
import { compose } from '@wordpress/compose';
import { displayShortcut } from '@wordpress/keycodes';
import { redo as redoIcon } from '@wordpress/icons';
import { forwardRef } from '@wordpress/element';

function EditorHistoryRedo( { hasRedo, redo } ) {
function EditorHistoryRedo( { hasRedo, redo, innerRef, ...props } ) {
return (
<Button
{ ...props }
ref={ innerRef }
icon={ redoIcon }
label={ __( 'Redo' ) }
shortcut={ displayShortcut.primaryShift( 'z' ) }
Expand All @@ -24,11 +27,15 @@ function EditorHistoryRedo( { hasRedo, redo } ) {
);
}

export default compose( [
const EnhancedEditorHistoryRedo = compose( [
Copy link
Member

Choose a reason for hiding this comment

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

Have you considered refactoring this and other similar components (EditorHistoryUndo or TableOfContents) to use hooks? I think it would remove the need to use innerRef and you could apply ref directly on the component.

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 haven't played with @wordpress/data hooks yet, but that's a good idea! Will take a note on that for a follow up PR.

Copy link
Member

Choose a reason for hiding this comment

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

Sure thing👍

withSelect( ( select ) => ( {
hasRedo: select( 'core/editor' ).hasEditorRedo(),
} ) ),
withDispatch( ( dispatch ) => ( {
redo: dispatch( 'core/editor' ).redo,
} ) ),
] )( EditorHistoryRedo );

export default forwardRef( ( props, ref ) => (
<EnhancedEditorHistoryRedo { ...props } innerRef={ ref } />
) );
11 changes: 9 additions & 2 deletions packages/editor/src/components/editor-history/undo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ import { withSelect, withDispatch } from '@wordpress/data';
import { compose } from '@wordpress/compose';
import { displayShortcut } from '@wordpress/keycodes';
import { undo as undoIcon } from '@wordpress/icons';
import { forwardRef } from '@wordpress/element';

function EditorHistoryUndo( { hasUndo, undo } ) {
function EditorHistoryUndo( { hasUndo, undo, innerRef, ...props } ) {
return (
<Button
{ ...props }
ref={ innerRef }
icon={ undoIcon }
label={ __( 'Undo' ) }
shortcut={ displayShortcut.primary( 'z' ) }
Expand All @@ -24,11 +27,15 @@ function EditorHistoryUndo( { hasUndo, undo } ) {
);
}

export default compose( [
const EnhancedEditorHistoryUndo = compose( [
withSelect( ( select ) => ( {
hasUndo: select( 'core/editor' ).hasEditorUndo(),
} ) ),
withDispatch( ( dispatch ) => ( {
undo: dispatch( 'core/editor' ).undo,
} ) ),
] )( EditorHistoryUndo );

export default forwardRef( ( props, ref ) => (
<EnhancedEditorHistoryUndo { ...props } innerRef={ ref } />
) );
16 changes: 14 additions & 2 deletions packages/editor/src/components/table-of-contents/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,28 @@ import { __ } from '@wordpress/i18n';
import { Dropdown, Button } from '@wordpress/components';
import { withSelect } from '@wordpress/data';
import { info } from '@wordpress/icons';
import { forwardRef } from '@wordpress/element';

/**
* Internal dependencies
*/
import TableOfContentsPanel from './panel';

function TableOfContents( { hasBlocks, hasOutlineItemsDisabled } ) {
function TableOfContents( {
hasBlocks,
hasOutlineItemsDisabled,
innerRef,
...props
} ) {
return (
<Dropdown
position="bottom"
className="table-of-contents"
contentClassName="table-of-contents__popover"
renderToggle={ ( { isOpen, onToggle } ) => (
<Button
{ ...props }
ref={ innerRef }
onClick={ hasBlocks ? onToggle : undefined }
icon={ info }
aria-expanded={ isOpen }
Expand All @@ -37,8 +45,12 @@ function TableOfContents( { hasBlocks, hasOutlineItemsDisabled } ) {
);
}

export default withSelect( ( select ) => {
const TableOfContentsWithSelect = withSelect( ( select ) => {
return {
hasBlocks: !! select( 'core/block-editor' ).getBlockCount(),
};
} )( TableOfContents );

export default forwardRef( ( props, ref ) => (
<TableOfContentsWithSelect { ...props } innerRef={ ref } />
) );