Skip to content

Commit

Permalink
Make the Blocks navigation a nested list to better communicate the bl…
Browse files Browse the repository at this point in the history
…ocks nesting level (#11734)

* Make the Blocks navigation menu a list.

* Simplify role and aria-orientation.

* Revert span to div.
  • Loading branch information
afercia authored and gziolo committed Nov 13, 2018
1 parent f8c4234 commit 4cc3ac0
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ exports[`core/embed block edit matches snapshot 1`] = `
<div
class="components-placeholder__label"
>
<div
<span
class="editor-block-icon has-colors"
>
<svg
Expand All @@ -24,7 +24,7 @@ exports[`core/embed block edit matches snapshot 1`] = `
d="M17 4H3c-1.1 0-2 .9-2 2v8c0 1.1.9 2 2 2h14c1.1 0 2-.9 2-2V6c0-1.1-.9-2-2-2zm-3 6.5L12.5 12l1.5 1.5V15l-3-3 3-3v1.5zm1 4.5v-1.5l1.5-1.5-1.5-1.5V9l3 3-3 3z"
/>
</svg>
</div>
</span>
Embed URL
</div>
<div
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/navigable-container/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export function NavigableMenu( {
stopNavigationEvents
onlyBrowserTabstops={ false }
role={ role }
aria-orientation={ orientation }
aria-orientation={ role === 'presentation' ? null : orientation }
eventToOffset={ eventToOffset }
{ ...rest }
/>
Expand Down
4 changes: 2 additions & 2 deletions packages/editor/src/components/block-icon/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export default function BlockIcon( { icon, showColors = false, className } ) {
} : {};

return (
<div
<span
style={ style }
className={ classnames(
'editor-block-icon',
Expand All @@ -32,6 +32,6 @@ export default function BlockIcon( { icon, showColors = false, className } ) {
) }
>
{ renderedIcon }
</div>
</span>
);
}
12 changes: 6 additions & 6 deletions packages/editor/src/components/block-icon/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,28 +20,28 @@ describe( 'BlockIcon', () => {
expect( wrapper.containsMatchingElement( <Icon icon="format-image" /> ) ).toBe( true );
} );

it( 'renders a div without the has-colors classname', () => {
it( 'renders a span without the has-colors classname', () => {
const wrapper = shallow( <BlockIcon icon="format-image" /> );

expect( wrapper.find( 'div' ).hasClass( 'has-colors' ) ).toBe( false );
expect( wrapper.find( 'span' ).hasClass( 'has-colors' ) ).toBe( false );
} );

it( 'renders a div with the has-colors classname', () => {
it( 'renders a span with the has-colors classname', () => {
const wrapper = shallow( <BlockIcon icon="format-image" showColors /> );

expect( wrapper.find( 'div' ).hasClass( 'has-colors' ) ).toBe( true );
expect( wrapper.find( 'span' ).hasClass( 'has-colors' ) ).toBe( true );
} );

it( 'skips adding background and foreground styles when colors are not enabled', () => {
const wrapper = shallow( <BlockIcon icon={ { background: 'white', foreground: 'black' } } /> );

expect( wrapper.find( 'div' ).prop( 'style' ) ).toEqual( {} );
expect( wrapper.find( 'span' ).prop( 'style' ) ).toEqual( {} );
} );

it( 'adds background and foreground styles when colors are enabled', () => {
const wrapper = shallow( <BlockIcon icon={ { background: 'white', foreground: 'black' } } showColors /> );

expect( wrapper.find( 'div' ).prop( 'style' ) ).toEqual( {
expect( wrapper.find( 'span' ).prop( 'style' ) ).toEqual( {
backgroundColor: 'white',
color: 'black',
} );
Expand Down
32 changes: 22 additions & 10 deletions packages/editor/src/components/block-navigation/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import { withSelect, withDispatch } from '@wordpress/data';
import { MenuItem, MenuGroup } from '@wordpress/components';
import { Button, NavigableMenu } from '@wordpress/components';
import { getBlockType } from '@wordpress/blocks';
import { compose } from '@wordpress/compose';
import { __ } from '@wordpress/i18n';
Expand All @@ -25,23 +25,30 @@ function BlockNavigationList( {
showNestedBlocks,
} ) {
return (
<ul className="editor-block-navigation__list" role="presentation">
/*
* Disable reason: The `list` ARIA role is redundant but
* Safari+VoiceOver won't announce the list otherwise.
*/
/* eslint-disable jsx-a11y/no-redundant-roles */
<ul className="editor-block-navigation__list" role="list">
{ map( blocks, ( block ) => {
const blockType = getBlockType( block.name );
const isSelected = block.clientId === selectedBlockClientId;

return (
<li key={ block.clientId } role="presentation">
<div role="presentation" className="editor-block-navigation__item">
<MenuItem
<li key={ block.clientId }>
<div className="editor-block-navigation__item">
<Button
className={ classnames( 'editor-block-navigation__item-button', {
'is-selected': block.clientId === selectedBlockClientId,
} ) }
onClick={ () => selectBlock( block.clientId ) }
isSelected={ block.clientId === selectedBlockClientId }
role="menuitemradio"
isSelected={ isSelected }
>
<BlockIcon icon={ blockType.icon } showColors />
{ blockType.title }
</MenuItem>
{ isSelected && <span className="screen-reader-text">{ __( '(selected block)' ) }</span> }
</Button>
</div>
{ showNestedBlocks && !! block.innerBlocks && !! block.innerBlocks.length && (
<BlockNavigationList
Expand All @@ -55,6 +62,7 @@ function BlockNavigationList( {
);
} ) }
</ul>
/* eslint-enable jsx-a11y/no-redundant-roles */
);
}

Expand All @@ -67,7 +75,11 @@ function BlockNavigation( { rootBlock, rootBlocks, selectedBlockClientId, select
);

return (
<MenuGroup label={ __( 'Block Navigation' ) }>
<NavigableMenu
role="presentation"
className="editor-block-navigation__container"
>
<p className="editor-block-navigation__label">{ __( 'Block Navigation' ) }</p>
{ hasHierarchy && (
<BlockNavigationList
blocks={ [ rootBlock ] }
Expand All @@ -90,7 +102,7 @@ function BlockNavigation( { rootBlock, rootBlocks, selectedBlockClientId, select
{ __( 'No blocks created yet.' ) }
</p>
) }
</MenuGroup>
</NavigableMenu>
);
}

Expand Down
22 changes: 21 additions & 1 deletion packages/editor/src/components/block-navigation/style.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
$tree-border-width: 2px;
$tree-item-height: 36px;

.editor-block-navigation__container {
padding: $grid-size - $border-width;
}

.editor-block-navigation__label {
margin: 0 0 $grid-size;
color: $dark-gray-300;
}

.editor-block-navigation__list,
.editor-block-navigation__paragraph {
padding: 0;
Expand Down Expand Up @@ -50,15 +59,26 @@ $tree-item-height: 36px;
}

.editor-block-navigation__item-button {
padding: 6px;
display: flex;
align-items: center;
width: 100%;
padding: 6px;
text-align: left;
color: $dark-gray-600;
border-radius: 4px;

.editor-block-icon {
margin-right: 6px;
}

&:hover:not(:disabled):not([aria-disabled="true"]) {
@include menu-style__hover;
}

&:focus:not(:disabled):not([aria-disabled="true"]) {
@include menu-style__focus;
}

&.is-selected,
&.is-selected:focus {
color: $dark-gray-700;
Expand Down

0 comments on commit 4cc3ac0

Please sign in to comment.