From 56a46d34083ee7b80d43c2e916243add1d3305f7 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 12 Oct 2017 13:45:30 +0100 Subject: [PATCH 1/8] Components: Add a generic NavigableMenu component and refactor DropDown menu to use it --- components/dropdown-menu/index.js | 277 +++++------------------- components/dropdown-menu/style.scss | 9 +- components/dropdown-menu/test/index.js | 125 +---------- components/dropdown/README.md | 4 +- components/index.js | 1 + components/navigable-menu/README.md | 40 ++++ components/navigable-menu/index.js | 63 ++++++ components/navigable-menu/test/index.js | 49 +++++ editor/assets/stylesheets/_z-index.scss | 1 - editor/block-switcher/index.js | 152 ++++++------- editor/block-switcher/style.scss | 5 +- 11 files changed, 288 insertions(+), 438 deletions(-) create mode 100644 components/navigable-menu/README.md create mode 100644 components/navigable-menu/index.js create mode 100644 components/navigable-menu/test/index.js diff --git a/components/dropdown-menu/index.js b/components/dropdown-menu/index.js index 6faf2d5675bf05..5e742be9ee43b1 100644 --- a/components/dropdown-menu/index.js +++ b/components/dropdown-menu/index.js @@ -2,12 +2,10 @@ * External dependencies */ import classnames from 'classnames'; -import clickOutside from 'react-click-outside'; /** * WordPress dependencies */ -import { Component, findDOMNode } from '@wordpress/element'; import { keycodes } from '@wordpress/utils'; /** @@ -16,226 +14,71 @@ import { keycodes } from '@wordpress/utils'; import './style.scss'; import IconButton from '../icon-button'; import Dashicon from '../dashicon'; - -const { TAB, ESCAPE, LEFT, UP, RIGHT, DOWN } = keycodes; - -export class DropdownMenu extends Component { - constructor() { - super( ...arguments ); - - this.bindReferenceNode = this.bindReferenceNode.bind( this ); - this.closeMenu = this.closeMenu.bind( this ); - this.toggleMenu = this.toggleMenu.bind( this ); - this.focusIndex = this.focusIndex.bind( this ); - this.focusPrevious = this.focusPrevious.bind( this ); - this.focusNext = this.focusNext.bind( this ); - this.handleKeyDown = this.handleKeyDown.bind( this ); - this.calculateMenuPosition = this.calculateMenuPosition.bind( this ); - - this.nodes = {}; - this.timer = null; - - this.state = { - activeIndex: null, - open: false, - menuLeft: 0, - }; - } - - bindReferenceNode( name ) { - return ( node ) => { - this.nodes[ name ] = node; - }; - } - - handleClickOutside() { - if ( ! this.state.open ) { - return; - } - - this.closeMenu(); - } - - closeMenu() { - this.setState( { - open: false, - } ); - } - - calculateMenuPosition() { - const { toggle } = this.nodes; - if ( ! toggle ) { - return; - } - const node = findDOMNode( toggle ); - let n = node; - let scrollLeft = 0; - while ( n !== null && n !== node.offsetParent ) { - scrollLeft += n.scrollLeft; - n = n.parentNode; - } - const menuLeft = node.offsetLeft - scrollLeft - 4; - if ( this.state.menuLeft !== menuLeft ) { - this.setState( { menuLeft } ); - } - } - - toggleMenu() { - const open = ! this.state.open; - if ( open ) { - this.calculateMenuPosition(); - } - this.setState( { open } ); - } - - focusIndex( activeIndex ) { - this.setState( { activeIndex } ); - } - - focusPrevious() { - const { activeIndex } = this.state; - const { controls } = this.props; - if ( ! controls ) { - return; - } - - const maxIndex = controls.length - 1; - const prevIndex = activeIndex <= 0 ? maxIndex : activeIndex - 1; - this.focusIndex( prevIndex ); - } - - focusNext() { - const { activeIndex } = this.state; - const { controls } = this.props; - if ( ! controls ) { - return; - } - - const nextIndex = ( activeIndex + 1 ) % controls.length; - this.focusIndex( nextIndex ); - } - - handleKeyDown( keydown ) { - if ( this.state.open ) { - switch ( keydown.keyCode ) { - case TAB: - keydown.stopPropagation(); - this.closeMenu(); - break; - - case LEFT: - case UP: - keydown.preventDefault(); - keydown.stopPropagation(); - this.focusPrevious(); - break; - - case RIGHT: - case DOWN: - keydown.preventDefault(); - keydown.stopPropagation(); - this.focusNext(); - break; - case ESCAPE: - keydown.preventDefault(); - keydown.stopPropagation(); - // eslint-disable-next-line react/no-find-dom-node - findDOMNode( this.nodes.toggle ).focus(); - this.closeMenu(); - break; - default: - break; - } - } else { - switch ( keydown.keyCode ) { - case DOWN: - keydown.preventDefault(); - keydown.stopPropagation(); - this.toggleMenu(); - break; - - default: - break; - } - } +import Dropdown from '../dropdown'; +import NavigableMenu from '../navigable-menu'; + +const { ESCAPE, DOWN } = keycodes; + +function DropdownMenu( { + icon = 'menu', + label, + menuLabel, + controls, +} ) { + if ( ! controls || ! controls.length ) { + return null; } - componentDidUpdate( prevProps, prevState ) { - const { open, activeIndex } = this.state; - - // Focus the first item when the menu opens. - if ( ! prevState.open && open ) { - this.focusIndex( 0 ); - } - - // Change focus to active index - const { menu } = this.nodes; - if ( prevState.activeIndex !== activeIndex && - Number.isInteger( activeIndex ) && - menu && menu.children[ activeIndex ] ) { - menu.children[ activeIndex ].focus(); - } - } - - render() { - const { - icon = 'menu', - label, - menuLabel, - controls, - } = this.props; - const { - open, - menuLeft, - } = this.state; - - if ( ! controls || ! controls.length ) { - return null; - } - // monitor the menu position when open - if ( open ) { - if ( this.timer === null ) { - this.timer = setInterval( this.calculateMenuPosition, 100 ); - } - } else if ( this.timer !== null ) { - clearInterval( this.timer ); - this.timer = null; - } - /* eslint-disable jsx-a11y/no-static-element-interactions */ - return ( -
- { + const openOnArrowDown = ( event ) => { + if ( ! isOpen && event.keyCode === DOWN ) { + event.stopPropagation(); + onToggle(); } - icon={ icon } - onClick={ this.toggleMenu } - aria-haspopup="true" - aria-expanded={ open } - label={ label } - ref={ this.bindReferenceNode( 'toggle' ) } - > - - - { open && -
+ + + ); + } } + renderContent={ ( { onClose, onToggle } ) => { + const closeOnEscape = ( event ) => { + if ( event.keyCode === ESCAPE ) { + event.stopPropagation(); + onToggle(); + } + }; + + return ( + { controls.map( ( control, index ) => ( { event.stopPropagation(); - this.closeMenu(); + onClose(); if ( control.onClick ) { control.onClick(); } @@ -243,17 +86,15 @@ export class DropdownMenu extends Component { className="components-dropdown-menu__menu-item" icon={ control.icon } role="menuitem" - tabIndex="-1" > { control.title } ) ) } -
- } -
- ); - /* eslint-enable jsx-a11y/no-static-element-interactions */ - } + + ); + } } + /> + ); } -export default clickOutside( DropdownMenu ); +export default DropdownMenu; diff --git a/components/dropdown-menu/style.scss b/components/dropdown-menu/style.scss index 1376eb8c48e596..830703b5e6857d 100644 --- a/components/dropdown-menu/style.scss +++ b/components/dropdown-menu/style.scss @@ -40,14 +40,13 @@ } } } +.components-dropdown-menu__popover .components-popover__content { + width: 200px; +} .components-dropdown-menu__menu { - position: absolute; - top: $block-controls-height - 1px; // note that left is set by react in a style attribute - box-shadow: $shadow-popover; - border: 1px solid $light-gray-500; - background: $white; + width: 100%; padding: 3px 3px 0 3px; font-family: $default-font; font-size: $default-font-size; diff --git a/components/dropdown-menu/test/index.js b/components/dropdown-menu/test/index.js index 189ac3521bc48c..e8cb9fbf5d1c99 100644 --- a/components/dropdown-menu/test/index.js +++ b/components/dropdown-menu/test/index.js @@ -11,9 +11,9 @@ import { keycodes } from '@wordpress/utils'; /** * Internal dependencies */ -import { DropdownMenu } from '../'; +import DropdownMenu from '../'; -const { TAB, ESCAPE, LEFT, UP, RIGHT, DOWN } = keycodes; +const { DOWN } = keycodes; describe( 'DropdownMenu', () => { let controls; @@ -55,132 +55,19 @@ describe( 'DropdownMenu', () => { expect( wrapper.type() ).toBeNull(); } ); - it( 'should render a collapsed menu button', () => { - const wrapper = shallow( - - ); - - expect( wrapper.state( 'open' ) ).toBe( false ); - expect( wrapper.state( 'activeIndex' ) ).toBeNull(); - expect( wrapper.find( '> IconButton' ).prop( 'label' ) ).toBe( 'Select a direction' ); - expect( wrapper.find( '> IconButton' ).prop( 'icon' ) ).toBe( 'menu' ); - expect( wrapper.find( '.components-dropdown-menu__menu' ) ).toHaveLength( 0 ); - } ); - - it( 'should render an expanded menu upon click', () => { - const wrapper = shallow( ); - - // Open menu - wrapper.find( '> IconButton' ).simulate( 'click' ); - - const options = wrapper.find( '.components-dropdown-menu__menu > IconButton' ); - expect( wrapper.state( 'open' ) ).toBe( true ); - expect( wrapper.state( 'activeIndex' ) ).toBe( 0 ); - expect( options ).toHaveLength( controls.length ); - expect( options.at( 0 ).prop( 'icon' ) ).toBe( 'arrow-up-alt' ); - expect( options.at( 0 ).children().text() ).toBe( 'Up' ); - } ); - it( 'should open menu on arrow down', () => { - const wrapper = shallow( ); + const wrapper = mount( ); // Close menu by keyup - wrapper.simulate( 'keydown', { + wrapper.find( '.components-dropdown-menu__toggle' ).simulate( 'keydown', { stopPropagation: () => {}, preventDefault: () => {}, keyCode: DOWN, } ); - expect( wrapper.state( 'open' ) ).toBe( true ); - } ); - - it( 'should call the control onClick callback and close menu', () => { - const wrapper = shallow( ); - - // Open menu - wrapper.find( '> IconButton' ).simulate( 'click' ); - - // Select option - const options = wrapper.find( '.components-dropdown-menu__menu > IconButton' ); - options.at( 0 ).simulate( 'click', { stopPropagation: () => {} } ); - - expect( controls[ 0 ].onClick ).toHaveBeenCalled(); - expect( wrapper.state( 'open' ) ).toBe( false ); - } ); - - it( 'should navigate by keypresses', () => { - const wrapper = shallow( ); - - // Open menu - wrapper.find( '> IconButton' ).simulate( 'click' ); - - // Navigate options - function assertKeyDown( keyCode, expectedActiveIndex ) { - wrapper.simulate( 'keydown', { - stopPropagation: () => {}, - preventDefault: () => {}, - keyCode, - } ); - - const activeIndex = wrapper.state( 'activeIndex' ); - expect( activeIndex ).toBe( expectedActiveIndex ); - } - - assertKeyDown( RIGHT, 1 ); - assertKeyDown( DOWN, 2 ); - assertKeyDown( DOWN, 3 ); - assertKeyDown( DOWN, 0 ); // Reset to beginning - assertKeyDown( DOWN, 1 ); - assertKeyDown( LEFT, 0 ); - assertKeyDown( UP, 3 ); // Reset to end - } ); - - it( 'should close menu on escape', () => { - // Mount: We need to access DOM node of rendered menu IconButton - const wrapper = mount( ); - - // Open menu - wrapper.find( '> IconButton' ).simulate( 'click' ); - - // Close menu by escape - wrapper.simulate( 'keydown', { - stopPropagation: () => {}, - preventDefault: () => {}, - keyCode: ESCAPE, - } ); - - expect( wrapper.state( 'open' ) ).toBe( false ); - } ); - - it( 'should close menu on click outside', () => { - const wrapper = shallow( ); - - // Open menu - wrapper.find( '> IconButton' ).simulate( 'click' ); - - // Close menu by click outside - wrapper.instance().handleClickOutside(); - - expect( wrapper.state( 'open' ) ).toBe( false ); - } ); - - it( 'should close menu on tab', () => { - const wrapper = shallow( ); - - // Open menu - wrapper.find( '> IconButton' ).simulate( 'click' ); - - // Close menu by tab - wrapper.simulate( 'keydown', { - stopPropagation: () => {}, - preventDefault: () => {}, - keyCode: TAB, - } ); + const popover = wrapper.find( 'Popover' ); - expect( wrapper.state( 'open' ) ).toBe( false ); + expect( popover.prop( 'isOpen' ) ).toBe( true ); } ); } ); } ); diff --git a/components/dropdown/README.md b/components/dropdown/README.md index df196372b96346..1985439b564c18 100644 --- a/components/dropdown/README.md +++ b/components/dropdown/README.md @@ -1,5 +1,5 @@ -Popover -======= +Dropdown +======== Dropdown is a React component to render a button that opens a floating content modal when clicked. This components takes care of updating the state of the dropdown menu (opened/closed), handles closing the menu when clicking outside diff --git a/components/index.js b/components/index.js index 1eb3dd2516e595..7b5facf5b442f0 100644 --- a/components/index.js +++ b/components/index.js @@ -14,6 +14,7 @@ export { default as FormToggle } from './form-toggle'; export { default as FormTokenField } from './form-token-field'; export { default as IconButton } from './icon-button'; export { default as KeyboardShortcuts } from './keyboard-shortcuts'; +export { default as NavigableMenu } from './navigable-menu'; export { default as Notice } from './notice'; export { default as NoticeList } from './notice/list'; export { default as Panel } from './panel'; diff --git a/components/navigable-menu/README.md b/components/navigable-menu/README.md new file mode 100644 index 00000000000000..202180a556fa3d --- /dev/null +++ b/components/navigable-menu/README.md @@ -0,0 +1,40 @@ +NavigableMenu +============= + +NavigableMenu is a React component to render a menu navigable using arrow keys. The children of the menu must be tabbables + +## Usage + + +```jsx +import { NavigableMenu, Button } from '@wordpress/components'; + +function MyMenu() { + return ( + + + + + + ); +} +``` + +## Props + +The component accepts the following props. Props not included in this set will be applied to the element wrapping Navigable Menu. + +### orientation + +The orientation of the menu. It could be "vertical" or "horizontal" + +- Type: `String` +- Required: No +- Default: `"vertical"` + +## onNavigate + +A callback invoked when the menu navigates to one of its children passing the index as an argument + +- Type: `Function` +- Required: No diff --git a/components/navigable-menu/index.js b/components/navigable-menu/index.js new file mode 100644 index 00000000000000..7c8861589ed4c6 --- /dev/null +++ b/components/navigable-menu/index.js @@ -0,0 +1,63 @@ +/** + * External Dependencies + */ +import { omit, noop } from 'lodash'; + +/** + * WordPress Dependencies + */ +import { Component } from '@wordpress/element'; +import { focus, keycodes } from '@wordpress/utils'; + +/** + * Module Constants + */ +const { UP, DOWN, LEFT, RIGHT } = keycodes; + +class NavigableMenu extends Component { + constructor() { + super( ...arguments ); + this.bindContainer = this.bindContainer.bind( this ); + this.onKeyDown = this.onKeyDown.bind( this ); + } + + bindContainer( ref ) { + this.container = ref; + } + + onKeyDown( event ) { + const { orientation = 'vertical', onNavigate = noop } = this.props; + if ( + ( orientation === 'vertical' && [ UP, DOWN ].indexOf( event.keyCode ) === -1 ) || + ( orientation === 'horizontal' && [ RIGHT, LEFT ].indexOf( event.keyCode ) === -1 ) + ) { + return; + } + + const tabbables = focus.tabbable + .find( this.container ) + .filter( ( node ) => node.parentElement === this.container ); + const indexOfTabbable = tabbables.indexOf( document.activeElement ); + const offset = [ UP, LEFT ].indexOf( event.keyCode ) === -1 ? 1 : -1; + const nextTabbable = tabbables[ indexOfTabbable + offset ]; + if ( indexOfTabbable !== -1 ) { + event.stopPropagation(); + if ( nextTabbable ) { + nextTabbable.focus(); + onNavigate( indexOfTabbable + offset ); + } + } + } + + render() { + const { children, ...props } = this.props; + + return ( +
+ { children } +
+ ); + } +} + +export default NavigableMenu; diff --git a/components/navigable-menu/test/index.js b/components/navigable-menu/test/index.js new file mode 100644 index 00000000000000..e14f21bedb7fcb --- /dev/null +++ b/components/navigable-menu/test/index.js @@ -0,0 +1,49 @@ +/** + * External dependencies + */ +import { mount } from 'enzyme'; + +/** + * WordPress dependencies + */ +import { keycodes } from '@wordpress/utils'; + +/** + * Internal dependencies + */ +import NavigableMenu from '../'; + +const { UP, DOWN } = keycodes; + +describe( 'NavigableMenu', () => { + // Skipping this this because the `isVisible` check in utils/focus/tabbable.js always returns false in tests + // Probbably a jsdom issue + it.skip( 'should navigate by keypresses', () => { + let currentIndex = 0; + const wrapper = mount( ( + currentIndex = index }> + + + + + ) ); + + const container = wrapper.find( 'div' ); + wrapper.find( '#btn1' ).get( 0 ).focus(); + + // Navigate options + function assertKeyDown( keyCode, expectedActiveIndex ) { + container.simulate( 'keydown', { + stopPropagation: () => {}, + preventDefault: () => {}, + keyCode, + } ); + + expect( currentIndex ).toBe( expectedActiveIndex ); + } + + assertKeyDown( DOWN, 1 ); + assertKeyDown( DOWN, 2 ); + assertKeyDown( UP, 1 ); + } ); +} ); diff --git a/editor/assets/stylesheets/_z-index.scss b/editor/assets/stylesheets/_z-index.scss index 7352b15a88e409..fde57453120ed1 100644 --- a/editor/assets/stylesheets/_z-index.scss +++ b/editor/assets/stylesheets/_z-index.scss @@ -15,7 +15,6 @@ $z-layers: ( '.editor-inserter__tab.is-active': 1, '.components-panel__header': 1, '.blocks-format-toolbar__link-modal': 1, - '.editor-block-switcher__menu': 2, '.editor-block-mover': 10, '.blocks-gallery-image__inline-menu': 10, '.editor-header': 20, diff --git a/editor/block-switcher/index.js b/editor/block-switcher/index.js index bc38d3f59dc569..b6f45a4b539d21 100644 --- a/editor/block-switcher/index.js +++ b/editor/block-switcher/index.js @@ -3,14 +3,12 @@ */ import { connect } from 'react-redux'; import { uniq, get, reduce, find } from 'lodash'; -import clickOutside from 'react-click-outside'; /** * WordPress dependencies */ import { __ } from '@wordpress/i18n'; -import { Component } from '@wordpress/element'; -import { Dashicon, IconButton } from '@wordpress/components'; +import { Dropdown, Dashicon, IconButton, Toolbar, NavigableMenu } from '@wordpress/components'; import { getBlockType, getBlockTypes, switchToBlockType } from '@wordpress/blocks'; /** @@ -20,97 +18,73 @@ import './style.scss'; import { replaceBlocks } from '../actions'; import { getBlock } from '../selectors'; -class BlockSwitcher extends Component { - constructor() { - super( ...arguments ); - this.toggleMenu = this.toggleMenu.bind( this ); - this.state = { - open: false, - }; - } - - handleClickOutside() { - if ( ! this.state.open ) { - return; - } - - this.toggleMenu(); - } - - toggleMenu() { - this.setState( ( state ) => ( { - open: ! state.open, - } ) ); - } - - switchBlockType( name ) { - return () => { - this.setState( { - open: false, - } ); - this.props.onTransform( this.props.block, name ); - }; - } - - render() { - const blockType = getBlockType( this.props.block.name ); - const blocksToBeTransformedFrom = reduce( getBlockTypes(), ( memo, block ) => { - const transformFrom = get( block, 'transforms.from', [] ); - const transformation = find( transformFrom, t => t.type === 'block' && t.blocks.indexOf( this.props.block.name ) !== -1 ); - return transformation ? memo.concat( [ block.name ] ) : memo; +function BlockSwitcher( { block, onTransform } ) { + const blockType = getBlockType( block.name ); + const blocksToBeTransformedFrom = reduce( getBlockTypes(), ( memo, type ) => { + const transformFrom = get( type, 'transforms.from', [] ); + const transformation = find( transformFrom, t => t.type === 'block' && t.blocks.indexOf( block.name ) !== -1 ); + return transformation ? memo.concat( [ type.name ] ) : memo; + }, [] ); + const blocksToBeTransformedTo = get( blockType, 'transforms.to', [] ) + .reduce( ( memo, transformation ) => memo.concat( transformation.blocks ), [] ); + const allowedBlocks = uniq( blocksToBeTransformedFrom.concat( blocksToBeTransformedTo ) ) + .reduce( ( memo, name ) => { + const type = getBlockType( name ); + return !! type ? memo.concat( type ) : memo; }, [] ); - const blocksToBeTransformedTo = get( blockType, 'transforms.to', [] ) - .reduce( ( memo, transformation ) => memo.concat( transformation.blocks ), [] ); - const allowedBlocks = uniq( blocksToBeTransformedFrom.concat( blocksToBeTransformedTo ) ) - .reduce( ( memo, name ) => { - const block = getBlockType( name ); - return !! block ? memo.concat( block ) : memo; - }, [] ); - if ( ! allowedBlocks.length ) { - return null; - } + if ( ! allowedBlocks.length ) { + return null; + } - return ( -
- ( + + + + + + ) } + renderContent={ ( { onClose } ) => ( + - - - { this.state.open && -
- + { allowedBlocks.map( ( { name, title, icon } ) => ( + { + onTransform( block, name ); + onClose(); + } } + className="editor-block-switcher__menu-item" + icon={ icon } + role="menuitem" > - { __( 'Transform into:' ) } - - { allowedBlocks.map( ( { name, title, icon } ) => ( - - { title } - - ) ) } -
- } -
- ); - } + { title } + + ) ) } + + ) } + /> + ); } export default connect( @@ -125,4 +99,4 @@ export default connect( ) ); }, } ) -)( clickOutside( BlockSwitcher ) ); +)( BlockSwitcher ); diff --git a/editor/block-switcher/style.scss b/editor/block-switcher/style.scss index 2f7f428b3f543b..bf1f97f1fa6f7f 100644 --- a/editor/block-switcher/style.scss +++ b/editor/block-switcher/style.scss @@ -1,4 +1,5 @@ .editor-block-switcher { + position: relative; border: 1px solid $light-gray-500; background-color: $white; font-family: $default-font; @@ -22,14 +23,10 @@ } .editor-block-switcher__menu { - position: absolute; - top: $block-controls-height - 1px; - left: 0; box-shadow: $shadow-popover; border: 1px solid $light-gray-500; background: $white; padding: 3px 3px 0 3px; - z-index: z-index( '.editor-block-switcher__menu' ); } .editor-block-switcher__menu-title { From 69e019678b78399f3d07f02427728e2d8222bb67 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 12 Oct 2017 14:10:32 +0100 Subject: [PATCH 2/8] Block switcher: Fix width and tabIndex --- editor/block-switcher/index.js | 1 - editor/block-switcher/style.scss | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/editor/block-switcher/index.js b/editor/block-switcher/index.js index b6f45a4b539d21..a860452351a2af 100644 --- a/editor/block-switcher/index.js +++ b/editor/block-switcher/index.js @@ -59,7 +59,6 @@ function BlockSwitcher( { block, onTransform } ) { Date: Thu, 12 Oct 2017 14:24:47 +0100 Subject: [PATCH 3/8] Components: Loop arrow navigation in NavigableeMenu --- components/dropdown-menu/index.js | 2 ++ components/navigable-menu/index.js | 19 ++++++++++++------- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/components/dropdown-menu/index.js b/components/dropdown-menu/index.js index 5e742be9ee43b1..d5a66f3aad9be2 100644 --- a/components/dropdown-menu/index.js +++ b/components/dropdown-menu/index.js @@ -36,6 +36,7 @@ function DropdownMenu( { renderToggle={ ( { isOpen, onToggle } ) => { const openOnArrowDown = ( event ) => { if ( ! isOpen && event.keyCode === DOWN ) { + event.preventDefault(); event.stopPropagation(); onToggle(); } @@ -61,6 +62,7 @@ function DropdownMenu( { renderContent={ ( { onClose, onToggle } ) => { const closeOnEscape = ( event ) => { if ( event.keyCode === ESCAPE ) { + event.preventDefault(); event.stopPropagation(); onToggle(); } diff --git a/components/navigable-menu/index.js b/components/navigable-menu/index.js index 7c8861589ed4c6..32f335305fd51d 100644 --- a/components/navigable-menu/index.js +++ b/components/navigable-menu/index.js @@ -38,14 +38,19 @@ class NavigableMenu extends Component { .find( this.container ) .filter( ( node ) => node.parentElement === this.container ); const indexOfTabbable = tabbables.indexOf( document.activeElement ); + if ( indexOfTabbable === -1 ) { + return; + } const offset = [ UP, LEFT ].indexOf( event.keyCode ) === -1 ? 1 : -1; - const nextTabbable = tabbables[ indexOfTabbable + offset ]; - if ( indexOfTabbable !== -1 ) { - event.stopPropagation(); - if ( nextTabbable ) { - nextTabbable.focus(); - onNavigate( indexOfTabbable + offset ); - } + let nextIndex = indexOfTabbable + offset; + nextIndex = nextIndex === -1 ? tabbables.length - 1 : nextIndex; + nextIndex = nextIndex === tabbables.length ? 0 : nextIndex; + const nextTabbable = tabbables[ nextIndex ]; + event.stopPropagation(); + event.preventDefault(); + if ( nextTabbable ) { + nextTabbable.focus(); + onNavigate( nextIndex ); } } From a33487248efe8381c47ad76022b275cc9f89ea78 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 12 Oct 2017 15:31:07 +0100 Subject: [PATCH 4/8] Block Switcher: Fix toggle styling --- editor/block-switcher/style.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/editor/block-switcher/style.scss b/editor/block-switcher/style.scss index 73529dc2ef3637..233d3de4971bcd 100644 --- a/editor/block-switcher/style.scss +++ b/editor/block-switcher/style.scss @@ -6,6 +6,7 @@ font-size: $default-font-size; line-height: $default-line-height; margin-right: -1px; + margin-bottom: -1px; } .editor-block-switcher__toggle { From 1be023ddca4e67de0c66ab17f7260b213467f05c Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 12 Oct 2017 15:58:59 +0100 Subject: [PATCH 5/8] Block Switcher: Open block switcher on arrow down --- editor/block-switcher/index.js | 45 +++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/editor/block-switcher/index.js b/editor/block-switcher/index.js index a860452351a2af..dc1d4ffd84dac1 100644 --- a/editor/block-switcher/index.js +++ b/editor/block-switcher/index.js @@ -10,6 +10,7 @@ import { uniq, get, reduce, find } from 'lodash'; import { __ } from '@wordpress/i18n'; import { Dropdown, Dashicon, IconButton, Toolbar, NavigableMenu } from '@wordpress/components'; import { getBlockType, getBlockTypes, switchToBlockType } from '@wordpress/blocks'; +import { keycodes } from '@wordpress/utils'; /** * Internal dependencies @@ -18,6 +19,11 @@ import './style.scss'; import { replaceBlocks } from '../actions'; import { getBlock } from '../selectors'; +/** + * Module Constants + */ +const { DOWN } = keycodes; + function BlockSwitcher( { block, onTransform } ) { const blockType = getBlockType( block.name ); const blocksToBeTransformedFrom = reduce( getBlockTypes(), ( memo, type ) => { @@ -41,20 +47,31 @@ function BlockSwitcher( { block, onTransform } ) { ( - - - - - - ) } + renderToggle={ ( { onToggle, isOpen } ) => { + const openOnArrowDown = ( event ) => { + if ( ! isOpen && event.keyCode === DOWN ) { + event.preventDefault(); + event.stopPropagation(); + onToggle(); + } + }; + + return ( + + + + + + ); + } } renderContent={ ( { onClose } ) => ( Date: Thu, 12 Oct 2017 15:59:20 +0100 Subject: [PATCH 6/8] DropdownMenu: Avoid conflicting ESCAPE handlers --- components/dropdown-menu/index.js | 13 ++----------- components/popover/index.js | 1 + 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/components/dropdown-menu/index.js b/components/dropdown-menu/index.js index d5a66f3aad9be2..0fc9cfc756dc39 100644 --- a/components/dropdown-menu/index.js +++ b/components/dropdown-menu/index.js @@ -17,7 +17,7 @@ import Dashicon from '../dashicon'; import Dropdown from '../dropdown'; import NavigableMenu from '../navigable-menu'; -const { ESCAPE, DOWN } = keycodes; +const { DOWN } = keycodes; function DropdownMenu( { icon = 'menu', @@ -59,21 +59,12 @@ function DropdownMenu( { ); } } - renderContent={ ( { onClose, onToggle } ) => { - const closeOnEscape = ( event ) => { - if ( event.keyCode === ESCAPE ) { - event.preventDefault(); - event.stopPropagation(); - onToggle(); - } - }; - + renderContent={ ( { onClose } ) => { return ( { controls.map( ( control, index ) => ( Date: Thu, 12 Oct 2017 18:47:36 +0100 Subject: [PATCH 7/8] Block Switcher: The navigable menu should only wrap the menu elements --- editor/block-switcher/index.js | 41 +++++++++++++++++----------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/editor/block-switcher/index.js b/editor/block-switcher/index.js index dc1d4ffd84dac1..7d78f6082ce957 100644 --- a/editor/block-switcher/index.js +++ b/editor/block-switcher/index.js @@ -73,31 +73,32 @@ function BlockSwitcher( { block, onTransform } ) { ); } } renderContent={ ( { onClose } ) => ( - +
{ __( 'Transform into:' ) } - { allowedBlocks.map( ( { name, title, icon } ) => ( - { - onTransform( block, name ); - onClose(); - } } - className="editor-block-switcher__menu-item" - icon={ icon } - role="menuitem" - > - { title } - - ) ) } - + + { allowedBlocks.map( ( { name, title, icon } ) => ( + { + onTransform( block, name ); + onClose(); + } } + className="editor-block-switcher__menu-item" + icon={ icon } + role="menuitem" + > + { title } + + ) ) } + +
) } /> ); From 9de328b8ed663ecb66982b1c223a81ab7518ed5a Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 13 Oct 2017 08:14:06 +0100 Subject: [PATCH 8/8] NavigableMenu: Restrict Tabbing within the navigable menu --- components/navigable-menu/index.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/components/navigable-menu/index.js b/components/navigable-menu/index.js index 32f335305fd51d..11ea3cbe1ebb18 100644 --- a/components/navigable-menu/index.js +++ b/components/navigable-menu/index.js @@ -12,7 +12,7 @@ import { focus, keycodes } from '@wordpress/utils'; /** * Module Constants */ -const { UP, DOWN, LEFT, RIGHT } = keycodes; +const { UP, DOWN, LEFT, RIGHT, TAB } = keycodes; class NavigableMenu extends Component { constructor() { @@ -28,8 +28,8 @@ class NavigableMenu extends Component { onKeyDown( event ) { const { orientation = 'vertical', onNavigate = noop } = this.props; if ( - ( orientation === 'vertical' && [ UP, DOWN ].indexOf( event.keyCode ) === -1 ) || - ( orientation === 'horizontal' && [ RIGHT, LEFT ].indexOf( event.keyCode ) === -1 ) + ( orientation === 'vertical' && [ UP, DOWN, TAB ].indexOf( event.keyCode ) === -1 ) || + ( orientation === 'horizontal' && [ RIGHT, LEFT, TAB ].indexOf( event.keyCode ) === -1 ) ) { return; } @@ -41,7 +41,11 @@ class NavigableMenu extends Component { if ( indexOfTabbable === -1 ) { return; } - const offset = [ UP, LEFT ].indexOf( event.keyCode ) === -1 ? 1 : -1; + + const offset = ( + [ UP, LEFT ].indexOf( event.keyCode ) !== -1 || + ( event.keyCode === TAB && event.shiftKey ) + ) ? -1 : 1; let nextIndex = indexOfTabbable + offset; nextIndex = nextIndex === -1 ? tabbables.length - 1 : nextIndex; nextIndex = nextIndex === tabbables.length ? 0 : nextIndex;