From 45ead3fa8d64586ae7f08bf062261721640a021b Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 7 Sep 2017 09:25:27 -0400 Subject: [PATCH 1/4] Sidebar: Close schedule by Popover prop Click outside is handled internally by the Popover component --- editor/sidebar/post-schedule/index.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/editor/sidebar/post-schedule/index.js b/editor/sidebar/post-schedule/index.js index 2c7470e6bb271..51b81ffad80cb 100644 --- a/editor/sidebar/post-schedule/index.js +++ b/editor/sidebar/post-schedule/index.js @@ -2,7 +2,6 @@ * External dependencies */ import { connect } from 'react-redux'; -import clickOutside from 'react-click-outside'; import DatePicker from 'react-datepicker'; import moment from 'moment'; import { flowRight } from 'lodash'; @@ -26,17 +25,20 @@ import { editPost } from '../../actions'; export class PostSchedule extends Component { constructor() { super( ...arguments ); + + this.toggleDialog = this.toggleDialog.bind( this ); + this.closeDialog = this.closeDialog.bind( this ); + this.state = { opened: false, }; - this.toggleDialog = this.toggleDialog.bind( this ); } toggleDialog() { this.setState( ( state ) => ( { opened: ! state.opened } ) ); } - handleClickOutside() { + closeDialog() { this.setState( { opened: false } ); } @@ -77,6 +79,7 @@ export class PostSchedule extends Component { { export default flowRight( applyConnect, - applyWithAPIData, - clickOutside + applyWithAPIData )( PostSchedule ); From deb86a78f14196dfe20fa5297a3a7970dd82a8b8 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 7 Sep 2017 09:28:46 -0400 Subject: [PATCH 2/4] Components: Split popover onClose and onClickOutside onClickOutside passes event which is relied upon by PostVisibility, but we don't want to guarantee this with signature of onClose --- components/popover/README.md | 14 ++++++++++++++ components/popover/index.js | 20 ++++++++++++++------ editor/inserter/index.js | 2 +- editor/sidebar/post-visibility/index.js | 12 ++++++++++-- 4 files changed, 39 insertions(+), 9 deletions(-) diff --git a/components/popover/README.md b/components/popover/README.md index fd7e2f464a87e..0497ecb00fb86 100644 --- a/components/popover/README.md +++ b/components/popover/README.md @@ -68,3 +68,17 @@ An optional additional class name to apply to the rendered popover. - Type: `String` - Required: No + +## onClose + +A callback invoked when the popover should be closed. + +- Type: `Function` +- Required: No + +## onClickOutside + +A callback invoked when the user clicks outside the opened popover, passing the click event. The popover should be closed in response to this interaction. Defaults to `onClose`. + +- Type: `Function` +- Required: No diff --git a/components/popover/index.js b/components/popover/index.js index 027d52ec0c43c..d403cf6a09004 100644 --- a/components/popover/index.js +++ b/components/popover/index.js @@ -178,11 +178,19 @@ export class Popover extends Component { } render() { - // Disable reason: We generate the `...contentProps` rest as remainder - // of props which aren't explicitly handled by this component. - // - // eslint-disable-next-line no-unused-vars - const { isOpen, onClose, position, children, className, ...contentProps } = this.props; + const { + isOpen, + onClose, + onClickOutside = onClose, + // Disable reason: We generate the `...contentProps` rest as remainder + // of props which aren't explicitly handled by this component. + // + // eslint-disable-next-line no-unused-vars + position, + children, + className, + ...contentProps + } = this.props; const [ yAxis, xAxis ] = this.getPositions(); if ( ! isOpen ) { @@ -200,7 +208,7 @@ export class Popover extends Component { return ( { createPortal( - +
diff --git a/editor/sidebar/post-visibility/index.js b/editor/sidebar/post-visibility/index.js index 90d9e18370709..21c6149826364 100644 --- a/editor/sidebar/post-visibility/index.js +++ b/editor/sidebar/post-visibility/index.js @@ -28,6 +28,7 @@ export class PostVisibility extends Component { this.toggleDialog = this.toggleDialog.bind( this ); this.stopPropagation = this.stopPropagation.bind( this ); this.closeOnClickOutside = this.closeOnClickOutside.bind( this ); + this.close = this.close.bind( this ); this.setPublic = this.setPublic.bind( this ); this.setPrivate = this.setPrivate.bind( this ); this.setPasswordProtected = this.setPasswordProtected.bind( this ); @@ -48,8 +49,14 @@ export class PostVisibility extends Component { } closeOnClickOutside( event ) { + if ( ! this.buttonNode.contains( event.target ) ) { + this.close(); + } + } + + close() { const { opened } = this.state; - if ( opened && ! this.buttonNode.contains( event.target ) ) { + if ( opened ) { this.toggleDialog(); } } @@ -133,7 +140,8 @@ export class PostVisibility extends Component { From be48f3aba6a6d934147090d4737465501cfe7cdd Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 7 Sep 2017 09:33:01 -0400 Subject: [PATCH 3/4] Components: Apply withFocusReturn to Popover --- components/popover/detect-outside.js | 11 ++++++++++- editor/inserter/menu.js | 3 +-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/components/popover/detect-outside.js b/components/popover/detect-outside.js index a890eb664fcac..476932d62d57e 100644 --- a/components/popover/detect-outside.js +++ b/components/popover/detect-outside.js @@ -2,12 +2,18 @@ * External dependencies */ import clickOutside from 'react-click-outside'; +import { flowRight } from 'lodash'; /** * WordPress dependencies */ import { Component } from '@wordpress/element'; +/** + * Internal dependencies + */ +import withFocusReturn from '../higher-order/with-focus-return'; + class PopoverDetectOutside extends Component { handleClickOutside( event ) { const { onClickOutside } = this.props; @@ -21,4 +27,7 @@ class PopoverDetectOutside extends Component { } } -export default clickOutside( PopoverDetectOutside ); +export default flowRight( [ + withFocusReturn, + clickOutside, +] )( PopoverDetectOutside ); diff --git a/editor/inserter/menu.js b/editor/inserter/menu.js index 5ad4ec4ee0dba..33e63f85c3c1e 100644 --- a/editor/inserter/menu.js +++ b/editor/inserter/menu.js @@ -9,7 +9,7 @@ import { connect } from 'react-redux'; */ import { __, _n, sprintf } from '@wordpress/i18n'; import { Component } from '@wordpress/element'; -import { withFocusReturn, withInstanceId, withSpokenMessages } from '@wordpress/components'; +import { withInstanceId, withSpokenMessages } from '@wordpress/components'; import { keycodes } from '@wordpress/utils'; import { getCategories, getBlockTypes, BlockIcon } from '@wordpress/blocks'; @@ -411,6 +411,5 @@ const connectComponent = connect( export default flow( withInstanceId, withSpokenMessages, - withFocusReturn, connectComponent )( InserterMenu ); From 9c539d002b5e1fea3c4025fdfc316387b81ed54f Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 7 Sep 2017 09:37:53 -0400 Subject: [PATCH 4/4] Components: Close popover by escape keypress --- components/popover/index.js | 29 +++++++++++++++++++++++++++-- editor/inserter/index.js | 20 ++++++++++---------- editor/inserter/menu.js | 6 +----- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/components/popover/index.js b/components/popover/index.js index d403cf6a09004..085a1db36b507 100644 --- a/components/popover/index.js +++ b/components/popover/index.js @@ -8,7 +8,7 @@ import { isEqual, noop } from 'lodash'; * WordPress dependencies */ import { createPortal, Component } from '@wordpress/element'; -import { focus } from '@wordpress/utils'; +import { focus, keycodes } from '@wordpress/utils'; /** * Internal dependencies @@ -16,8 +16,12 @@ import { focus } from '@wordpress/utils'; import './style.scss'; import PopoverDetectOutside from './detect-outside'; +const { ESCAPE } = keycodes; + /** - * module constants + * Offset by which popover should adjust horizontally to account for tail. + * + * @type {Number} */ const ARROW_OFFSET = 20; @@ -29,6 +33,7 @@ export class Popover extends Component { this.bindNode = this.bindNode.bind( this ); this.setOffset = this.setOffset.bind( this ); this.throttledSetOffset = this.throttledSetOffset.bind( this ); + this.maybeClose = this.maybeClose.bind( this ); this.nodes = {}; @@ -173,6 +178,20 @@ export class Popover extends Component { ]; } + maybeClose( event ) { + const { onKeyDown, onClose } = this.props; + + // Close on escape + if ( event.keyCode === ESCAPE && onClose ) { + onClose(); + } + + // Preserve original content prop behavior + if ( onKeyDown ) { + onKeyDown( event ); + } + } + bindNode( name ) { return ( node ) => this.nodes[ name ] = node; } @@ -205,6 +224,10 @@ export class Popover extends Component { 'is-' + xAxis, ); + // Disable reason: We care to capture the _bubbled_ events from inputs + // within popover as inferring close intent. + + /* eslint-disable jsx-a11y/no-static-element-interactions */ return ( { createPortal( @@ -214,6 +237,7 @@ export class Popover extends Component { className={ classes } tabIndex="0" { ...contentProps } + onKeyDown={ this.maybeClose } >
); + /* eslint-enable jsx-a11y/no-static-element-interactions */ } } diff --git a/editor/inserter/index.js b/editor/inserter/index.js index 2a1b3e21db5c9..6929b1e3d8ac3 100644 --- a/editor/inserter/index.js +++ b/editor/inserter/index.js @@ -56,16 +56,15 @@ class Inserter extends Component { } insertBlock( name ) { - if ( name ) { - const { - insertionPoint, - onInsertBlock, - } = this.props; - onInsertBlock( - name, - insertionPoint - ); - } + const { + insertionPoint, + onInsertBlock, + } = this.props; + + onInsertBlock( + name, + insertionPoint + ); this.close(); } @@ -89,6 +88,7 @@ class Inserter extends Component { diff --git a/editor/inserter/menu.js b/editor/inserter/menu.js index 33e63f85c3c1e..e4af834aca28b 100644 --- a/editor/inserter/menu.js +++ b/editor/inserter/menu.js @@ -20,7 +20,7 @@ import './style.scss'; import { getBlocks, getRecentlyUsedBlocks } from '../selectors'; import { showInsertionPoint, hideInsertionPoint } from '../actions'; -const { TAB, ESCAPE, LEFT, UP, RIGHT, DOWN } = keycodes; +const { TAB, LEFT, UP, RIGHT, DOWN } = keycodes; export const searchBlocks = ( blocks, searchTerm ) => { const normalizedSearchTerm = searchTerm.toLowerCase().trim(); @@ -233,11 +233,7 @@ export class InserterMenu extends Component { keydown.preventDefault(); this.focusNext( this ); break; - case ESCAPE: - keydown.preventDefault(); - this.props.onSelect( null ); - break; case LEFT: if ( this.state.currentFocus === 'search' ) { return;