From bf9f2871fc01f8c5af0d57a0886d3be5dd4f00bc Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 18 Apr 2018 22:15:21 +0200 Subject: [PATCH 01/67] Initial implementation modal --- .../higher-order/with-focus-contain/index.js | 53 ++++++++ components/index.js | 1 + components/modal/README.md | 125 ++++++++++++++++++ components/modal/aria-helper.js | 19 +++ components/modal/index.js | 110 +++++++++++++++ components/modal/modal-content.js | 111 ++++++++++++++++ components/modal/style.scss | 9 ++ edit-post/assets/stylesheets/_z-index.scss | 4 + 8 files changed, 432 insertions(+) create mode 100644 components/higher-order/with-focus-contain/index.js create mode 100644 components/modal/README.md create mode 100644 components/modal/aria-helper.js create mode 100644 components/modal/index.js create mode 100644 components/modal/modal-content.js create mode 100644 components/modal/style.scss diff --git a/components/higher-order/with-focus-contain/index.js b/components/higher-order/with-focus-contain/index.js new file mode 100644 index 00000000000000..999c623e581416 --- /dev/null +++ b/components/higher-order/with-focus-contain/index.js @@ -0,0 +1,53 @@ +/** + * WordPress dependencies + */ +import { Component, createRef } from '@wordpress/element'; +import { focus } from '@wordpress/utils'; + +const withFocusContain = ( WrappedComponent ) => { + return class extends Component { + constructor() { + super( ...arguments ); + + this.focusContainRef = createRef(); + this.handleTabBehaviour = this.handleTabBehaviour.bind( this ); + } + + handleTabBehaviour( event ) { + if ( event.keyCode === 9 ) { + const tabbables = focus.tabbable.find( this.focusContainRef.current ); + if ( ! tabbables.length ) { + return; + } + const firstTabbable = tabbables[ 0 ]; + const lastTabbable = tabbables[ tabbables.length - 1 ]; + + if ( event.shiftKey && event.target === firstTabbable ) { + event.preventDefault(); + return lastTabbable.focus(); + } else if ( ! event.shiftKey && event.target === lastTabbable ) { + event.preventDefault(); + return firstTabbable.focus(); + } + } + } + + componentDidMount() { + this.focusContainRef.current.addEventListener( 'keydown', this.handleTabBehaviour ); + } + + componentWillUnmount() { + this.focusContainRef.current.addEventListener( 'keydown', this.handleTabBehaviour ); + } + + render() { + return ( +
+ +
+ ); + } + }; +}; + +export default withFocusContain; diff --git a/components/index.js b/components/index.js index 38731f65401287..1933ec52135ddf 100644 --- a/components/index.js +++ b/components/index.js @@ -25,6 +25,7 @@ export { default as KeyboardShortcuts } from './keyboard-shortcuts'; export { default as MenuGroup } from './menu-group'; export { default as MenuItem } from './menu-item'; export { default as MenuItemsChoice } from './menu-items-choice'; +export { default as Modal } from './modal'; export { default as ScrollLock } from './scroll-lock'; export { NavigableMenu, TabbableContainer } from './navigable-container'; export { default as Notice } from './notice'; diff --git a/components/modal/README.md b/components/modal/README.md new file mode 100644 index 00000000000000..313fadd568665a --- /dev/null +++ b/components/modal/README.md @@ -0,0 +1,125 @@ +RangeControl +======= + +The modal is used to create an accessible modal over an application. + +**Note:** The API for this modal has been mimicked to resemble `react-modal`. + +## Usage + +Render a screen overlay with a modal on top. +```js +// When the app element is set it puts an aria-hidden="true" to the provided node. +Modal.setAppElement( document.getElementById( 'wpwrap' ).parentNode ) +``` +```jsx + { + return document.getElementById( 'wpwrap' ); + } ) + > + + + + + +``` + +## Props + +The set of props accepted by the component will be specified below. +Props not included in this set will be applied to the input elements. + +### onRequestClose + +This function is called to indicate that the modal should be closed. + +- Type: `function` +- Required: Yes + +### contentLabel + +If this property is added, it will be added to the modal content `div` as `aria-label`. +You are encouraged to use this if `aria.labelledby` is not provided. + +- Type: `String` +- Required: No + +### aria.labelledby + +If this property is added, it will be added to the modal content `div` as `aria-labelledby`. +You are encouraged to use this when the modal is visually labelled. + +- Type: `String` +- Required: No + +### aria.describedby + +If this property is added, it will be added to the modal content `div` as `aria-describedby`. + +- Type: `String` +- Required: No + +### focusOnMount + +If this property is true, it will focus the first tabbable element rendered in the modal. + +- Type: `bool` +- Required: No +- Default: true + +### shouldCloseOnEsc + +If this property is added, it will determine whether the modal requests to close when the escape key is pressed. + +- Type: `bool` +- Required: No +- Default: true + +### shouldCloseOnClickOutside + +If this property is added, it will determine whether the modal requests to close when a mouse click occurs outside of the modal content. + +- Type: `bool` +- Required: No +- Default: true + +### parentSelector + +If this property is added, it overrides the default behaviour for selecting the dom node where the modal should mount. + +- Type: `function` +- Required: No +- Default: `() => document.body` + +### style.content + +If this property is added, it will add inline styles to the modal content `div`. + +- Type: `Object` +- Required: No + +### style.overlay + +If this property is added, it will add inline styles to the modal overlay `div`. + +- Type: `Object` +- Required: No + +### className + +If this property is added, it will an additional class name to the modal content `div`. + +- Type: `String` +- Required: No + +### overlayClassName + +If this property is added, it will an additional class name to the modal overlay `div`. + +- Type: `String` +- Required: No diff --git a/components/modal/aria-helper.js b/components/modal/aria-helper.js new file mode 100644 index 00000000000000..c7eacb4fa3b2a1 --- /dev/null +++ b/components/modal/aria-helper.js @@ -0,0 +1,19 @@ +let appElement = null; + +export function setAppElement( node ) { + if ( ! appElement ) { + appElement = node; + } +} + +export function hideApp() { + if ( appElement ) { + appElement.setAttribute( 'aria-hidden', 'true' ); + } +} + +export function showApp() { + if ( appElement ) { + appElement.removeAttribute( 'aria-hidden' ); + } +} diff --git a/components/modal/index.js b/components/modal/index.js new file mode 100644 index 00000000000000..e637341c4c92d4 --- /dev/null +++ b/components/modal/index.js @@ -0,0 +1,110 @@ +/** + * External dependencies + */ +import classnames from 'classnames'; +import { noop } from 'lodash'; + +/** + * WordPress dependencies + */ +import { Component, createPortal } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import ModalContent from './modal-content'; +import * as ariaHelper from './aria-helper'; +import './style.scss'; + +let modalCount = 0; + +function getParentElement( parentSelector ) { + return parentSelector ? parentSelector() : document.body; +} + +class Modal extends Component { + static setAppElement( node ) { + ariaHelper.setAppElement( node ); + } + + componentDidMount() { + modalCount++; + ariaHelper.hideApp(); + + getParentElement( + this.props.parentSelector + ).appendChild( this.node ); + } + + componentWillUnmount() { + modalCount--; + if ( modalCount === 0 ) { + ariaHelper.showApp(); + } + + getParentElement( + this.props.parentSelector + ).removeChild( this.node ); + ariaHelper.showApp(); + } + + render() { + this.props.style = this.props.style || {}; + const { + overlayClassName, + className, + style: { + content, + overlay, + }, + children, + ...otherProps + } = this.props; + + if ( ! this.node ) { + this.node = document.createElement( 'div' ); + } + + return createPortal( +
+ + { children } + +
, + this.node + ); + } +} + +Modal.defaultProps = { + className: null, + overlayClassName: null, + onRequestClose: noop, + focusOnMount: true, + shouldCloseOnEsc: true, + shouldCloseOnClickOutside: true, + parentSelector: () => document.body, + style: { + content: null, + overlay: null, + }, + /* accessibility */ + contentLabel: null, + aria: { + labelledby: null, + describedby: null, + }, +}; + +export default Modal; diff --git a/components/modal/modal-content.js b/components/modal/modal-content.js new file mode 100644 index 00000000000000..3a223f620bf81c --- /dev/null +++ b/components/modal/modal-content.js @@ -0,0 +1,111 @@ +/** + * External dependencies + */ +import clickOutside from 'react-click-outside'; +import { defer } from 'lodash'; + +/** + * WordPress dependencies + */ +import { Component, compose, createRef } from '@wordpress/element'; +import { focus } from '@wordpress/utils'; + +/** + * Internal dependencies + */ +import './style.scss'; +import withFocusReturn from '../higher-order/with-focus-return'; +import withFocusContain from '../higher-order/with-focus-contain'; + +const ESC_KEY = 27; + +class ModalContent extends Component { + constructor() { + super( ...arguments ); + + this.containerRef = createRef(); + this.handleKeyPressEvents = this.handleKeyPressEvents.bind( this ); + } + + componentDidMount() { + // Focus on mount + if ( this.props.focusOnMount ) { + this.focusFirstTabbable(); + } + // Key events + window.addEventListener( 'keydown', this.handleKeyPressEvents ); + } + + componentWillUnmount() { + window.removeEventListener( 'keydown', this.handleKeyPressEvents ); + } + + focusFirstTabbable() { + // Required because the node is appended to the DOM after rendering. + defer( () => { + const tabbables = focus.tabbable.find( this.containerRef.current ); + if ( tabbables.length ) { + tabbables[ 0 ].focus(); + } + } ); + } + + handleClickOutside( event ) { + if ( this.props.shouldCloseOnClickOutside ) { + this.onRequestClose( event ); + } + } + + handleKeyPressEvents( event ) { + if ( event.keyCode === ESC_KEY ) { + this.handleEscapePress( event ); + } + } + + handleEscapePress( event ) { + if ( this.props.shouldCloseOnEsc ) { + event.preventDefault(); + this.onRequestClose( event ); + } + } + + onRequestClose( event ) { + const { onRequestClose } = this.props; + if ( onRequestClose ) { + onRequestClose( event ); + } + } + + render() { + const { + contentLabel, + aria: { + describedby, + labelledby, + }, + children, + className, + style, + } = this.props; + + return ( +
+ { children } +
+ ); + } +} + +export default compose( [ + withFocusReturn, + withFocusContain, + clickOutside, +] )( ModalContent ); diff --git a/components/modal/style.scss b/components/modal/style.scss new file mode 100644 index 00000000000000..be74e9a01aae57 --- /dev/null +++ b/components/modal/style.scss @@ -0,0 +1,9 @@ +.components-modal { + &__screen-overlay { + z-index: z-index( '.components-modal__screen-overlay' ); + } + + &__content { + z-index: z-index( '.components-modal__content' ); + } +} diff --git a/edit-post/assets/stylesheets/_z-index.scss b/edit-post/assets/stylesheets/_z-index.scss index 7c509a1867ace5..5c8f947ecf3fe1 100644 --- a/edit-post/assets/stylesheets/_z-index.scss +++ b/edit-post/assets/stylesheets/_z-index.scss @@ -60,6 +60,10 @@ $z-layers: ( // #adminmenuwrap { z-index: 9990 } '.components-notice-list': 9989, + // Show modal under the wp-admin menus and the popover + '.components-modal__screen-overlay': 100000, + '.components-modal__content': 100001, + // Show popovers above wp-admin menus and submenus and sidebar: // #adminmenuwrap { z-index: 9990 } '.components-popover': 1000000, From aba96360fd446cbd3f3ed4aaf0df32acfed8dcbb Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Thu, 19 Apr 2018 12:09:20 +0200 Subject: [PATCH 02/67] removed style prop assignment causing error --- components/modal/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/components/modal/index.js b/components/modal/index.js index e637341c4c92d4..7f4ca1ceeeea35 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -49,7 +49,6 @@ class Modal extends Component { } render() { - this.props.style = this.props.style || {}; const { overlayClassName, className, From b0700fb13b3075a66678893929cce2db436e4238 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Mon, 23 Apr 2018 10:07:33 +0200 Subject: [PATCH 03/67] Set default mount node to #wpwrap --- components/modal/README.md | 8 -------- components/modal/index.js | 38 +++++++++++++++++++++++++------------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/components/modal/README.md b/components/modal/README.md index 313fadd568665a..cc4c6390140fd7 100644 --- a/components/modal/README.md +++ b/components/modal/README.md @@ -88,14 +88,6 @@ If this property is added, it will determine whether the modal requests to close - Required: No - Default: true -### parentSelector - -If this property is added, it overrides the default behaviour for selecting the dom node where the modal should mount. - -- Type: `function` -- Required: No -- Default: `() => document.body` - ### style.content If this property is added, it will add inline styles to the modal content `div`. diff --git a/components/modal/index.js b/components/modal/index.js index 7f4ca1ceeeea35..fe70118bfab459 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -16,36 +16,40 @@ import ModalContent from './modal-content'; import * as ariaHelper from './aria-helper'; import './style.scss'; +// Used to count the number of open modals. let modalCount = 0; -function getParentElement( parentSelector ) { - return parentSelector ? parentSelector() : document.body; -} +let parentElement; class Modal extends Component { static setAppElement( node ) { ariaHelper.setAppElement( node ); } + static setParentElement( node ) { + if ( ! parentElement ) { + parentElement = node; + } + } + componentDidMount() { modalCount++; - ariaHelper.hideApp(); - getParentElement( - this.props.parentSelector - ).appendChild( this.node ); + if ( ! this.parentElement ) { + setElements(); + } + + ariaHelper.hideApp(); + parentElement.appendChild( this.node ); } componentWillUnmount() { modalCount--; + if ( modalCount === 0 ) { ariaHelper.showApp(); } - - getParentElement( - this.props.parentSelector - ).removeChild( this.node ); - ariaHelper.showApp(); + parentElement.removeChild( this.node ); } render() { @@ -93,7 +97,6 @@ Modal.defaultProps = { focusOnMount: true, shouldCloseOnEsc: true, shouldCloseOnClickOutside: true, - parentSelector: () => document.body, style: { content: null, overlay: null, @@ -106,4 +109,13 @@ Modal.defaultProps = { }, }; +function setElements() { + const wpwrapEl = document.getElementById( 'wpwrap' ); + + if ( wpwrapEl ) { + Modal.setAppElement( wpwrapEl ); + Modal.setParentElement( wpwrapEl.parentNode ); + } +} + export default Modal; From 66367ddf2ea546a058d985ba132732e9f0ebd888 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 25 Apr 2018 11:33:38 +0200 Subject: [PATCH 04/67] Implemented default styling --- components/modal/README.md | 2 +- components/modal/header.js | 29 +++++++++++ components/modal/index.js | 27 +++++++++-- components/modal/style.scss | 96 +++++++++++++++++++++++++++++++++++++ 4 files changed, 148 insertions(+), 6 deletions(-) create mode 100644 components/modal/header.js diff --git a/components/modal/README.md b/components/modal/README.md index cc4c6390140fd7..f2f798cb2df60b 100644 --- a/components/modal/README.md +++ b/components/modal/README.md @@ -1,4 +1,4 @@ -RangeControl +Modal ======= The modal is used to create an accessible modal over an application. diff --git a/components/modal/header.js b/components/modal/header.js new file mode 100644 index 00000000000000..c2fb3c8875999b --- /dev/null +++ b/components/modal/header.js @@ -0,0 +1,29 @@ +import IconButton from '../icon-button'; +import './style.scss'; +import { __ } from '@wordpress/i18n'; + +const ModalHeader = ( { icon, title, onClose, closeLabel } ) => { + const label = closeLabel ? closeLabel : __( 'Close window' ); + + return ( +
+
+ +

+ { title } +

+
+ +
+ ); +}; + +export default ModalHeader; diff --git a/components/modal/index.js b/components/modal/index.js index fe70118bfab459..4c7d445fe61c9f 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -13,13 +13,13 @@ import { Component, createPortal } from '@wordpress/element'; * Internal dependencies */ import ModalContent from './modal-content'; +import ModalHeader from './header'; import * as ariaHelper from './aria-helper'; import './style.scss'; // Used to count the number of open modals. -let modalCount = 0; - -let parentElement; +let parentElement, + modalCount = 0; class Modal extends Component { static setAppElement( node ) { @@ -54,12 +54,18 @@ class Modal extends Component { render() { const { + isOpen, overlayClassName, className, + onRequestClose, style: { content, overlay, }, + /* header */ + title, + icon, + onClose, children, ...otherProps } = this.props; @@ -68,6 +74,10 @@ class Modal extends Component { this.node = document.createElement( 'div' ); } + if ( ! isOpen ) { + return null; + } + return createPortal(
- { children } + +
+ { children } +
, this.node diff --git a/components/modal/style.scss b/components/modal/style.scss index be74e9a01aae57..5114171264b249 100644 --- a/components/modal/style.scss +++ b/components/modal/style.scss @@ -1,9 +1,105 @@ .components-modal { &__screen-overlay { + position: fixed; + top: 0; + left: 0; + right: 0; + bottom: 0; + background-color: rgba(0, 0, 0, 0.6); z-index: z-index( '.components-modal__screen-overlay' ); + // on mobile the main content area has to scroll + // otherwise you can invoke the overscroll bounce on the non-scrolling container, causing (ノಠ益ಠ)ノ彡┻━┻ + @include break-small { + position: fixed; + top: $admin-bar-height-big; + } + + @include break-medium() { + top: $admin-bar-height; + } + } + + &__frame { + position: absolute; + top: 50%; + left: 50%; + right: auto; + bottom: auto; + width: auto; + max-width: 90%; + max-height: 90%; + border: 0; + border-radius: 0; + margin-right: -50%; + transform: translate(-50%, -50%); + background-color: #fff; + outline: none; + box-shadow: 0 3px 25px #000; + + // In small screens the content needs to be full width. + @media ( max-width: #{ ( $break-small ) } ) { + position: fixed; + top: 0; + bottom: 0; + right: 0; + left: 0; + margin: 0; + transform: initial; + max-width: none; + max-height: none; + } + } + + &__header { + height: $admin-bar-height; + border-bottom: 1px solid $light-gray-500; + padding: 10px; + display: flex; + flex-direction: row; + align-items: stretch; + justify-content: space-between; + z-index: z-index( '.edit-post-header' ); + left: 0; + right: 0; + + div { + align-items: center; + flex-grow: 1; + display: flex; + flex-direction: row; + justify-content: left; + + h1 { + font-size: 1em; + font-weight: normal; + padding-left: 5px; + } + + span { + display: inline-block; + } + + svg { + max-width: $icon-button-size; + max-height: $icon-button-size; + padding: 8px; + } + } } &__content { + // The height of the content is the height of it's parent, minus the header. after that, the offset was 3px. + max-height: calc( 100vh - 200px - #{ $admin-bar-height } - 3px ); + overflow: auto; + padding: 10px; z-index: z-index( '.components-modal__content' ); + + @media ( max-width: #{ ( $break-small ) } ) { + max-height: calc( 100vh ); + } } } + +@include editor-left( '.components-modal__screen-overlay' ); + + From aca49d0aa730224c10d44bf66ae1ec0ae86c4e3a Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 25 Apr 2018 13:00:55 +0200 Subject: [PATCH 05/67] Improved styling --- components/modal/{modal-content.js => frame.js} | 4 ++-- components/modal/index.js | 11 +++++------ components/modal/style.scss | 7 ++++--- edit-post/components/layout/index.js | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) rename components/modal/{modal-content.js => frame.js} (97%) diff --git a/components/modal/modal-content.js b/components/modal/frame.js similarity index 97% rename from components/modal/modal-content.js rename to components/modal/frame.js index 3a223f620bf81c..217dfeae38ce92 100644 --- a/components/modal/modal-content.js +++ b/components/modal/frame.js @@ -19,7 +19,7 @@ import withFocusContain from '../higher-order/with-focus-contain'; const ESC_KEY = 27; -class ModalContent extends Component { +class ModalFrame extends Component { constructor() { super( ...arguments ); @@ -108,4 +108,4 @@ export default compose( [ withFocusReturn, withFocusContain, clickOutside, -] )( ModalContent ); +] )( ModalFrame ); diff --git a/components/modal/index.js b/components/modal/index.js index 4c7d445fe61c9f..9d7b9300e8a99b 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -12,7 +12,7 @@ import { Component, createPortal } from '@wordpress/element'; /** * Internal dependencies */ -import ModalContent from './modal-content'; +import ModalFrame from './frame'; import ModalHeader from './header'; import * as ariaHelper from './aria-helper'; import './style.scss'; @@ -62,10 +62,8 @@ class Modal extends Component { content, overlay, }, - /* header */ title, icon, - onClose, children, ...otherProps } = this.props; @@ -85,7 +83,7 @@ class Modal extends Component { overlayClassName ) } style={ overlay }> - + title={ title } + icon={ icon } />
{ children }
- + , this.node ); diff --git a/components/modal/style.scss b/components/modal/style.scss index 5114171264b249..a949ff96ce4ede 100644 --- a/components/modal/style.scss +++ b/components/modal/style.scss @@ -26,7 +26,7 @@ right: auto; bottom: auto; width: auto; - max-width: 90%; + max-width: calc( 100vh - 200px ); max-height: 90%; border: 0; border-radius: 0; @@ -51,7 +51,8 @@ } &__header { - height: $admin-bar-height; + box-sizing: border-box; + height: $header-height; border-bottom: 1px solid $light-gray-500; padding: 10px; display: flex; @@ -89,7 +90,7 @@ &__content { // The height of the content is the height of it's parent, minus the header. after that, the offset was 3px. - max-height: calc( 100vh - 200px - #{ $admin-bar-height } - 3px ); + max-height: calc( 100vh - 200px - #{ $header-height } ); overflow: auto; padding: 10px; z-index: z-index( '.components-modal__content' ); diff --git a/edit-post/components/layout/index.js b/edit-post/components/layout/index.js index cef9eacf5e7a28..94dfb3010cfa83 100644 --- a/edit-post/components/layout/index.js +++ b/edit-post/components/layout/index.js @@ -7,7 +7,7 @@ import { some } from 'lodash'; /** * WordPress dependencies */ -import { Popover, ScrollLock, navigateRegions } from '@wordpress/components'; +import { Popover, ScrollLock, navigateRegions, Modal } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; import { AutosaveMonitor, From 3a5d75c11f3d078f2a86e659162cce628f83a869 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 25 Apr 2018 13:16:49 +0200 Subject: [PATCH 06/67] Applied code review feedback to with-focus-contain HOC --- .../higher-order/with-focus-contain/index.js | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/components/higher-order/with-focus-contain/index.js b/components/higher-order/with-focus-contain/index.js index 999c623e581416..fd0bf48d272c1a 100644 --- a/components/higher-order/with-focus-contain/index.js +++ b/components/higher-order/with-focus-contain/index.js @@ -2,7 +2,11 @@ * WordPress dependencies */ import { Component, createRef } from '@wordpress/element'; -import { focus } from '@wordpress/utils'; +import { focus, keycodes } from '@wordpress/utils'; + +const { + TAB, +} = keycodes; const withFocusContain = ( WrappedComponent ) => { return class extends Component { @@ -14,35 +18,32 @@ const withFocusContain = ( WrappedComponent ) => { } handleTabBehaviour( event ) { - if ( event.keyCode === 9 ) { - const tabbables = focus.tabbable.find( this.focusContainRef.current ); - if ( ! tabbables.length ) { - return; - } - const firstTabbable = tabbables[ 0 ]; - const lastTabbable = tabbables[ tabbables.length - 1 ]; - - if ( event.shiftKey && event.target === firstTabbable ) { - event.preventDefault(); - return lastTabbable.focus(); - } else if ( ! event.shiftKey && event.target === lastTabbable ) { - event.preventDefault(); - return firstTabbable.focus(); - } + if ( ! event.keyCode === TAB ) { + return; } - } - - componentDidMount() { - this.focusContainRef.current.addEventListener( 'keydown', this.handleTabBehaviour ); - } - componentWillUnmount() { - this.focusContainRef.current.addEventListener( 'keydown', this.handleTabBehaviour ); + const tabbables = focus.tabbable.find( this.focusContainRef.current ); + if ( ! tabbables.length ) { + return; + } + const firstTabbable = tabbables[ 0 ]; + const lastTabbable = tabbables[ tabbables.length - 1 ]; + + if ( event.shiftKey && event.target === firstTabbable ) { + event.preventDefault(); + return lastTabbable.focus(); + } else if ( ! event.shiftKey && event.target === lastTabbable ) { + event.preventDefault(); + return firstTabbable.focus(); + } } render() { return ( -
+
); From 371e66be5088c58b90659abe7aa1522b79810911 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 25 Apr 2018 14:03:02 +0200 Subject: [PATCH 07/67] Added eslint ignore for jsx-a11y/no-static-element-interactions --- components/higher-order/with-focus-contain/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/higher-order/with-focus-contain/index.js b/components/higher-order/with-focus-contain/index.js index fd0bf48d272c1a..5591698d33e5d0 100644 --- a/components/higher-order/with-focus-contain/index.js +++ b/components/higher-order/with-focus-contain/index.js @@ -1,3 +1,4 @@ +/* eslint-disable jsx-a11y/no-static-element-interactions */ /** * WordPress dependencies */ @@ -41,7 +42,6 @@ const withFocusContain = ( WrappedComponent ) => { render() { return (
From bd48b5ab471e2bdc9e6a84e4808c1e809c5c88ae Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 25 Apr 2018 14:59:48 +0200 Subject: [PATCH 08/67] Implemented withGlobalEvents HOC --- components/modal/frame.js | 26 +++++++++++++------------- components/modal/index.js | 10 ++++++---- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/components/modal/frame.js b/components/modal/frame.js index 217dfeae38ce92..f2579c7adb4630 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -8,7 +8,7 @@ import { defer } from 'lodash'; * WordPress dependencies */ import { Component, compose, createRef } from '@wordpress/element'; -import { focus } from '@wordpress/utils'; +import { focus, keycodes } from '@wordpress/utils'; /** * Internal dependencies @@ -16,15 +16,18 @@ import { focus } from '@wordpress/utils'; import './style.scss'; import withFocusReturn from '../higher-order/with-focus-return'; import withFocusContain from '../higher-order/with-focus-contain'; +import withGlobalEvents from '../higher-order/with-global-events'; -const ESC_KEY = 27; +const { + ESCAPE, +} = keycodes; class ModalFrame extends Component { constructor() { super( ...arguments ); this.containerRef = createRef(); - this.handleKeyPressEvents = this.handleKeyPressEvents.bind( this ); + this.handleKeyDown = this.handleKeyDown.bind( this ); } componentDidMount() { @@ -32,12 +35,6 @@ class ModalFrame extends Component { if ( this.props.focusOnMount ) { this.focusFirstTabbable(); } - // Key events - window.addEventListener( 'keydown', this.handleKeyPressEvents ); - } - - componentWillUnmount() { - window.removeEventListener( 'keydown', this.handleKeyPressEvents ); } focusFirstTabbable() { @@ -56,13 +53,13 @@ class ModalFrame extends Component { } } - handleKeyPressEvents( event ) { - if ( event.keyCode === ESC_KEY ) { - this.handleEscapePress( event ); + handleKeyDown( event ) { + if ( event.keyCode === ESCAPE ) { + this.handleEscapeKeyDown( event ); } } - handleEscapePress( event ) { + handleEscapeKeyDown( event ) { if ( this.props.shouldCloseOnEsc ) { event.preventDefault(); this.onRequestClose( event ); @@ -105,6 +102,9 @@ class ModalFrame extends Component { } export default compose( [ + withGlobalEvents( { + keydown: 'handleKeyDown', + } ), withFocusReturn, withFocusContain, clickOutside, diff --git a/components/modal/index.js b/components/modal/index.js index 9d7b9300e8a99b..c8dcb48cdd024a 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -22,6 +22,12 @@ let parentElement, modalCount = 0; class Modal extends Component { + constructor() { + super( ...arguments ); + + this.node = document.createElement( 'div' ); + } + static setAppElement( node ) { ariaHelper.setAppElement( node ); } @@ -68,10 +74,6 @@ class Modal extends Component { ...otherProps } = this.props; - if ( ! this.node ) { - this.node = document.createElement( 'div' ); - } - if ( ! isOpen ) { return null; } From 809bfdd1ed5dd27fc5336b0ced0f61bf4e260885 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Mon, 30 Apr 2018 15:29:18 +0200 Subject: [PATCH 09/67] withGlobalEvents HOC now forwards refs --- .../higher-order/with-global-events/index.js | 22 +++++++++++++------ components/modal/frame.js | 7 +++--- element/index.js | 6 +++++ 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/components/higher-order/with-global-events/index.js b/components/higher-order/with-global-events/index.js index a04b1d8020b351..90b9b1f8386cbd 100644 --- a/components/higher-order/with-global-events/index.js +++ b/components/higher-order/with-global-events/index.js @@ -8,7 +8,7 @@ import { forEach } from 'lodash'; */ import { Component, - createRef, + forwardRef, createHigherOrderComponent, } from '@wordpress/element'; @@ -26,13 +26,12 @@ const listener = new Listener(); function withGlobalEvents( eventTypesToHandlers ) { return createHigherOrderComponent( ( WrappedComponent ) => { - return class extends Component { + class Wrapper extends Component { constructor() { super( ...arguments ); this.handleEvent = this.handleEvent.bind( this ); - - this.ref = createRef(); + this.setRef = this.setRef.bind( this ); } componentDidMount() { @@ -49,15 +48,24 @@ function withGlobalEvents( eventTypesToHandlers ) { handleEvent( event ) { const handler = eventTypesToHandlers[ event.type ]; - if ( typeof this.ref.current[ handler ] === 'function' ) { - this.ref.current[ handler ]( event ); + if ( typeof this.wrappedRef[ handler ] === 'function' ) { + this.wrappedRef[ handler ]( event ); } } + setRef( el ) { + this.wrappedRef = el; + this.props.forwardedRef( el ); + } + render() { - return ; + return ; } }; + + return forwardRef( ( props, ref ) => { + return ; + } ); }, 'withGlobalEvents' ); } diff --git a/components/modal/frame.js b/components/modal/frame.js index f2579c7adb4630..c0a67e481ede22 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -28,6 +28,7 @@ class ModalFrame extends Component { this.containerRef = createRef(); this.handleKeyDown = this.handleKeyDown.bind( this ); + this.handleClickOutside = this.handleClickOutside.bind( this ); } componentDidMount() { @@ -102,10 +103,10 @@ class ModalFrame extends Component { } export default compose( [ - withGlobalEvents( { - keydown: 'handleKeyDown', - } ), withFocusReturn, withFocusContain, clickOutside, + withGlobalEvents( { + keydown: 'handleKeyDown', + } ), ] )( ModalFrame ); diff --git a/element/index.js b/element/index.js index ff524b1b7645f1..120795d2df0a14 100644 --- a/element/index.js +++ b/element/index.js @@ -5,6 +5,7 @@ import { createElement, createContext, createRef, + forwardRef, Component, cloneElement, Children, @@ -46,6 +47,11 @@ export { createElement }; */ export { createRef }; +/** + * todo + */ +export { forwardRef }; + /** * Renders a given element into the target DOM node. * From 47219be2ff85ae045f5095ff2216c6819c4f4b64 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Mon, 30 Apr 2018 16:10:31 +0200 Subject: [PATCH 10/67] Replace lodash defer with withSafeTimeout --- components/modal/frame.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/components/modal/frame.js b/components/modal/frame.js index c0a67e481ede22..e12131d24c8f0d 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -2,7 +2,6 @@ * External dependencies */ import clickOutside from 'react-click-outside'; -import { defer } from 'lodash'; /** * WordPress dependencies @@ -17,6 +16,7 @@ import './style.scss'; import withFocusReturn from '../higher-order/with-focus-return'; import withFocusContain from '../higher-order/with-focus-contain'; import withGlobalEvents from '../higher-order/with-global-events'; +import withSafeTimeout from '../higher-order/with-safe-timeout'; const { ESCAPE, @@ -29,6 +29,7 @@ class ModalFrame extends Component { this.containerRef = createRef(); this.handleKeyDown = this.handleKeyDown.bind( this ); this.handleClickOutside = this.handleClickOutside.bind( this ); + this.focusFirstTabbable = this.focusFirstTabbable.bind( this ); } componentDidMount() { @@ -40,12 +41,13 @@ class ModalFrame extends Component { focusFirstTabbable() { // Required because the node is appended to the DOM after rendering. - defer( () => { + const { setTimeout } = this.props; + setTimeout( () => { const tabbables = focus.tabbable.find( this.containerRef.current ); if ( tabbables.length ) { tabbables[ 0 ].focus(); } - } ); + }, 0 ); } handleClickOutside( event ) { @@ -105,6 +107,7 @@ class ModalFrame extends Component { export default compose( [ withFocusReturn, withFocusContain, + withSafeTimeout, clickOutside, withGlobalEvents( { keydown: 'handleKeyDown', From 9b93633089678c4eaf675b51bf8e91a0da82dbf1 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Mon, 30 Apr 2018 16:20:01 +0200 Subject: [PATCH 11/67] Removed unnecessary return statements --- components/higher-order/with-focus-contain/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/higher-order/with-focus-contain/index.js b/components/higher-order/with-focus-contain/index.js index 5591698d33e5d0..a8456f25b64b69 100644 --- a/components/higher-order/with-focus-contain/index.js +++ b/components/higher-order/with-focus-contain/index.js @@ -32,10 +32,10 @@ const withFocusContain = ( WrappedComponent ) => { if ( event.shiftKey && event.target === firstTabbable ) { event.preventDefault(); - return lastTabbable.focus(); + lastTabbable.focus(); } else if ( ! event.shiftKey && event.target === lastTabbable ) { event.preventDefault(); - return firstTabbable.focus(); + firstTabbable.focus(); } } From de29a46f7795594b343d796e2b68b33da11a77a2 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 1 May 2018 09:02:06 +0200 Subject: [PATCH 12/67] Created separate styling rules --- components/modal/style.scss | 166 ++++++++++++++++++------------------ 1 file changed, 82 insertions(+), 84 deletions(-) diff --git a/components/modal/style.scss b/components/modal/style.scss index a949ff96ce4ede..756ec33e15fae0 100644 --- a/components/modal/style.scss +++ b/components/modal/style.scss @@ -1,103 +1,101 @@ -.components-modal { - &__screen-overlay { +.components-modal__screen-overlay { + position: fixed; + top: 0; + left: 0; + right: 0; + bottom: 0; + background-color: rgba(0, 0, 0, 0.6); + z-index: z-index( '.components-modal__screen-overlay' ); + // on mobile the main content area has to scroll + // otherwise you can invoke the overscroll bounce on the non-scrolling container, causing (ノಠ益ಠ)ノ彡┻━┻ + @include break-small { position: fixed; - top: 0; - left: 0; - right: 0; - bottom: 0; - background-color: rgba(0, 0, 0, 0.6); - z-index: z-index( '.components-modal__screen-overlay' ); - // on mobile the main content area has to scroll - // otherwise you can invoke the overscroll bounce on the non-scrolling container, causing (ノಠ益ಠ)ノ彡┻━┻ - @include break-small { - position: fixed; - top: $admin-bar-height-big; - } + top: $admin-bar-height-big; + } - @include break-medium() { - top: $admin-bar-height; - } + @include break-medium() { + top: $admin-bar-height; } +} - &__frame { - position: absolute; - top: 50%; - left: 50%; - right: auto; - bottom: auto; - width: auto; - max-width: calc( 100vh - 200px ); - max-height: 90%; - border: 0; - border-radius: 0; - margin-right: -50%; - transform: translate(-50%, -50%); - background-color: #fff; - outline: none; - box-shadow: 0 3px 25px #000; +.components-modal__frame { + position: absolute; + top: 50%; + left: 50%; + right: auto; + bottom: auto; + width: auto; + max-width: calc( 100vh - 200px ); + max-height: 90%; + border: 0; + border-radius: 0; + margin-right: -50%; + transform: translate(-50%, -50%); + background-color: #fff; + outline: none; + box-shadow: 0 3px 25px #000; - // In small screens the content needs to be full width. - @media ( max-width: #{ ( $break-small ) } ) { - position: fixed; - top: 0; - bottom: 0; - right: 0; - left: 0; - margin: 0; - transform: initial; - max-width: none; - max-height: none; - } + // In small screens the content needs to be full width. + @media ( max-width: #{ ( $break-small ) } ) { + position: fixed; + top: 0; + bottom: 0; + right: 0; + left: 0; + margin: 0; + transform: initial; + max-width: none; + max-height: none; } +} - &__header { - box-sizing: border-box; - height: $header-height; - border-bottom: 1px solid $light-gray-500; - padding: 10px; +.components-modal__header { + box-sizing: border-box; + height: $header-height; + border-bottom: 1px solid $light-gray-500; + padding: 10px; + display: flex; + flex-direction: row; + align-items: stretch; + justify-content: space-between; + z-index: z-index( '.edit-post-header' ); + left: 0; + right: 0; + + div { + align-items: center; + flex-grow: 1; display: flex; flex-direction: row; - align-items: stretch; - justify-content: space-between; - z-index: z-index( '.edit-post-header' ); - left: 0; - right: 0; + justify-content: left; - div { - align-items: center; - flex-grow: 1; - display: flex; - flex-direction: row; - justify-content: left; - - h1 { - font-size: 1em; - font-weight: normal; - padding-left: 5px; - } + h1 { + font-size: 1em; + font-weight: normal; + padding-left: 5px; + } - span { - display: inline-block; - } + span { + display: inline-block; + } - svg { - max-width: $icon-button-size; - max-height: $icon-button-size; - padding: 8px; - } + svg { + max-width: $icon-button-size; + max-height: $icon-button-size; + padding: 8px; } } +} - &__content { - // The height of the content is the height of it's parent, minus the header. after that, the offset was 3px. - max-height: calc( 100vh - 200px - #{ $header-height } ); - overflow: auto; - padding: 10px; - z-index: z-index( '.components-modal__content' ); +.components-modal__content { + // The height of the content is the height of it's parent, minus the header. after that, the offset was 3px. + max-height: calc( 100vh - 200px - #{ $header-height } ); + overflow: auto; + padding: 10px; + z-index: z-index( '.components-modal__content' ); - @media ( max-width: #{ ( $break-small ) } ) { - max-height: calc( 100vh ); - } + @media ( max-width: #{ ( $break-small ) } ) { + max-height: calc( 100vh ); } } From 4a1b2c4050b07d5c150ad8a56be23fa5fba3a09a Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 1 May 2018 09:54:46 +0200 Subject: [PATCH 13/67] Added documentation for forwardRef function --- element/index.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/element/index.js b/element/index.js index 120795d2df0a14..d2970eeec3309e 100644 --- a/element/index.js +++ b/element/index.js @@ -48,7 +48,14 @@ export { createElement }; export { createRef }; /** - * todo + * Allows a `ref` to be forwarded to a component further down the component + * tree. + * + * @param {Function} forwardFunction A function receiving the component's props and ref, + * allowing the ref to be mapped to a different prop in + * order to be passed further down the tree. + * + * @return {Component} The component with the forwarded ref. */ export { forwardRef }; From 1092fbd21bc5e060e9a781dbdd162db01c21f7c6 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 1 May 2018 11:41:12 +0200 Subject: [PATCH 14/67] Made mount location for modal configurable --- components/index.js | 2 +- components/modal/context.js | 32 +++++++++++++++++++++++++++++ components/modal/index.js | 20 ++++++++++++++---- editor/components/provider/index.js | 17 +++++++++++++++ lib/client-assets.php | 1 + 5 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 components/modal/context.js diff --git a/components/index.js b/components/index.js index 1933ec52135ddf..022e3e20ff6cdc 100644 --- a/components/index.js +++ b/components/index.js @@ -25,7 +25,7 @@ export { default as KeyboardShortcuts } from './keyboard-shortcuts'; export { default as MenuGroup } from './menu-group'; export { default as MenuItem } from './menu-item'; export { default as MenuItemsChoice } from './menu-items-choice'; -export { default as Modal } from './modal'; +export { default as Modal, ModalContextProvider } from './modal'; export { default as ScrollLock } from './scroll-lock'; export { NavigableMenu, TabbableContainer } from './navigable-container'; export { default as Notice } from './notice'; diff --git a/components/modal/context.js b/components/modal/context.js new file mode 100644 index 00000000000000..6a25613db449e7 --- /dev/null +++ b/components/modal/context.js @@ -0,0 +1,32 @@ +/** + * WordPress dependencies + */ +import { createContext, createHigherOrderComponent } from '@wordpress/element'; + +const { Consumer, Provider } = createContext( { + elementId: null, +} ); + +export { Provider as ModalContextProvider }; + +/** + * A Higher-order Component used to inject Modal context into the wrapped + * component. + * + * @param {Component} OriginalComponent Component to wrap. + * + * @return {Component} Component with Modal context injected. + */ +export const withModalContext = createHigherOrderComponent( + ( OriginalComponent ) => ( props ) => ( + + { ( modalContext ) => ( + + ) } + + ), + 'withModalContext' +); diff --git a/components/modal/index.js b/components/modal/index.js index c8dcb48cdd024a..6b494470ff095e 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -12,6 +12,7 @@ import { Component, createPortal } from '@wordpress/element'; /** * Internal dependencies */ +import { withModalContext } from './context'; import ModalFrame from './frame'; import ModalHeader from './header'; import * as ariaHelper from './aria-helper'; @@ -41,8 +42,13 @@ class Modal extends Component { componentDidMount() { modalCount++; + const { + modalContext, + } = this.props; + if ( ! this.parentElement ) { - setElements(); + console.log( modalContext ); + setElements( modalContext.elementId ); } ariaHelper.hideApp(); @@ -127,8 +133,13 @@ Modal.defaultProps = { }, }; -function setElements() { - const wpwrapEl = document.getElementById( 'wpwrap' ); +/** + * Sets the element where the modal should mount itself. + * + * @param {string} elementId The element id. + */ +function setElements( elementId ) { + const wpwrapEl = document.getElementById( elementId ); if ( wpwrapEl ) { Modal.setAppElement( wpwrapEl ); @@ -136,4 +147,5 @@ function setElements() { } } -export default Modal; +export { ModalContextProvider } from './context'; +export default withModalContext( Modal ); diff --git a/editor/components/provider/index.js b/editor/components/provider/index.js index 37dd459c721a31..a835a5b706f41c 100644 --- a/editor/components/provider/index.js +++ b/editor/components/provider/index.js @@ -12,6 +12,7 @@ import { APIProvider, DropZoneProvider, SlotFillProvider, + ModalContextProvider, } from '@wordpress/components'; import { withDispatch } from '@wordpress/data'; @@ -36,6 +37,10 @@ class EditorProvider extends Component { redo, createUndoLevel, } = this.props; + const { + modalElementId, + ...editorSettings, + } = settings; const providers = [ // Editor settings provider [ @@ -91,6 +96,18 @@ class EditorProvider extends Component { [ DropZoneProvider, ], + + // Modal provider + // + // - context.modalContext.elementId + [ + ModalContextProvider, + { + value: { + elementId: modalElementId, + }, + }, + ] ]; const createEditorElement = flow( diff --git a/lib/client-assets.php b/lib/client-assets.php index bb52445e67ead3..0dde1ceebcca41 100644 --- a/lib/client-assets.php +++ b/lib/client-assets.php @@ -942,6 +942,7 @@ function gutenberg_editor_scripts_and_styles( $hook ) { $allowed_block_types = apply_filters( 'allowed_block_types', true, $post ); $editor_settings = array( + 'modalElementId' => 'wpwrap', 'alignWide' => $align_wide || ! empty( $gutenberg_theme_support[0]['wide-images'] ), // Backcompat. Use `align-wide` outside of `gutenberg` array. 'availableTemplates' => wp_get_theme()->get_page_templates( get_post( $post_to_edit['id'] ) ), 'allowedBlockTypes' => $allowed_block_types, From 44d91d3ff1ee0cdaf2140ddd40eb784accc89f2d Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 1 May 2018 11:54:20 +0200 Subject: [PATCH 15/67] Renamed elementId to appElementId for clarity --- components/modal/README.md | 34 ++++++++++++++--------------- components/modal/context.js | 2 +- components/modal/index.js | 18 ++++++++------- editor/components/provider/index.js | 4 ++-- lib/client-assets.php | 2 +- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/components/modal/README.md b/components/modal/README.md index f2f798cb2df60b..d76ed371261c7f 100644 --- a/components/modal/README.md +++ b/components/modal/README.md @@ -8,25 +8,23 @@ The modal is used to create an accessible modal over an application. ## Usage Render a screen overlay with a modal on top. -```js -// When the app element is set it puts an aria-hidden="true" to the provided node. -Modal.setAppElement( document.getElementById( 'wpwrap' ).parentNode ) -``` ```jsx - { - return document.getElementById( 'wpwrap' ); - } ) - > - - - - - + + { + return document.getElementById( 'wpwrap' ); + } ) + > + + + + + + ``` ## Props diff --git a/components/modal/context.js b/components/modal/context.js index 6a25613db449e7..1b52ea3f7df993 100644 --- a/components/modal/context.js +++ b/components/modal/context.js @@ -4,7 +4,7 @@ import { createContext, createHigherOrderComponent } from '@wordpress/element'; const { Consumer, Provider } = createContext( { - elementId: null, + appElementId: null, } ); export { Provider as ModalContextProvider }; diff --git a/components/modal/index.js b/components/modal/index.js index 6b494470ff095e..e497e73af1b2c1 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -47,8 +47,7 @@ class Modal extends Component { } = this.props; if ( ! this.parentElement ) { - console.log( modalContext ); - setElements( modalContext.elementId ); + setElements( modalContext.appElementId ); } ariaHelper.hideApp(); @@ -136,14 +135,17 @@ Modal.defaultProps = { /** * Sets the element where the modal should mount itself. * - * @param {string} elementId The element id. + * Note that the modal will mount itself as a sibling of this element, so using body is + * not possible since it cannot have additional sibling elements. + * + * @param {string} appElementId The element id of the element that contains your application. */ -function setElements( elementId ) { - const wpwrapEl = document.getElementById( elementId ); +function setElements( appElementId ) { + const element = document.getElementById( appElementId ); - if ( wpwrapEl ) { - Modal.setAppElement( wpwrapEl ); - Modal.setParentElement( wpwrapEl.parentNode ); + if ( element ) { + Modal.setAppElement( element ); + Modal.setParentElement( element.parentNode ); } } diff --git a/editor/components/provider/index.js b/editor/components/provider/index.js index a835a5b706f41c..217dccd232c335 100644 --- a/editor/components/provider/index.js +++ b/editor/components/provider/index.js @@ -38,7 +38,7 @@ class EditorProvider extends Component { createUndoLevel, } = this.props; const { - modalElementId, + modalAppElementId, ...editorSettings, } = settings; const providers = [ @@ -104,7 +104,7 @@ class EditorProvider extends Component { ModalContextProvider, { value: { - elementId: modalElementId, + appElementId: modalAppElementId, }, }, ] diff --git a/lib/client-assets.php b/lib/client-assets.php index 0dde1ceebcca41..4faaeaeeb7efb6 100644 --- a/lib/client-assets.php +++ b/lib/client-assets.php @@ -942,7 +942,7 @@ function gutenberg_editor_scripts_and_styles( $hook ) { $allowed_block_types = apply_filters( 'allowed_block_types', true, $post ); $editor_settings = array( - 'modalElementId' => 'wpwrap', + 'modalAppElementId' => 'wpwrap', 'alignWide' => $align_wide || ! empty( $gutenberg_theme_support[0]['wide-images'] ), // Backcompat. Use `align-wide` outside of `gutenberg` array. 'availableTemplates' => wp_get_theme()->get_page_templates( get_post( $post_to_edit['id'] ) ), 'allowedBlockTypes' => $allowed_block_types, From 4aef9b6622160ea2042abfeb1abf8d2f40fafd3a Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 1 May 2018 14:36:59 +0200 Subject: [PATCH 16/67] Added noop for when no reg is provided in forwardRef --- components/higher-order/with-global-events/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/higher-order/with-global-events/index.js b/components/higher-order/with-global-events/index.js index 90b9b1f8386cbd..461cfcc45c5a9a 100644 --- a/components/higher-order/with-global-events/index.js +++ b/components/higher-order/with-global-events/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { forEach } from 'lodash'; +import { forEach, noop } from 'lodash'; /** * WordPress dependencies @@ -53,18 +53,18 @@ function withGlobalEvents( eventTypesToHandlers ) { } } - setRef( el ) { + handleRef( el ) { this.wrappedRef = el; this.props.forwardedRef( el ); } render() { - return ; + return ; } }; return forwardRef( ( props, ref ) => { - return ; + return ; } ); }, 'withGlobalEvents' ); } From 5c0568c752812861f79f5e508deccfc95b09d0b5 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 1 May 2018 14:37:57 +0200 Subject: [PATCH 17/67] Fixed error in EditorProvider --- editor/components/provider/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/editor/components/provider/index.js b/editor/components/provider/index.js index 217dccd232c335..9d36e190840480 100644 --- a/editor/components/provider/index.js +++ b/editor/components/provider/index.js @@ -39,7 +39,7 @@ class EditorProvider extends Component { } = this.props; const { modalAppElementId, - ...editorSettings, + ...editorSettings } = settings; const providers = [ // Editor settings provider @@ -48,7 +48,7 @@ class EditorProvider extends Component { { value: { ...EditorSettings.defaultSettings, - ...settings, + ...editorSettings, }, }, ], From 887193dbd7f149867cbbb3946498a3b1a9bf16cf Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 1 May 2018 14:40:11 +0200 Subject: [PATCH 18/67] Fix eslint errors --- components/higher-order/with-global-events/index.js | 2 +- edit-post/components/layout/index.js | 2 +- editor/components/provider/index.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/components/higher-order/with-global-events/index.js b/components/higher-order/with-global-events/index.js index 461cfcc45c5a9a..bc3a79628cf2d6 100644 --- a/components/higher-order/with-global-events/index.js +++ b/components/higher-order/with-global-events/index.js @@ -61,7 +61,7 @@ function withGlobalEvents( eventTypesToHandlers ) { render() { return ; } - }; + } return forwardRef( ( props, ref ) => { return ; diff --git a/edit-post/components/layout/index.js b/edit-post/components/layout/index.js index 94dfb3010cfa83..cef9eacf5e7a28 100644 --- a/edit-post/components/layout/index.js +++ b/edit-post/components/layout/index.js @@ -7,7 +7,7 @@ import { some } from 'lodash'; /** * WordPress dependencies */ -import { Popover, ScrollLock, navigateRegions, Modal } from '@wordpress/components'; +import { Popover, ScrollLock, navigateRegions } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; import { AutosaveMonitor, diff --git a/editor/components/provider/index.js b/editor/components/provider/index.js index 9d36e190840480..5eb9c381d277af 100644 --- a/editor/components/provider/index.js +++ b/editor/components/provider/index.js @@ -107,7 +107,7 @@ class EditorProvider extends Component { appElementId: modalAppElementId, }, }, - ] + ], ]; const createEditorElement = flow( From b0c4d823b241571f162efdc08e5c6890c943ddb1 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 1 May 2018 15:10:05 +0200 Subject: [PATCH 19/67] Fixed incorrectly bound function --- components/higher-order/with-global-events/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/higher-order/with-global-events/index.js b/components/higher-order/with-global-events/index.js index bc3a79628cf2d6..321f79ef5f258e 100644 --- a/components/higher-order/with-global-events/index.js +++ b/components/higher-order/with-global-events/index.js @@ -31,7 +31,7 @@ function withGlobalEvents( eventTypesToHandlers ) { super( ...arguments ); this.handleEvent = this.handleEvent.bind( this ); - this.setRef = this.setRef.bind( this ); + this.handleRef = this.handleRef.bind( this ); } componentDidMount() { From f25b989a265c14dda127cbc9195a0c94c8ccc2d5 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 2 May 2018 10:00:24 +0200 Subject: [PATCH 20/67] Modal now by default mounts to the body and hides all other elements --- components/index.js | 2 +- components/modal/aria-helper.js | 46 ++++++++++++----- components/modal/context.js | 32 ------------ components/modal/index.js | 77 ++++++++++++++--------------- editor/components/provider/index.js | 20 +------- lib/client-assets.php | 1 - 6 files changed, 76 insertions(+), 102 deletions(-) delete mode 100644 components/modal/context.js diff --git a/components/index.js b/components/index.js index 022e3e20ff6cdc..1933ec52135ddf 100644 --- a/components/index.js +++ b/components/index.js @@ -25,7 +25,7 @@ export { default as KeyboardShortcuts } from './keyboard-shortcuts'; export { default as MenuGroup } from './menu-group'; export { default as MenuItem } from './menu-item'; export { default as MenuItemsChoice } from './menu-items-choice'; -export { default as Modal, ModalContextProvider } from './modal'; +export { default as Modal } from './modal'; export { default as ScrollLock } from './scroll-lock'; export { NavigableMenu, TabbableContainer } from './navigable-container'; export { default as Notice } from './notice'; diff --git a/components/modal/aria-helper.js b/components/modal/aria-helper.js index c7eacb4fa3b2a1..2e0c1dff46f163 100644 --- a/components/modal/aria-helper.js +++ b/components/modal/aria-helper.js @@ -1,19 +1,43 @@ -let appElement = null; +/** + * External dependencies + */ +import { forEach } from 'lodash'; -export function setAppElement( node ) { - if ( ! appElement ) { - appElement = node; - } -} +let hiddenElements = [], + isHidden = false; -export function hideApp() { - if ( appElement ) { - appElement.setAttribute( 'aria-hidden', 'true' ); +/** + * Hides all elements in the body element from screen-readers except + * the provided element. + * + * @param {Element} unhiddenElement The element that should not be hidden. + */ +export function hideApp( unhiddenElement ) { + if ( isHidden ) { + return; } + const elements = document.body.children; + forEach( elements, element => { + if ( element === unhiddenElement ) { + return; + } + element.setAttribute( 'aria-hidden', 'true' ); + hiddenElements.push( element ); + } ); + isHidden = true; } +/** + * Makes all elements in the body that have been hidden by `hideApp` + * visible again to screen-readers. + */ export function showApp() { - if ( appElement ) { - appElement.removeAttribute( 'aria-hidden' ); + if ( ! isHidden ) { + return; } + forEach( hiddenElements, element => { + element.removeAttribute( 'aria-hidden' ); + } ); + hiddenElements = []; + isHidden = false; } diff --git a/components/modal/context.js b/components/modal/context.js deleted file mode 100644 index 1b52ea3f7df993..00000000000000 --- a/components/modal/context.js +++ /dev/null @@ -1,32 +0,0 @@ -/** - * WordPress dependencies - */ -import { createContext, createHigherOrderComponent } from '@wordpress/element'; - -const { Consumer, Provider } = createContext( { - appElementId: null, -} ); - -export { Provider as ModalContextProvider }; - -/** - * A Higher-order Component used to inject Modal context into the wrapped - * component. - * - * @param {Component} OriginalComponent Component to wrap. - * - * @return {Component} Component with Modal context injected. - */ -export const withModalContext = createHigherOrderComponent( - ( OriginalComponent ) => ( props ) => ( - - { ( modalContext ) => ( - - ) } - - ), - 'withModalContext' -); diff --git a/components/modal/index.js b/components/modal/index.js index e497e73af1b2c1..ce634b526970f4 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -12,7 +12,6 @@ import { Component, createPortal } from '@wordpress/element'; /** * Internal dependencies */ -import { withModalContext } from './context'; import ModalFrame from './frame'; import ModalHeader from './header'; import * as ariaHelper from './aria-helper'; @@ -20,47 +19,65 @@ import './style.scss'; // Used to count the number of open modals. let parentElement, - modalCount = 0; + modalCount = 0, + openModalCount = 0; class Modal extends Component { constructor() { super( ...arguments ); + if ( ! parentElement ) { + parentElement = document.createElement( 'div' ); + document.body.appendChild( parentElement ); + } this.node = document.createElement( 'div' ); } - static setAppElement( node ) { - ariaHelper.setAppElement( node ); - } + componentDidMount() { + modalCount++; - static setParentElement( node ) { - if ( ! parentElement ) { - parentElement = node; + const { isOpen } = this.props; + if ( isOpen ) { + this.openModal(); } } - componentDidMount() { - modalCount++; + componentDidUpdate( prevProps ) { + const openStateChanged = this.props.isOpen !== prevProps.isOpen; + if ( openStateChanged && ! prevProps.isOpen ) { + this.openModal(); + } else if ( openStateChanged && prevProps.isOpen ) { + this.closeModal(); + } + } - const { - modalContext, - } = this.props; + componentWillUnmount() { + const { isOpen } = this.props; + if ( isOpen ) { + this.closeModal(); + } - if ( ! this.parentElement ) { - setElements( modalContext.appElementId ); + modalCount--; + if ( modalCount === 0 ) { + document.body.removeChild( parentElement ); + parentElement = null; } + } - ariaHelper.hideApp(); + openModal() { + openModalCount++; + + ariaHelper.hideApp( parentElement ); parentElement.appendChild( this.node ); } - componentWillUnmount() { - modalCount--; + closeModal() { + openModalCount--; - if ( modalCount === 0 ) { + parentElement.removeChild( this.node ); + if ( openModalCount === 0 ) { ariaHelper.showApp(); } - parentElement.removeChild( this.node ); } render() { @@ -132,22 +149,4 @@ Modal.defaultProps = { }, }; -/** - * Sets the element where the modal should mount itself. - * - * Note that the modal will mount itself as a sibling of this element, so using body is - * not possible since it cannot have additional sibling elements. - * - * @param {string} appElementId The element id of the element that contains your application. - */ -function setElements( appElementId ) { - const element = document.getElementById( appElementId ); - - if ( element ) { - Modal.setAppElement( element ); - Modal.setParentElement( element.parentNode ); - } -} - -export { ModalContextProvider } from './context'; -export default withModalContext( Modal ); +export default Modal; diff --git a/editor/components/provider/index.js b/editor/components/provider/index.js index 5eb9c381d277af..fde7d6fde14b93 100644 --- a/editor/components/provider/index.js +++ b/editor/components/provider/index.js @@ -12,7 +12,6 @@ import { APIProvider, DropZoneProvider, SlotFillProvider, - ModalContextProvider, } from '@wordpress/components'; import { withDispatch } from '@wordpress/data'; @@ -37,10 +36,7 @@ class EditorProvider extends Component { redo, createUndoLevel, } = this.props; - const { - modalAppElementId, - ...editorSettings - } = settings; + const providers = [ // Editor settings provider [ @@ -48,7 +44,7 @@ class EditorProvider extends Component { { value: { ...EditorSettings.defaultSettings, - ...editorSettings, + ...settings, }, }, ], @@ -96,18 +92,6 @@ class EditorProvider extends Component { [ DropZoneProvider, ], - - // Modal provider - // - // - context.modalContext.elementId - [ - ModalContextProvider, - { - value: { - appElementId: modalAppElementId, - }, - }, - ], ]; const createEditorElement = flow( diff --git a/lib/client-assets.php b/lib/client-assets.php index 4faaeaeeb7efb6..bb52445e67ead3 100644 --- a/lib/client-assets.php +++ b/lib/client-assets.php @@ -942,7 +942,6 @@ function gutenberg_editor_scripts_and_styles( $hook ) { $allowed_block_types = apply_filters( 'allowed_block_types', true, $post ); $editor_settings = array( - 'modalAppElementId' => 'wpwrap', 'alignWide' => $align_wide || ! empty( $gutenberg_theme_support[0]['wide-images'] ), // Backcompat. Use `align-wide` outside of `gutenberg` array. 'availableTemplates' => wp_get_theme()->get_page_templates( get_post( $post_to_edit['id'] ) ), 'allowedBlockTypes' => $allowed_block_types, From c36074231f3f4ab5e3c7f32d217029c2c552d499 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 2 May 2018 10:21:06 +0200 Subject: [PATCH 21/67] hideApp no longer unhides elements that already had a aria-hidden=true attribute and ignores script tags --- components/modal/aria-helper.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/components/modal/aria-helper.js b/components/modal/aria-helper.js index 2e0c1dff46f163..9c6e2d94b925c6 100644 --- a/components/modal/aria-helper.js +++ b/components/modal/aria-helper.js @@ -8,7 +8,8 @@ let hiddenElements = [], /** * Hides all elements in the body element from screen-readers except - * the provided element. + * the provided element, script elements and elements that already have + * an `aria-hidden="true"` attribute. * * @param {Element} unhiddenElement The element that should not be hidden. */ @@ -18,7 +19,11 @@ export function hideApp( unhiddenElement ) { } const elements = document.body.children; forEach( elements, element => { - if ( element === unhiddenElement ) { + if ( + element === unhiddenElement || + element.tagName === 'SCRIPT' || + element.hasAttribute( 'aria-hidden', 'true') + ) { return; } element.setAttribute( 'aria-hidden', 'true' ); From 984ef8e8acc747ead5a59dba673c790aff55bacb Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 2 May 2018 10:58:19 +0200 Subject: [PATCH 22/67] Improved a11y and updated documentation --- .../higher-order/with-focus-contain/index.js | 2 +- components/modal/README.md | 70 ++++++++++++++++--- components/modal/frame.js | 3 +- components/modal/header.js | 10 +-- components/modal/index.js | 6 +- 5 files changed, 73 insertions(+), 18 deletions(-) diff --git a/components/higher-order/with-focus-contain/index.js b/components/higher-order/with-focus-contain/index.js index a8456f25b64b69..b15dfc6efdc5c0 100644 --- a/components/higher-order/with-focus-contain/index.js +++ b/components/higher-order/with-focus-contain/index.js @@ -19,7 +19,7 @@ const withFocusContain = ( WrappedComponent ) => { } handleTabBehaviour( event ) { - if ( ! event.keyCode === TAB ) { + if ( event.keyCode !== TAB ) { return; } diff --git a/components/modal/README.md b/components/modal/README.md index d76ed371261c7f..b2d591426db7da 100644 --- a/components/modal/README.md +++ b/components/modal/README.md @@ -9,22 +9,54 @@ The modal is used to create an accessible modal over an application. Render a screen overlay with a modal on top. ```jsx - - { - return document.getElementById( 'wpwrap' ); - } ) - > + - +``` + +## Implement close logic + +```js +const { Component, Fragment } = wp.element; +class MyModalWrapper extends Component { + constructor() { + super( ...arguments ); + this.state = { + isOpen: true, + } + } + + closeModal() { + if ( this.state.isOpen ) { + this.setState( { isOpen: false } ); + } + } + + openModal() { + if ( ! this.state.isOpen ) { + this.setState( { isOpen: true } ); + } + } + + render() { + return ( + + + + + + + ); + } +} ``` ## Props @@ -32,6 +64,13 @@ Render a screen overlay with a modal on top. The set of props accepted by the component will be specified below. Props not included in this set will be applied to the input elements. +### title + +This property is used as the modal header's title. It is required for accessibility reasons. + +- Type: `String` +- Required: Yes + ### onRequestClose This function is called to indicate that the modal should be closed. @@ -54,6 +93,7 @@ You are encouraged to use this when the modal is visually labelled. - Type: `String` - Required: No +- Default: `modal-heading` ### aria.describedby @@ -107,6 +147,14 @@ If this property is added, it will an additional class name to the modal content - Type: `String` - Required: No +### role + +If this property is added, it will override the default role of the modal. + +- Type: `String` +- Required: No +- Default: `dialog` + ### overlayClassName If this property is added, it will an additional class name to the modal overlay `div`. diff --git a/components/modal/frame.js b/components/modal/frame.js index e12131d24c8f0d..25fc9cd3514669 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -85,6 +85,7 @@ class ModalFrame extends Component { }, children, className, + role, style, } = this.props; @@ -93,7 +94,7 @@ class ModalFrame extends Component { className={ className } style={ style } ref={ this.containerRef } - role="dialog" + role={ role } aria-modal={ true } aria-label={ contentLabel } aria-labelledby={ labelledby } diff --git a/components/modal/header.js b/components/modal/header.js index c2fb3c8875999b..0b4cbade52c261 100644 --- a/components/modal/header.js +++ b/components/modal/header.js @@ -10,10 +10,12 @@ const ModalHeader = ( { icon, title, onClose, closeLabel } ) => { className={ 'components-modal__header' } >
- -

+ { icon && + + } +

{ title }

diff --git a/components/modal/index.js b/components/modal/index.js index ce634b526970f4..6eb76c46e0bb7c 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -92,6 +92,7 @@ class Modal extends Component { }, title, icon, + closeButtonLabel, children, ...otherProps } = this.props; @@ -116,6 +117,7 @@ class Modal extends Component { onRequestClose={ onRequestClose } { ...otherProps } > @@ -132,6 +134,8 @@ class Modal extends Component { Modal.defaultProps = { className: null, + role: 'dialog', + title: null, overlayClassName: null, onRequestClose: noop, focusOnMount: true, @@ -144,7 +148,7 @@ Modal.defaultProps = { /* accessibility */ contentLabel: null, aria: { - labelledby: null, + labelledby: 'modal-heading', describedby: null, }, }; From 6563c63182fa5e14d38776b8aad584ee0a10d2e4 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 2 May 2018 11:02:51 +0200 Subject: [PATCH 23/67] Updated documentation --- components/modal/README.md | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/components/modal/README.md b/components/modal/README.md index b2d591426db7da..a284d20ee6b0e7 100644 --- a/components/modal/README.md +++ b/components/modal/README.md @@ -9,18 +9,25 @@ The modal is used to create an accessible modal over an application. Render a screen overlay with a modal on top. ```jsx - - - - - - + + + + + ``` ## Implement close logic +For the modal to properly work it's important you implement the close logic for the modal properly. The following example shows you how to properly implement a modal. + ```js const { Component, Fragment } = wp.element; +const { Modal } = wp.components; + class MyModalWrapper extends Component { constructor() { super( ...arguments ); From 59255fff7982c01dd276b0143a80b6e3763db535 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 2 May 2018 11:04:25 +0200 Subject: [PATCH 24/67] Removed forwardRef from element| --- element/index.js | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/element/index.js b/element/index.js index d2970eeec3309e..ff524b1b7645f1 100644 --- a/element/index.js +++ b/element/index.js @@ -5,7 +5,6 @@ import { createElement, createContext, createRef, - forwardRef, Component, cloneElement, Children, @@ -47,18 +46,6 @@ export { createElement }; */ export { createRef }; -/** - * Allows a `ref` to be forwarded to a component further down the component - * tree. - * - * @param {Function} forwardFunction A function receiving the component's props and ref, - * allowing the ref to be mapped to a different prop in - * order to be passed further down the tree. - * - * @return {Component} The component with the forwarded ref. - */ -export { forwardRef }; - /** * Renders a given element into the target DOM node. * From c6ee481c3b458823eecce6254e2aab284361c1f6 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 2 May 2018 11:07:19 +0200 Subject: [PATCH 25/67] Changed default close label to Close dialog --- components/modal/header.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/modal/header.js b/components/modal/header.js index 0b4cbade52c261..7aee1dfc9dfe2d 100644 --- a/components/modal/header.js +++ b/components/modal/header.js @@ -3,7 +3,7 @@ import './style.scss'; import { __ } from '@wordpress/i18n'; const ModalHeader = ( { icon, title, onClose, closeLabel } ) => { - const label = closeLabel ? closeLabel : __( 'Close window' ); + const label = closeLabel ? closeLabel : __( 'Close dialog' ); return (
Date: Wed, 2 May 2018 11:53:00 +0200 Subject: [PATCH 26/67] Add modal-open className to body when modal is opened --- components/modal/index.js | 36 ++++++++++++++++++++---------------- element/index.js | 13 +++++++++++++ 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/components/modal/index.js b/components/modal/index.js index 6eb76c46e0bb7c..814d51277ac9e4 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -19,23 +19,16 @@ import './style.scss'; // Used to count the number of open modals. let parentElement, - modalCount = 0, openModalCount = 0; class Modal extends Component { constructor() { super( ...arguments ); - if ( ! parentElement ) { - parentElement = document.createElement( 'div' ); - document.body.appendChild( parentElement ); - } this.node = document.createElement( 'div' ); } componentDidMount() { - modalCount++; - const { isOpen } = this.props; if ( isOpen ) { this.openModal(); @@ -56,30 +49,40 @@ class Modal extends Component { if ( isOpen ) { this.closeModal(); } - - modalCount--; - if ( modalCount === 0 ) { - document.body.removeChild( parentElement ); - parentElement = null; - } } openModal() { openModalCount++; - ariaHelper.hideApp( parentElement ); + if ( openModalCount === 1 ) { + this.openFirstModal(); + } parentElement.appendChild( this.node ); } + openFirstModal() { + parentElement = document.createElement( 'div' ); + document.body.appendChild( parentElement ); + ariaHelper.hideApp( parentElement ); + document.body.classList.add( this.props.bodyOpenClassName ); + } + closeModal() { openModalCount--; parentElement.removeChild( this.node ); if ( openModalCount === 0 ) { - ariaHelper.showApp(); + this.closeLastModal(); } } + closeLastModal() { + document.body.classList.remove( this.props.bodyOpenClassName ); + ariaHelper.showApp(); + document.body.removeChild( parentElement ); + parentElement = null; + } + render() { const { isOpen, @@ -134,9 +137,10 @@ class Modal extends Component { Modal.defaultProps = { className: null, + overlayClassName: null, + bodyOpenClassName: 'modal-open', role: 'dialog', title: null, - overlayClassName: null, onRequestClose: noop, focusOnMount: true, shouldCloseOnEsc: true, diff --git a/element/index.js b/element/index.js index ff524b1b7645f1..d2970eeec3309e 100644 --- a/element/index.js +++ b/element/index.js @@ -5,6 +5,7 @@ import { createElement, createContext, createRef, + forwardRef, Component, cloneElement, Children, @@ -46,6 +47,18 @@ export { createElement }; */ export { createRef }; +/** + * Allows a `ref` to be forwarded to a component further down the component + * tree. + * + * @param {Function} forwardFunction A function receiving the component's props and ref, + * allowing the ref to be mapped to a different prop in + * order to be passed further down the tree. + * + * @return {Component} The component with the forwarded ref. + */ +export { forwardRef }; + /** * Renders a given element into the target DOM node. * From c4b433b00152af1ff9559b620f1b4f6421658623 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 2 May 2018 12:07:33 +0200 Subject: [PATCH 27/67] Added documentation to modal/index.js --- components/modal/index.js | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/components/modal/index.js b/components/modal/index.js index 814d51277ac9e4..1a4aa14ef4eefa 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -28,6 +28,9 @@ class Modal extends Component { this.node = document.createElement( 'div' ); } + /** + * Opens the modal if the initial isOpen prop is true. + */ componentDidMount() { const { isOpen } = this.props; if ( isOpen ) { @@ -35,6 +38,11 @@ class Modal extends Component { } } + /** + * Opens or closes the modal based on whether the isOpen prop changed. + * + * @param {Object} prevProps The previous props. + */ componentDidUpdate( prevProps ) { const openStateChanged = this.props.isOpen !== prevProps.isOpen; if ( openStateChanged && ! prevProps.isOpen ) { @@ -44,6 +52,9 @@ class Modal extends Component { } } + /** + * Closes the modal if it is open before unmount. + */ componentWillUnmount() { const { isOpen } = this.props; if ( isOpen ) { @@ -51,6 +62,11 @@ class Modal extends Component { } } + /** + * Appends the modal's node to the DOM, so the portal can render the + * modal in it. Also calls the openFirstModal when this is the first modal to be + * opened. + */ openModal() { openModalCount++; @@ -60,6 +76,13 @@ class Modal extends Component { parentElement.appendChild( this.node ); } + /** + * Prepares the DOM for this modal and any additional modal to be mounted. + * + * It appends an additional div to the body for the modals to be rendered in, + * it hides any other elements from screen-readers and adds an additional class + * to the body to prevent scrolling while the modal is open. + */ openFirstModal() { parentElement = document.createElement( 'div' ); document.body.appendChild( parentElement ); @@ -67,6 +90,10 @@ class Modal extends Component { document.body.classList.add( this.props.bodyOpenClassName ); } + /** + * Removes the modal's node from the DOM. Also calls closeLastModal when this is + * the last modal to be closed. + */ closeModal() { openModalCount--; @@ -76,6 +103,10 @@ class Modal extends Component { } } + /** + * Cleans up the DOM after the last modal is closed and makes the app available + * for screen-readers again. + */ closeLastModal() { document.body.classList.remove( this.props.bodyOpenClassName ); ariaHelper.showApp(); @@ -83,6 +114,11 @@ class Modal extends Component { parentElement = null; } + /** + * Renders the modal. + * + * @return {WPElement} The modal element. + */ render() { const { isOpen, From cca2a0e08f3be2d6027d5bc44f297ada05e4eff5 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 2 May 2018 12:24:13 +0200 Subject: [PATCH 28/67] Removed aria-modal=true and explained why in aria-helper.js --- components/modal/aria-helper.js | 5 +++++ components/modal/frame.js | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/components/modal/aria-helper.js b/components/modal/aria-helper.js index 9c6e2d94b925c6..018ad6fc878909 100644 --- a/components/modal/aria-helper.js +++ b/components/modal/aria-helper.js @@ -11,6 +11,11 @@ let hiddenElements = [], * the provided element, script elements and elements that already have * an `aria-hidden="true"` attribute. * + * The reason we do this is because `aria-modal="true"` currently is bugged + * in Safari, and support is spotty in other browsers overall. In the future + * we should consider removing these helper functions in favor of + * `aria-modal="true"`. + * * @param {Element} unhiddenElement The element that should not be hidden. */ export function hideApp( unhiddenElement ) { diff --git a/components/modal/frame.js b/components/modal/frame.js index 25fc9cd3514669..1e645c91c64969 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -32,6 +32,9 @@ class ModalFrame extends Component { this.focusFirstTabbable = this.focusFirstTabbable.bind( this ); } + /** + * Focuses the first tabbable element when props.focusOnMount is true. + */ componentDidMount() { // Focus on mount if ( this.props.focusOnMount ) { @@ -95,7 +98,6 @@ class ModalFrame extends Component { style={ style } ref={ this.containerRef } role={ role } - aria-modal={ true } aria-label={ contentLabel } aria-labelledby={ labelledby } aria-describedby={ describedby }> From 21a722c3d805d96d6b680fd2043d81fd49f0d031 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 2 May 2018 13:30:08 +0200 Subject: [PATCH 29/67] Documented modal/frame.js --- components/modal/frame.js | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/components/modal/frame.js b/components/modal/frame.js index 1e645c91c64969..f51326cfa0be3b 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -42,6 +42,9 @@ class ModalFrame extends Component { } } + /** + * Focuses the first tabbable element. + */ focusFirstTabbable() { // Required because the node is appended to the DOM after rendering. const { setTimeout } = this.props; @@ -53,18 +56,35 @@ class ModalFrame extends Component { }, 0 ); } + /** + * Callback function called when clicked outside the modal. + * + * @param {Object} event Mouse click event. + */ handleClickOutside( event ) { if ( this.props.shouldCloseOnClickOutside ) { this.onRequestClose( event ); } } + /** + * Callback function called when a key is pressed. + * + * @param {Object} event Key down event. + */ handleKeyDown( event ) { if ( event.keyCode === ESCAPE ) { this.handleEscapeKeyDown( event ); } } + /** + * Handles a escape key down event. + * + * Calls onRequestClose and prevents default key press behaviour. + * + * @param {Object} event Key down event. + */ handleEscapeKeyDown( event ) { if ( this.props.shouldCloseOnEsc ) { event.preventDefault(); @@ -72,6 +92,11 @@ class ModalFrame extends Component { } } + /** + * Calls the onRequestClose callback props when it is available. + * + * @param {Object} event Event object. + */ onRequestClose( event ) { const { onRequestClose } = this.props; if ( onRequestClose ) { @@ -79,6 +104,11 @@ class ModalFrame extends Component { } } + /** + * Renders the modal frame element. + * + * @return {WPElement} The modal frame element. + */ render() { const { contentLabel, From e7a8f4f9ff4f13b21c2ffbbc7ae149cec98f590f Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 29 May 2018 10:38:40 +0200 Subject: [PATCH 30/67] Addressed eslint issues --- components/modal/aria-helper.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/modal/aria-helper.js b/components/modal/aria-helper.js index 018ad6fc878909..d17719121a676d 100644 --- a/components/modal/aria-helper.js +++ b/components/modal/aria-helper.js @@ -23,11 +23,11 @@ export function hideApp( unhiddenElement ) { return; } const elements = document.body.children; - forEach( elements, element => { + forEach( elements, ( element ) => { if ( element === unhiddenElement || element.tagName === 'SCRIPT' || - element.hasAttribute( 'aria-hidden', 'true') + element.hasAttribute( 'aria-hidden', 'true' ) ) { return; } @@ -45,7 +45,7 @@ export function showApp() { if ( ! isHidden ) { return; } - forEach( hiddenElements, element => { + forEach( hiddenElements, ( element ) => { element.removeAttribute( 'aria-hidden' ); } ); hiddenElements = []; From 94a3216d873683f30aa18bcdf239d7ee55ca99ba Mon Sep 17 00:00:00 2001 From: Joen Asmussen Date: Wed, 6 Jun 2018 10:03:18 +0200 Subject: [PATCH 31/67] Polish the visuals a bit. --- components/modal/style.scss | 81 ++++++++++---------- edit-post/assets/stylesheets/_variables.scss | 1 + 2 files changed, 42 insertions(+), 40 deletions(-) diff --git a/components/modal/style.scss b/components/modal/style.scss index 756ec33e15fae0..e05232a3771fba 100644 --- a/components/modal/style.scss +++ b/components/modal/style.scss @@ -1,13 +1,14 @@ +// The scrim behind the modal window. .components-modal__screen-overlay { position: fixed; top: 0; - left: 0; right: 0; bottom: 0; - background-color: rgba(0, 0, 0, 0.6); + left: 0; + background-color: rgba( $white, .4 ); z-index: z-index( '.components-modal__screen-overlay' ); - // on mobile the main content area has to scroll - // otherwise you can invoke the overscroll bounce on the non-scrolling container, causing (ノಠ益ಠ)ノ彡┻━┻ + + // On mobile the main content area has to scroll, otherwise you can invoke the overscroll bounce on the non-scrolling container. @include break-small { position: fixed; top: $admin-bar-height-big; @@ -18,42 +19,49 @@ } } +// The modal window element. .components-modal__frame { - position: absolute; - top: 50%; - left: 50%; - right: auto; - bottom: auto; - width: auto; - max-width: calc( 100vh - 200px ); - max-height: 90%; - border: 0; - border-radius: 0; - margin-right: -50%; - transform: translate(-50%, -50%); - background-color: #fff; - outline: none; - box-shadow: 0 3px 25px #000; - // In small screens the content needs to be full width. - @media ( max-width: #{ ( $break-small ) } ) { - position: fixed; - top: 0; - bottom: 0; - right: 0; - left: 0; - margin: 0; - transform: initial; - max-width: none; - max-height: none; + position: fixed; + top: 0; + right: 0; + bottom: 0; + left: 0; + margin: 0; + + // Show slightly bigger on small screens. + @include break-small() { + position: absolute; + right: auto; + bottom: auto; + max-width: calc( 100% - #{ $panel-padding } - #{ $panel-padding } ); + margin-right: -50%; + transform: translate( -50%, 0 ); + top: $panel-padding; + left: 50%; + height: 90%; } + + // Show pretty big on desktop breakpoints. + @include break-medium () { + max-width: calc( #{ $break-medium } - #{ $panel-padding } - #{ $panel-padding } ); + transform: translate( -50%, -30% ); + top: 30%; + left: 50%; + height: 70%; + } + + border: 1px solid $light-gray-500; + background-color: $white; + box-shadow: $shadow-modal; + outline: none; } .components-modal__header { box-sizing: border-box; height: $header-height; border-bottom: 1px solid $light-gray-500; - padding: 10px; + padding: $item-spacing $item-spacing $item-spacing $panel-padding; display: flex; flex-direction: row; align-items: stretch; @@ -72,7 +80,6 @@ h1 { font-size: 1em; font-weight: normal; - padding-left: 5px; } span { @@ -89,16 +96,10 @@ .components-modal__content { // The height of the content is the height of it's parent, minus the header. after that, the offset was 3px. - max-height: calc( 100vh - 200px - #{ $header-height } ); + height: calc( 100% - #{ $header-height } - #{ $admin-bar-height } ); overflow: auto; - padding: 10px; + padding: $panel-padding; z-index: z-index( '.components-modal__content' ); - - @media ( max-width: #{ ( $break-small ) } ) { - max-height: calc( 100vh ); - } } @include editor-left( '.components-modal__screen-overlay' ); - - diff --git a/edit-post/assets/stylesheets/_variables.scss b/edit-post/assets/stylesheets/_variables.scss index 51f5ad40d269a8..dad924c09c08f0 100644 --- a/edit-post/assets/stylesheets/_variables.scss +++ b/edit-post/assets/stylesheets/_variables.scss @@ -29,6 +29,7 @@ $admin-sidebar-width-collapsed: 36px; $shadow-popover: 0 3px 20px rgba( $dark-gray-900, .1 ), 0 1px 3px rgba( $dark-gray-900, .1 ); $shadow-toolbar: 0 2px 10px rgba( $dark-gray-900, .1 ), 0 0 2px rgba( $dark-gray-900, .1 ); $shadow-below-only: 0 5px 10px rgba( $dark-gray-900, .1 ), 0 2px 2px rgba( $dark-gray-900, .1 ); +$shadow-modal: 0 3px 30px rgba( $dark-gray-900, .2 ); // Editor Widths $sidebar-width: 280px; From 6ac30dd7997bb0ddb8a027cfa8bb5afc477130ee Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Thu, 7 Jun 2018 15:58:54 +0200 Subject: [PATCH 32/67] Disabled jsx-a11y/no-static-element-interactions in render function of withFocusContain instead of whole file --- components/higher-order/with-focus-contain/index.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/components/higher-order/with-focus-contain/index.js b/components/higher-order/with-focus-contain/index.js index b15dfc6efdc5c0..645d37e7546a10 100644 --- a/components/higher-order/with-focus-contain/index.js +++ b/components/higher-order/with-focus-contain/index.js @@ -1,4 +1,3 @@ -/* eslint-disable jsx-a11y/no-static-element-interactions */ /** * WordPress dependencies */ @@ -40,13 +39,18 @@ const withFocusContain = ( WrappedComponent ) => { } render() { + // Disable reason: this component is non-interactive, but must capture + // events from the wrapped component to determine when the Tab key is used. + /* eslint-disable jsx-a11y/no-static-element-interactions */ return (
+ ref={ this.focusContainRef } + >
); + /* eslint-enable jsx-a11y/no-static-element-interactions */ } }; }; From eda32a6d133c14ba8c2dc5f4049e54dbe62fa676 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Mon, 18 Jun 2018 12:18:02 +0200 Subject: [PATCH 33/67] Addressed CR concerns --- .../higher-order/with-global-events/index.js | 2 +- components/modal/README.md | 24 ++++++++++++------- components/modal/header.js | 17 +++++++++---- components/modal/index.js | 3 --- components/modal/style.scss | 17 +++++++------ 5 files changed, 36 insertions(+), 27 deletions(-) diff --git a/components/higher-order/with-global-events/index.js b/components/higher-order/with-global-events/index.js index 321f79ef5f258e..6fe5b8a107d610 100644 --- a/components/higher-order/with-global-events/index.js +++ b/components/higher-order/with-global-events/index.js @@ -64,7 +64,7 @@ function withGlobalEvents( eventTypesToHandlers ) { } return forwardRef( ( props, ref ) => { - return ; + return ; } ); }, 'withGlobalEvents' ); } diff --git a/components/modal/README.md b/components/modal/README.md index a284d20ee6b0e7..5f5acabfe19316 100644 --- a/components/modal/README.md +++ b/components/modal/README.md @@ -3,7 +3,7 @@ Modal The modal is used to create an accessible modal over an application. -**Note:** The API for this modal has been mimicked to resemble `react-modal`. +**Note:** The API for this modal has been mimicked to resemble [`react-modal`](https://github.com/reactjs/react-modal). ## Usage @@ -13,6 +13,9 @@ Render a screen overlay with a modal on top. title="My Modal" onRequestClose={ closeFunction } isOpen={ openState } + aria={ { + describedby: "modal-description", + } } > @@ -34,12 +37,9 @@ class MyModalWrapper extends Component { this.state = { isOpen: true, } - } - closeModal() { - if ( this.state.isOpen ) { - this.setState( { isOpen: false } ); - } + this.openModal = this.openModal.bind( this ); + this.closeModal = this.closeModal.bind( this ); } openModal() { @@ -48,15 +48,21 @@ class MyModalWrapper extends Component { } } + closeModal() { + if ( this.state.isOpen ) { + this.setState( { isOpen: false } ); + } + } + render() { return ( - + - diff --git a/components/modal/header.js b/components/modal/header.js index 7aee1dfc9dfe2d..dd37d99c9fb6c5 100644 --- a/components/modal/header.js +++ b/components/modal/header.js @@ -1,21 +1,28 @@ +/** + * WordPress dependencies + */ +import { __ } from '@wordpress/i18n'; + +/** + * Internal dependencies. + */ import IconButton from '../icon-button'; import './style.scss'; -import { __ } from '@wordpress/i18n'; const ModalHeader = ( { icon, title, onClose, closeLabel } ) => { const label = closeLabel ? closeLabel : __( 'Close dialog' ); return (
-
+
{ icon && -
diff --git a/components/modal/index.js b/components/modal/index.js index 1a4aa14ef4eefa..be02e3dd49749b 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -172,8 +172,6 @@ class Modal extends Component { } Modal.defaultProps = { - className: null, - overlayClassName: null, bodyOpenClassName: 'modal-open', role: 'dialog', title: null, @@ -186,7 +184,6 @@ Modal.defaultProps = { overlay: null, }, /* accessibility */ - contentLabel: null, aria: { labelledby: 'modal-heading', describedby: null, diff --git a/components/modal/style.scss b/components/modal/style.scss index e05232a3771fba..50176065eb3aa4 100644 --- a/components/modal/style.scss +++ b/components/modal/style.scss @@ -66,25 +66,24 @@ flex-direction: row; align-items: stretch; justify-content: space-between; - z-index: z-index( '.edit-post-header' ); left: 0; right: 0; - div { + &-heading-container { align-items: center; flex-grow: 1; display: flex; flex-direction: row; justify-content: left; + } - h1 { - font-size: 1em; - font-weight: normal; - } + &-heading { + font-size: 1em; + font-weight: normal; + } - span { - display: inline-block; - } + &-icon-container { + display: inline-block; svg { max-width: $icon-button-size; From dde71f2c1e838a24734c68787f0523ca078fd830 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Mon, 18 Jun 2018 13:07:45 +0200 Subject: [PATCH 34/67] Removed unused variable --- components/higher-order/with-global-events/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/higher-order/with-global-events/index.js b/components/higher-order/with-global-events/index.js index 6fe5b8a107d610..e6859fb7f87cb2 100644 --- a/components/higher-order/with-global-events/index.js +++ b/components/higher-order/with-global-events/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { forEach, noop } from 'lodash'; +import { forEach } from 'lodash'; /** * WordPress dependencies From ca8512a6fb689a79c40a3f259ffa608bf4696144 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Mon, 18 Jun 2018 15:09:39 +0200 Subject: [PATCH 35/67] Replaced focus.tabbables.find from @wordpress/utils with @wordpress/dom --- components/higher-order/with-focus-contain/index.js | 3 ++- components/modal/frame.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/components/higher-order/with-focus-contain/index.js b/components/higher-order/with-focus-contain/index.js index 645d37e7546a10..c9f76139c0c27e 100644 --- a/components/higher-order/with-focus-contain/index.js +++ b/components/higher-order/with-focus-contain/index.js @@ -2,7 +2,8 @@ * WordPress dependencies */ import { Component, createRef } from '@wordpress/element'; -import { focus, keycodes } from '@wordpress/utils'; +import { keycodes } from '@wordpress/utils'; +import { focus } from '@wordpress/dom'; const { TAB, diff --git a/components/modal/frame.js b/components/modal/frame.js index f51326cfa0be3b..04eeac163b1c7f 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -7,7 +7,8 @@ import clickOutside from 'react-click-outside'; * WordPress dependencies */ import { Component, compose, createRef } from '@wordpress/element'; -import { focus, keycodes } from '@wordpress/utils'; +import { keycodes } from '@wordpress/utils'; +import { focus } from '@wordpress/dom'; /** * Internal dependencies From b6ef31fed6c5c175f0ccab12f9767826cb418d12 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 20 Jun 2018 14:11:40 +0200 Subject: [PATCH 36/67] Fixed failing tests after updating react-test-renderer to version 16.4.1 for forwardRef support --- components/higher-order/with-global-events/index.js | 4 +++- .../higher-order/with-global-events/test/index.js | 12 +++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/components/higher-order/with-global-events/index.js b/components/higher-order/with-global-events/index.js index e6859fb7f87cb2..1b93ceb40bf319 100644 --- a/components/higher-order/with-global-events/index.js +++ b/components/higher-order/with-global-events/index.js @@ -55,7 +55,9 @@ function withGlobalEvents( eventTypesToHandlers ) { handleRef( el ) { this.wrappedRef = el; - this.props.forwardedRef( el ); + if ( this.props.forwardRef ) { + this.props.forwardedRef( el ); + } } render() { diff --git a/components/higher-order/with-global-events/test/index.js b/components/higher-order/with-global-events/test/index.js index 9054e5515d2524..c3ccadc7af5982 100644 --- a/components/higher-order/with-global-events/test/index.js +++ b/components/higher-order/with-global-events/test/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { mount } from 'enzyme'; +import TestRenderer from 'react-test-renderer'; /** * External dependencies @@ -62,20 +62,22 @@ describe( 'withGlobalEvents', () => { resize: 'handleResize', } )( OriginalComponent ); - wrapper = mount( Hello ); + wrapper = TestRenderer.create( Hello ); } it( 'renders with original component', () => { mountEnhancedComponent(); - expect( wrapper.childAt( 0 ).childAt( 0 ).type() ).toBe( 'div' ); - expect( wrapper.childAt( 0 ).text() ).toBe( 'Hello' ); + expect( wrapper.root.findByType( 'div' ).children[ 0 ] ).toBe( 'Hello' ); } ); it( 'binds events from passed object', () => { mountEnhancedComponent(); - expect( Listener._instance.add ).toHaveBeenCalledWith( 'resize', wrapper.instance() ); + // Get the HOC wrapper instance + const hocInstance = wrapper.root.findByType( OriginalComponent ).parent.instance; + + expect( Listener._instance.add ).toHaveBeenCalledWith( 'resize', hocInstance ); } ); it( 'handles events', () => { From b74d1e6607bd0ae74af9e61f0c9f599976eae2fd Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Thu, 21 Jun 2018 11:19:25 +0200 Subject: [PATCH 37/67] CSS Tweaks --- components/modal/style.scss | 38 ++++++++++++-------------- edit-post/components/layout/style.scss | 2 ++ 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/components/modal/style.scss b/components/modal/style.scss index 50176065eb3aa4..9a23c2f0c6f684 100644 --- a/components/modal/style.scss +++ b/components/modal/style.scss @@ -68,28 +68,28 @@ justify-content: space-between; left: 0; right: 0; +} - &-heading-container { - align-items: center; - flex-grow: 1; - display: flex; - flex-direction: row; - justify-content: left; - } +.components-modal__header-heading-container { + align-items: center; + flex-grow: 1; + display: flex; + flex-direction: row; + justify-content: left; +} - &-heading { - font-size: 1em; - font-weight: normal; - } +.components-modal__header-heading { + font-size: 1em; + font-weight: normal; +} - &-icon-container { - display: inline-block; +.components-modal__header-icon-container { + display: inline-block; - svg { - max-width: $icon-button-size; - max-height: $icon-button-size; - padding: 8px; - } + svg { + max-width: $icon-button-size; + max-height: $icon-button-size; + padding: 8px; } } @@ -100,5 +100,3 @@ padding: $panel-padding; z-index: z-index( '.components-modal__content' ); } - -@include editor-left( '.components-modal__screen-overlay' ); diff --git a/edit-post/components/layout/style.scss b/edit-post/components/layout/style.scss index fa6899853fb066..e01a96aeccdfe4 100644 --- a/edit-post/components/layout/style.scss +++ b/edit-post/components/layout/style.scss @@ -56,6 +56,8 @@ @include editor-left('.components-notice-list'); @include editor-right('.components-notice-list'); +@include editor-left( '.components-modal__screen-overlay' ); + .edit-post-layout__metaboxes:not(:empty) { border-top: 1px solid $light-gray-500; margin-top: 10px; From dbac1974b4467c61af3c9efb0a2e8ede35e89ff0 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 26 Jun 2018 10:47:08 +0200 Subject: [PATCH 38/67] Fixed error when clicking outside of the modal --- components/higher-order/with-global-events/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/higher-order/with-global-events/index.js b/components/higher-order/with-global-events/index.js index 1b93ceb40bf319..7527ef8078f8e9 100644 --- a/components/higher-order/with-global-events/index.js +++ b/components/higher-order/with-global-events/index.js @@ -55,7 +55,7 @@ function withGlobalEvents( eventTypesToHandlers ) { handleRef( el ) { this.wrappedRef = el; - if ( this.props.forwardRef ) { + if ( this.props.forwardedRef ) { this.props.forwardedRef( el ); } } From 61ac9559a65b2b489fb6f1079cc29ed44b385afa Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 26 Jun 2018 12:54:47 +0200 Subject: [PATCH 39/67] Move focus to first element with tabindex=-1 on mount --- components/modal/frame.js | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/components/modal/frame.js b/components/modal/frame.js index 04eeac163b1c7f..485b04721a5391 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -2,6 +2,7 @@ * External dependencies */ import clickOutside from 'react-click-outside'; +import { forEach } from 'lodash'; /** * WordPress dependencies @@ -30,6 +31,7 @@ class ModalFrame extends Component { this.containerRef = createRef(); this.handleKeyDown = this.handleKeyDown.bind( this ); this.handleClickOutside = this.handleClickOutside.bind( this ); + this.focusFirstFocusable = this.focusFirstFocusable.bind( this ); this.focusFirstTabbable = this.focusFirstTabbable.bind( this ); } @@ -39,7 +41,24 @@ class ModalFrame extends Component { componentDidMount() { // Focus on mount if ( this.props.focusOnMount ) { - this.focusFirstTabbable(); + const { setTimeout } = this.props; + // Required because the node is appended to the DOM after rendering. + setTimeout( () => { + this.focusFirstTabbable(); + this.focusFirstFocusable(); + }, 0 ); + } + } + + focusFirstFocusable() { + const focusables = focus.focusable.find( this.containerRef.current ); + if ( focusables.length ) { + forEach( focusables, ( focusable ) => { + if ( focusable.hasAttribute( 'tabindex', -1 ) ) { + focusable.focus(); + return false; + } + } ); } } @@ -47,14 +66,10 @@ class ModalFrame extends Component { * Focuses the first tabbable element. */ focusFirstTabbable() { - // Required because the node is appended to the DOM after rendering. - const { setTimeout } = this.props; - setTimeout( () => { - const tabbables = focus.tabbable.find( this.containerRef.current ); - if ( tabbables.length ) { - tabbables[ 0 ].focus(); - } - }, 0 ); + const tabbables = focus.tabbable.find( this.containerRef.current ); + if ( tabbables.length ) { + tabbables[ 0 ].focus(); + } } /** From d94e4ef6ef841c25f1996bdbdc263d52312892ab Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 27 Jun 2018 13:35:43 +0200 Subject: [PATCH 40/67] Make sure the dic the modal is renderd in is apprended to the document's body/ DOM is ready before the modal is rendered --- components/modal/frame.js | 10 ++-------- components/modal/index.js | 34 ++++++++++++++++++++++------------ 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/components/modal/frame.js b/components/modal/frame.js index 485b04721a5391..76f4d3cb617387 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -18,7 +18,6 @@ import './style.scss'; import withFocusReturn from '../higher-order/with-focus-return'; import withFocusContain from '../higher-order/with-focus-contain'; import withGlobalEvents from '../higher-order/with-global-events'; -import withSafeTimeout from '../higher-order/with-safe-timeout'; const { ESCAPE, @@ -41,12 +40,8 @@ class ModalFrame extends Component { componentDidMount() { // Focus on mount if ( this.props.focusOnMount ) { - const { setTimeout } = this.props; - // Required because the node is appended to the DOM after rendering. - setTimeout( () => { - this.focusFirstTabbable(); - this.focusFirstFocusable(); - }, 0 ); + this.focusFirstTabbable(); + this.focusFirstFocusable(); } } @@ -156,7 +151,6 @@ class ModalFrame extends Component { export default compose( [ withFocusReturn, withFocusContain, - withSafeTimeout, clickOutside, withGlobalEvents( { keydown: 'handleKeyDown', diff --git a/components/modal/index.js b/components/modal/index.js index be02e3dd49749b..b6b0d8231447b8 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -25,17 +25,32 @@ class Modal extends Component { constructor() { super( ...arguments ); - this.node = document.createElement( 'div' ); + this.prepareDOM(); } /** - * Opens the modal if the initial isOpen prop is true. + * Prepares the DOM for the modals to be rendered. + * + * Every modal is mounted in a separate div appended to a parent div + * that is appended to the document body. + * + * The parent div will be created if it does not yet exist, and the + * separate div for this specific modal will be appended to that. */ - componentDidMount() { - const { isOpen } = this.props; - if ( isOpen ) { - this.openModal(); + prepareDOM() { + if ( ! parentElement ) { + parentElement = document.createElement( 'div' ); + document.body.appendChild( parentElement ); } + this.node = document.createElement( 'div' ); + parentElement.appendChild( this.node ); + } + + /** + * Removes the specific mounting point for this modal from the DOM. + */ + cleanDOM() { + parentElement.removeChild( this.node ); } /** @@ -73,7 +88,6 @@ class Modal extends Component { if ( openModalCount === 1 ) { this.openFirstModal(); } - parentElement.appendChild( this.node ); } /** @@ -84,8 +98,6 @@ class Modal extends Component { * to the body to prevent scrolling while the modal is open. */ openFirstModal() { - parentElement = document.createElement( 'div' ); - document.body.appendChild( parentElement ); ariaHelper.hideApp( parentElement ); document.body.classList.add( this.props.bodyOpenClassName ); } @@ -97,7 +109,7 @@ class Modal extends Component { closeModal() { openModalCount--; - parentElement.removeChild( this.node ); + this.cleanDOM(); if ( openModalCount === 0 ) { this.closeLastModal(); } @@ -110,8 +122,6 @@ class Modal extends Component { closeLastModal() { document.body.classList.remove( this.props.bodyOpenClassName ); ariaHelper.showApp(); - document.body.removeChild( parentElement ); - parentElement = null; } /** From 6eb388619654d57560bf3d50fac75647ca2ab587 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 27 Jun 2018 13:51:40 +0200 Subject: [PATCH 41/67] Addressed minor codestyle issues in frame.js --- components/modal/frame.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/components/modal/frame.js b/components/modal/frame.js index 76f4d3cb617387..0ac9914586e001 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -45,16 +45,17 @@ class ModalFrame extends Component { } } + /** + * Focusses the first focusable with a `tabindex` of -1. + */ focusFirstFocusable() { const focusables = focus.focusable.find( this.containerRef.current ); - if ( focusables.length ) { - forEach( focusables, ( focusable ) => { - if ( focusable.hasAttribute( 'tabindex', -1 ) ) { - focusable.focus(); - return false; - } - } ); - } + forEach( focusables, ( focusable ) => { + if ( focusable.getAttribute( 'tabindex' ) === '-1' ) { + focusable.focus(); + return false; + } + } ); } /** From ee6f5207fba9b6b35728196b5f83189f227f21c4 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 27 Jun 2018 15:23:03 +0200 Subject: [PATCH 42/67] Fixed bug when opening modal the second time --- components/modal/frame.js | 15 --------------- components/modal/index.js | 2 +- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/components/modal/frame.js b/components/modal/frame.js index 0ac9914586e001..63b6225cd3be47 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -30,7 +30,6 @@ class ModalFrame extends Component { this.containerRef = createRef(); this.handleKeyDown = this.handleKeyDown.bind( this ); this.handleClickOutside = this.handleClickOutside.bind( this ); - this.focusFirstFocusable = this.focusFirstFocusable.bind( this ); this.focusFirstTabbable = this.focusFirstTabbable.bind( this ); } @@ -41,23 +40,9 @@ class ModalFrame extends Component { // Focus on mount if ( this.props.focusOnMount ) { this.focusFirstTabbable(); - this.focusFirstFocusable(); } } - /** - * Focusses the first focusable with a `tabindex` of -1. - */ - focusFirstFocusable() { - const focusables = focus.focusable.find( this.containerRef.current ); - forEach( focusables, ( focusable ) => { - if ( focusable.getAttribute( 'tabindex' ) === '-1' ) { - focusable.focus(); - return false; - } - } ); - } - /** * Focuses the first tabbable element. */ diff --git a/components/modal/index.js b/components/modal/index.js index b6b0d8231447b8..274c0d021e83a6 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -75,6 +75,7 @@ class Modal extends Component { if ( isOpen ) { this.closeModal(); } + this.cleanDOM(); } /** @@ -109,7 +110,6 @@ class Modal extends Component { closeModal() { openModalCount--; - this.cleanDOM(); if ( openModalCount === 0 ) { this.closeLastModal(); } From ca59960617a55f068fe1e214a2282c6f00e97f37 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 27 Jun 2018 15:28:51 +0200 Subject: [PATCH 43/67] Removed unused import --- components/modal/frame.js | 1 - 1 file changed, 1 deletion(-) diff --git a/components/modal/frame.js b/components/modal/frame.js index 63b6225cd3be47..c7412a50dbe001 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -2,7 +2,6 @@ * External dependencies */ import clickOutside from 'react-click-outside'; -import { forEach } from 'lodash'; /** * WordPress dependencies From e2a50a18a9a3673633eba454ff68c5585f682f2e Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Wed, 27 Jun 2018 15:32:31 +0200 Subject: [PATCH 44/67] replaced react-click-outside with internal withFocusOutside HOC --- components/modal/frame.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/components/modal/frame.js b/components/modal/frame.js index c7412a50dbe001..03a2263c31110c 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -1,8 +1,3 @@ -/** - * External dependencies - */ -import clickOutside from 'react-click-outside'; - /** * WordPress dependencies */ @@ -17,6 +12,7 @@ import './style.scss'; import withFocusReturn from '../higher-order/with-focus-return'; import withFocusContain from '../higher-order/with-focus-contain'; import withGlobalEvents from '../higher-order/with-global-events'; +import withFocusOutside from '../higher-order/with-focus-outside'; const { ESCAPE, @@ -28,7 +24,7 @@ class ModalFrame extends Component { this.containerRef = createRef(); this.handleKeyDown = this.handleKeyDown.bind( this ); - this.handleClickOutside = this.handleClickOutside.bind( this ); + this.handleFocusOutside = this.handleFocusOutside.bind( this ); this.focusFirstTabbable = this.focusFirstTabbable.bind( this ); } @@ -57,7 +53,7 @@ class ModalFrame extends Component { * * @param {Object} event Mouse click event. */ - handleClickOutside( event ) { + handleFocusOutside( event ) { if ( this.props.shouldCloseOnClickOutside ) { this.onRequestClose( event ); } @@ -136,7 +132,7 @@ class ModalFrame extends Component { export default compose( [ withFocusReturn, withFocusContain, - clickOutside, + withFocusOutside, withGlobalEvents( { keydown: 'handleKeyDown', } ), From 9968113dcf8c7daf3b556a2ede9547c84f83edaa Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Mon, 2 Jul 2018 11:47:28 +0200 Subject: [PATCH 45/67] Replaced withFocusContain with withConstrainedTabbing --- .../higher-order/with-focus-contain/index.js | 59 ------------------- components/modal/frame.js | 4 +- 2 files changed, 2 insertions(+), 61 deletions(-) delete mode 100644 components/higher-order/with-focus-contain/index.js diff --git a/components/higher-order/with-focus-contain/index.js b/components/higher-order/with-focus-contain/index.js deleted file mode 100644 index c9f76139c0c27e..00000000000000 --- a/components/higher-order/with-focus-contain/index.js +++ /dev/null @@ -1,59 +0,0 @@ -/** - * WordPress dependencies - */ -import { Component, createRef } from '@wordpress/element'; -import { keycodes } from '@wordpress/utils'; -import { focus } from '@wordpress/dom'; - -const { - TAB, -} = keycodes; - -const withFocusContain = ( WrappedComponent ) => { - return class extends Component { - constructor() { - super( ...arguments ); - - this.focusContainRef = createRef(); - this.handleTabBehaviour = this.handleTabBehaviour.bind( this ); - } - - handleTabBehaviour( event ) { - if ( event.keyCode !== TAB ) { - return; - } - - const tabbables = focus.tabbable.find( this.focusContainRef.current ); - if ( ! tabbables.length ) { - return; - } - const firstTabbable = tabbables[ 0 ]; - const lastTabbable = tabbables[ tabbables.length - 1 ]; - - if ( event.shiftKey && event.target === firstTabbable ) { - event.preventDefault(); - lastTabbable.focus(); - } else if ( ! event.shiftKey && event.target === lastTabbable ) { - event.preventDefault(); - firstTabbable.focus(); - } - } - - render() { - // Disable reason: this component is non-interactive, but must capture - // events from the wrapped component to determine when the Tab key is used. - /* eslint-disable jsx-a11y/no-static-element-interactions */ - return ( -
- -
- ); - /* eslint-enable jsx-a11y/no-static-element-interactions */ - } - }; -}; - -export default withFocusContain; diff --git a/components/modal/frame.js b/components/modal/frame.js index 03a2263c31110c..7b802b8f9c65e3 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -10,7 +10,7 @@ import { focus } from '@wordpress/dom'; */ import './style.scss'; import withFocusReturn from '../higher-order/with-focus-return'; -import withFocusContain from '../higher-order/with-focus-contain'; +import withConstrainedTabbing from '../higher-order/with-constrained-tabbing'; import withGlobalEvents from '../higher-order/with-global-events'; import withFocusOutside from '../higher-order/with-focus-outside'; @@ -131,7 +131,7 @@ class ModalFrame extends Component { export default compose( [ withFocusReturn, - withFocusContain, + withConstrainedTabbing, withFocusOutside, withGlobalEvents( { keydown: 'handleKeyDown', From 5dc9b58080e7dd4e3377f7c6f9a243b3c90b8df1 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Mon, 2 Jul 2018 11:53:26 +0200 Subject: [PATCH 46/67] Replaced withFocusOutside with react-click-outside again --- components/modal/frame.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/components/modal/frame.js b/components/modal/frame.js index 7b802b8f9c65e3..77cc0a013008c4 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -5,6 +5,11 @@ import { Component, compose, createRef } from '@wordpress/element'; import { keycodes } from '@wordpress/utils'; import { focus } from '@wordpress/dom'; +/** + * External dependencies + */ +import clickOutside from 'react-click-outside'; + /** * Internal dependencies */ @@ -12,7 +17,6 @@ import './style.scss'; import withFocusReturn from '../higher-order/with-focus-return'; import withConstrainedTabbing from '../higher-order/with-constrained-tabbing'; import withGlobalEvents from '../higher-order/with-global-events'; -import withFocusOutside from '../higher-order/with-focus-outside'; const { ESCAPE, @@ -24,7 +28,7 @@ class ModalFrame extends Component { this.containerRef = createRef(); this.handleKeyDown = this.handleKeyDown.bind( this ); - this.handleFocusOutside = this.handleFocusOutside.bind( this ); + this.handleClickOutside = this.handleClickOutside.bind( this ); this.focusFirstTabbable = this.focusFirstTabbable.bind( this ); } @@ -53,7 +57,7 @@ class ModalFrame extends Component { * * @param {Object} event Mouse click event. */ - handleFocusOutside( event ) { + handleClickOutside( event ) { if ( this.props.shouldCloseOnClickOutside ) { this.onRequestClose( event ); } @@ -132,7 +136,7 @@ class ModalFrame extends Component { export default compose( [ withFocusReturn, withConstrainedTabbing, - withFocusOutside, + clickOutside, withGlobalEvents( { keydown: 'handleKeyDown', } ), From 7f829440f5e03e365cf301840fe23e33dae1221b Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 11:34:07 +0200 Subject: [PATCH 47/67] Replaced @wordpress/utils keycodes with @wordpress/keycodes in frame.js --- components/modal/frame.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/components/modal/frame.js b/components/modal/frame.js index 77cc0a013008c4..e83eda433a8518 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -2,7 +2,7 @@ * WordPress dependencies */ import { Component, compose, createRef } from '@wordpress/element'; -import { keycodes } from '@wordpress/utils'; +import { ESCAPE } from '@wordpress/keycodes'; import { focus } from '@wordpress/dom'; /** @@ -18,10 +18,6 @@ import withFocusReturn from '../higher-order/with-focus-return'; import withConstrainedTabbing from '../higher-order/with-constrained-tabbing'; import withGlobalEvents from '../higher-order/with-global-events'; -const { - ESCAPE, -} = keycodes; - class ModalFrame extends Component { constructor() { super( ...arguments ); From 146f5620157a304136ceb36ccd1fc226ce4598df Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 13:36:39 +0200 Subject: [PATCH 48/67] don't pass props.aria.labelledby to frame div when props.contentLabel is provided --- components/modal/frame.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/modal/frame.js b/components/modal/frame.js index e83eda433a8518..153659c8da23bd 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -121,7 +121,7 @@ class ModalFrame extends Component { ref={ this.containerRef } role={ role } aria-label={ contentLabel } - aria-labelledby={ labelledby } + aria-labelledby={ contentLabel ? null : labelledby } aria-describedby={ describedby }> { children }
From 30ab946287e7fd4da18c0cb69d61f725b158ff75 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 14:01:39 +0200 Subject: [PATCH 49/67] Added logic and tests for aria-helper to not hide (implicitely) live regions --- components/modal/aria-helper.js | 38 ++++++++++++--- components/modal/test/aria-helper.js | 69 ++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 components/modal/test/aria-helper.js diff --git a/components/modal/aria-helper.js b/components/modal/aria-helper.js index d17719121a676d..7951cc345d8c92 100644 --- a/components/modal/aria-helper.js +++ b/components/modal/aria-helper.js @@ -8,8 +8,8 @@ let hiddenElements = [], /** * Hides all elements in the body element from screen-readers except - * the provided element, script elements and elements that already have - * an `aria-hidden="true"` attribute. + * the provided element and elements that should not be hidden from + * screen-readers. * * The reason we do this is because `aria-modal="true"` currently is bugged * in Safari, and support is spotty in other browsers overall. In the future @@ -25,18 +25,42 @@ export function hideApp( unhiddenElement ) { const elements = document.body.children; forEach( elements, ( element ) => { if ( - element === unhiddenElement || - element.tagName === 'SCRIPT' || - element.hasAttribute( 'aria-hidden', 'true' ) + element === unhiddenElement ) { return; } - element.setAttribute( 'aria-hidden', 'true' ); - hiddenElements.push( element ); + if ( elementShouldBeHidden( element ) ) { + element.setAttribute( 'aria-hidden', 'true' ); + hiddenElements.push( element ); + } } ); isHidden = true; } +/** + * Determines if the passed element should not be hidden from screen readers. + * + * @param {HTMLElement} element The element that should be checked. + * + * @return {boolean} Whether the element should not be hidden from screen-readers. + */ +export function elementShouldBeHidden( element ) { + const liveRegionAriaRoles = [ + 'alert', + 'status', + 'log', + 'marquee', + 'timer', + ]; + const role = element.getAttribute( 'role' ); + return ! ( + element.tagName === 'SCRIPT' || + element.hasAttribute( 'aria-hidden' ) || + element.hasAttribute( 'aria-live' ) || + liveRegionAriaRoles.includes( role ) + ); +} + /** * Makes all elements in the body that have been hidden by `hideApp` * visible again to screen-readers. diff --git a/components/modal/test/aria-helper.js b/components/modal/test/aria-helper.js new file mode 100644 index 00000000000000..0ee9b0b947aa51 --- /dev/null +++ b/components/modal/test/aria-helper.js @@ -0,0 +1,69 @@ +/** + * Internal dependencies + */ +import { elementShouldBeHidden } from '../aria-helper'; + +describe( 'aria-helper', () => { + describe( 'elementShouldBeHidden', () => { + it( 'should return true when a div element without attributes is passed', () => { + const element = document.createElement( 'div' ); + + expect( elementShouldBeHidden( element ) ).toBe( true ); + } ); + + it( 'should return false when a script element without attributes is passed', () => { + const element = document.createElement( 'script' ); + + expect( elementShouldBeHidden( element ) ).toBe( false ); + } ); + + it( 'should return false when an element has the aria-hidden attribute with value "true"', () => { + const element = document.createElement( 'div' ); + element.setAttribute( 'aria-hidden', 'true' ); + + expect( elementShouldBeHidden( element ) ).toBe( false ); + } ); + + it( 'should return false when an element has the aria-hidden attribute with value "false"', () => { + const element = document.createElement( 'div' ); + element.setAttribute( 'aria-hidden', 'false' ); + + expect( elementShouldBeHidden( element ) ).toBe( false ); + } ); + + it( 'should return false when an element has the role attribute with value "alert"', () => { + const element = document.createElement( 'div' ); + element.setAttribute( 'role', 'alert' ); + + expect( elementShouldBeHidden( element ) ).toBe( false ); + } ); + + it( 'should return false when an element has the role attribute with value "status"', () => { + const element = document.createElement( 'div' ); + element.setAttribute( 'role', 'status' ); + + expect( elementShouldBeHidden( element ) ).toBe( false ); + } ); + + it( 'should return false when an element has the role attribute with value "log"', () => { + const element = document.createElement( 'div' ); + element.setAttribute( 'role', 'log' ); + + expect( elementShouldBeHidden( element ) ).toBe( false ); + } ); + + it( 'should return false when an element has the role attribute with value "marquee"', () => { + const element = document.createElement( 'div' ); + element.setAttribute( 'role', 'marquee' ); + + expect( elementShouldBeHidden( element ) ).toBe( false ); + } ); + + it( 'should return false when an element has the role attribute with value "timer"', () => { + const element = document.createElement( 'div' ); + element.setAttribute( 'role', 'timer' ); + + expect( elementShouldBeHidden( element ) ).toBe( false ); + } ); + } ); +} ); From bd1050b26a1c99810af396a4c597615050153f62 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 14:25:44 +0200 Subject: [PATCH 50/67] Removed isOpen props from the modal --- components/modal/README.md | 17 +++++------ components/modal/index.js | 58 ++++++++++---------------------------- 2 files changed, 24 insertions(+), 51 deletions(-) diff --git a/components/modal/README.md b/components/modal/README.md index 5f5acabfe19316..e36b01b6f7f963 100644 --- a/components/modal/README.md +++ b/components/modal/README.md @@ -58,14 +58,15 @@ class MyModalWrapper extends Component { return ( - - - + { this.state.isOpen ? + + + + : null } ); } diff --git a/components/modal/index.js b/components/modal/index.js index 274c0d021e83a6..50ba5fec1e36cc 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -53,37 +53,12 @@ class Modal extends Component { parentElement.removeChild( this.node ); } - /** - * Opens or closes the modal based on whether the isOpen prop changed. - * - * @param {Object} prevProps The previous props. - */ - componentDidUpdate( prevProps ) { - const openStateChanged = this.props.isOpen !== prevProps.isOpen; - if ( openStateChanged && ! prevProps.isOpen ) { - this.openModal(); - } else if ( openStateChanged && prevProps.isOpen ) { - this.closeModal(); - } - } - - /** - * Closes the modal if it is open before unmount. - */ - componentWillUnmount() { - const { isOpen } = this.props; - if ( isOpen ) { - this.closeModal(); - } - this.cleanDOM(); - } - /** * Appends the modal's node to the DOM, so the portal can render the * modal in it. Also calls the openFirstModal when this is the first modal to be * opened. */ - openModal() { + componentDidMount() { openModalCount++; if ( openModalCount === 1 ) { @@ -91,6 +66,20 @@ class Modal extends Component { } } + /** + * Removes the modal's node from the DOM. Also calls closeLastModal when this is + * the last modal to be closed. + */ + componentWillUnmount() { + openModalCount--; + + if ( openModalCount === 0 ) { + this.closeLastModal(); + } + + this.cleanDOM(); + } + /** * Prepares the DOM for this modal and any additional modal to be mounted. * @@ -103,18 +92,6 @@ class Modal extends Component { document.body.classList.add( this.props.bodyOpenClassName ); } - /** - * Removes the modal's node from the DOM. Also calls closeLastModal when this is - * the last modal to be closed. - */ - closeModal() { - openModalCount--; - - if ( openModalCount === 0 ) { - this.closeLastModal(); - } - } - /** * Cleans up the DOM after the last modal is closed and makes the app available * for screen-readers again. @@ -131,7 +108,6 @@ class Modal extends Component { */ render() { const { - isOpen, overlayClassName, className, onRequestClose, @@ -146,10 +122,6 @@ class Modal extends Component { ...otherProps } = this.props; - if ( ! isOpen ) { - return null; - } - return createPortal(
Date: Tue, 3 Jul 2018 14:28:24 +0200 Subject: [PATCH 51/67] Removed the ability to add inline styles to the modal --- components/modal/README.md | 14 -------------- components/modal/index.js | 8 +------- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/components/modal/README.md b/components/modal/README.md index e36b01b6f7f963..024eeb195d3a5f 100644 --- a/components/modal/README.md +++ b/components/modal/README.md @@ -140,20 +140,6 @@ If this property is added, it will determine whether the modal requests to close - Required: No - Default: true -### style.content - -If this property is added, it will add inline styles to the modal content `div`. - -- Type: `Object` -- Required: No - -### style.overlay - -If this property is added, it will add inline styles to the modal overlay `div`. - -- Type: `Object` -- Required: No - ### className If this property is added, it will an additional class name to the modal content `div`. diff --git a/components/modal/index.js b/components/modal/index.js index 50ba5fec1e36cc..b884ffc45925a5 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -111,10 +111,6 @@ class Modal extends Component { overlayClassName, className, onRequestClose, - style: { - content, - overlay, - }, title, icon, closeButtonLabel, @@ -127,10 +123,8 @@ class Modal extends Component { className={ classnames( 'components-modal__screen-overlay', overlayClassName - ) } - style={ overlay }> + ) }> Date: Tue, 3 Jul 2018 14:38:20 +0200 Subject: [PATCH 52/67] Removed useless css --- components/modal/style.scss | 2 -- 1 file changed, 2 deletions(-) diff --git a/components/modal/style.scss b/components/modal/style.scss index 9a23c2f0c6f684..608e23d1e7572f 100644 --- a/components/modal/style.scss +++ b/components/modal/style.scss @@ -66,8 +66,6 @@ flex-direction: row; align-items: stretch; justify-content: space-between; - left: 0; - right: 0; } .components-modal__header-heading-container { From 9400adc03afebcc4257c4b808d03dc1c838edddf Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 14:52:06 +0200 Subject: [PATCH 53/67] Removed redundant z-index --- components/modal/style.scss | 1 - edit-post/assets/stylesheets/_z-index.scss | 1 - 2 files changed, 2 deletions(-) diff --git a/components/modal/style.scss b/components/modal/style.scss index 608e23d1e7572f..d13ffaa8c61719 100644 --- a/components/modal/style.scss +++ b/components/modal/style.scss @@ -96,5 +96,4 @@ height: calc( 100% - #{ $header-height } - #{ $admin-bar-height } ); overflow: auto; padding: $panel-padding; - z-index: z-index( '.components-modal__content' ); } diff --git a/edit-post/assets/stylesheets/_z-index.scss b/edit-post/assets/stylesheets/_z-index.scss index 7a276027260f21..52f6327660f447 100644 --- a/edit-post/assets/stylesheets/_z-index.scss +++ b/edit-post/assets/stylesheets/_z-index.scss @@ -67,7 +67,6 @@ $z-layers: ( // Show modal under the wp-admin menus and the popover '.components-modal__screen-overlay': 100000, - '.components-modal__content': 100001, // Show popovers above wp-admin menus and submenus and sidebar: // #adminmenuwrap { z-index: 9990 } From a9567325763b5fe3f27857ce18fdd9cd341c559b Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 15:10:59 +0200 Subject: [PATCH 54/67] Add full page overlay --- components/modal/index.js | 44 ++++++++++++++------------ components/modal/style.scss | 11 ++++++- edit-post/components/layout/style.scss | 2 +- 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/components/modal/index.js b/components/modal/index.js index b884ffc45925a5..ef33acd0e066f2 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -119,28 +119,30 @@ class Modal extends Component { } = this.props; return createPortal( -
- +
- -
- { children } -
- + 'components-modal__screen-visible-overlay', + overlayClassName + ) }> + + +
+ { children } +
+ +
, this.node ); diff --git a/components/modal/style.scss b/components/modal/style.scss index d13ffaa8c61719..15945fe414b812 100644 --- a/components/modal/style.scss +++ b/components/modal/style.scss @@ -5,8 +5,17 @@ right: 0; bottom: 0; left: 0; - background-color: rgba( $white, .4 ); z-index: z-index( '.components-modal__screen-overlay' ); +} + +// The scrim behind the modal window. +.components-modal__screen-visible-overlay { + position: fixed; + top: 0; + right: 0; + bottom: 0; + left: 0; + background-color: rgba( $white, .4 ); // On mobile the main content area has to scroll, otherwise you can invoke the overscroll bounce on the non-scrolling container. @include break-small { diff --git a/edit-post/components/layout/style.scss b/edit-post/components/layout/style.scss index e01a96aeccdfe4..9b98d7e2545dd2 100644 --- a/edit-post/components/layout/style.scss +++ b/edit-post/components/layout/style.scss @@ -56,7 +56,7 @@ @include editor-left('.components-notice-list'); @include editor-right('.components-notice-list'); -@include editor-left( '.components-modal__screen-overlay' ); +@include editor-left( '.components-modal__screen-visible-overlay' ); .edit-post-layout__metaboxes:not(:empty) { border-top: 1px solid $light-gray-500; From 0679405f2c5a66fb797f1dcef3fe7d0d5951616a Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 15:11:26 +0200 Subject: [PATCH 55/67] Don't render h1 tag when no title is provided --- components/modal/header.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/components/modal/header.js b/components/modal/header.js index dd37d99c9fb6c5..20c3fc20adca0d 100644 --- a/components/modal/header.js +++ b/components/modal/header.js @@ -22,9 +22,11 @@ const ModalHeader = ( { icon, title, onClose, closeLabel } ) => { { icon } } -

- { title } -

+ { title && +

+ { title } +

+ }
Date: Tue, 3 Jul 2018 16:15:09 +0200 Subject: [PATCH 56/67] Made modal screen-verlay full screen --- components/modal/index.js | 43 ++++++++++++-------------- components/modal/style.scss | 21 +------------ edit-post/components/layout/style.scss | 2 -- 3 files changed, 21 insertions(+), 45 deletions(-) diff --git a/components/modal/index.js b/components/modal/index.js index ef33acd0e066f2..ccaae7efa0c813 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -119,30 +119,27 @@ class Modal extends Component { } = this.props; return createPortal( -
-
+ - - -
- { children } -
- -
+ 'components-modal__frame', + className + ) } + onRequestClose={ onRequestClose } + { ...otherProps } > + +
+ { children } +
+
, this.node ); diff --git a/components/modal/style.scss b/components/modal/style.scss index 15945fe414b812..44f6187192b444 100644 --- a/components/modal/style.scss +++ b/components/modal/style.scss @@ -1,31 +1,12 @@ // The scrim behind the modal window. .components-modal__screen-overlay { - position: fixed; - top: 0; - right: 0; - bottom: 0; - left: 0; - z-index: z-index( '.components-modal__screen-overlay' ); -} - -// The scrim behind the modal window. -.components-modal__screen-visible-overlay { position: fixed; top: 0; right: 0; bottom: 0; left: 0; background-color: rgba( $white, .4 ); - - // On mobile the main content area has to scroll, otherwise you can invoke the overscroll bounce on the non-scrolling container. - @include break-small { - position: fixed; - top: $admin-bar-height-big; - } - - @include break-medium() { - top: $admin-bar-height; - } + z-index: z-index( '.components-modal__screen-overlay' ); } // The modal window element. diff --git a/edit-post/components/layout/style.scss b/edit-post/components/layout/style.scss index 9b98d7e2545dd2..fa6899853fb066 100644 --- a/edit-post/components/layout/style.scss +++ b/edit-post/components/layout/style.scss @@ -56,8 +56,6 @@ @include editor-left('.components-notice-list'); @include editor-right('.components-notice-list'); -@include editor-left( '.components-modal__screen-visible-overlay' ); - .edit-post-layout__metaboxes:not(:empty) { border-top: 1px solid $light-gray-500; margin-top: 10px; From 12dd5fe9d20fdc901fa9464ea903d74642aaf4d9 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 16:40:22 +0200 Subject: [PATCH 57/67] generate unique id for modal labelledby attribute --- components/modal/header.js | 5 +++-- components/modal/index.js | 14 +++++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/components/modal/header.js b/components/modal/header.js index 20c3fc20adca0d..4ce5fec67f7667 100644 --- a/components/modal/header.js +++ b/components/modal/header.js @@ -9,7 +9,7 @@ import { __ } from '@wordpress/i18n'; import IconButton from '../icon-button'; import './style.scss'; -const ModalHeader = ( { icon, title, onClose, closeLabel } ) => { +const ModalHeader = ( { icon, title, onClose, closeLabel, headingId } ) => { const label = closeLabel ? closeLabel : __( 'Close dialog' ); return ( @@ -23,7 +23,8 @@ const ModalHeader = ( { icon, title, onClose, closeLabel } ) => { } { title && -

+

{ title }

} diff --git a/components/modal/index.js b/components/modal/index.js index ccaae7efa0c813..4396b252b50c6a 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -2,7 +2,7 @@ * External dependencies */ import classnames from 'classnames'; -import { noop } from 'lodash'; +import { noop, uniqueId } from 'lodash'; /** * WordPress dependencies @@ -22,10 +22,12 @@ let parentElement, openModalCount = 0; class Modal extends Component { - constructor() { - super( ...arguments ); + constructor( props ) { + super( props ); this.prepareDOM(); + + this.headingId = uniqueId( props.aria.labelledby ); } /** @@ -115,6 +117,7 @@ class Modal extends Component { icon, closeButtonLabel, children, + aria, ...otherProps } = this.props; @@ -129,11 +132,16 @@ class Modal extends Component { className ) } onRequestClose={ onRequestClose } + aria={ { + labelledby: this.headingId, + describedby: aria.describedby, + } } { ...otherProps } >
From f6073303883480657146f0eca4797dc14f0fa1c1 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 21:29:32 +0200 Subject: [PATCH 58/67] Replaced function scoped array liveRegionAriaRoles with file scoped constant LIVE_REGION_ARIA_ROLES --- components/modal/aria-helper.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/components/modal/aria-helper.js b/components/modal/aria-helper.js index 7951cc345d8c92..8b714b147d55e4 100644 --- a/components/modal/aria-helper.js +++ b/components/modal/aria-helper.js @@ -3,6 +3,14 @@ */ import { forEach } from 'lodash'; +const LIVE_REGION_ARIA_ROLES = new Set( [ + 'alert', + 'status', + 'log', + 'marquee', + 'timer', +] ); + let hiddenElements = [], isHidden = false; @@ -45,19 +53,12 @@ export function hideApp( unhiddenElement ) { * @return {boolean} Whether the element should not be hidden from screen-readers. */ export function elementShouldBeHidden( element ) { - const liveRegionAriaRoles = [ - 'alert', - 'status', - 'log', - 'marquee', - 'timer', - ]; const role = element.getAttribute( 'role' ); return ! ( element.tagName === 'SCRIPT' || element.hasAttribute( 'aria-hidden' ) || element.hasAttribute( 'aria-live' ) || - liveRegionAriaRoles.includes( role ) + LIVE_REGION_ARIA_ROLES.has( role ) ); } From 40cc3f97220e3b83da6d1f16ee9644fe59906318 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 21:55:05 +0200 Subject: [PATCH 59/67] Removed check whtehr forwardedRef is defined --- components/higher-order/with-global-events/index.js | 4 +--- components/higher-order/with-global-events/test/index.js | 4 +++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/components/higher-order/with-global-events/index.js b/components/higher-order/with-global-events/index.js index 7527ef8078f8e9..e6859fb7f87cb2 100644 --- a/components/higher-order/with-global-events/index.js +++ b/components/higher-order/with-global-events/index.js @@ -55,9 +55,7 @@ function withGlobalEvents( eventTypesToHandlers ) { handleRef( el ) { this.wrappedRef = el; - if ( this.props.forwardedRef ) { - this.props.forwardedRef( el ); - } + this.props.forwardedRef( el ); } render() { diff --git a/components/higher-order/with-global-events/test/index.js b/components/higher-order/with-global-events/test/index.js index c3ccadc7af5982..31c89bffd77be9 100644 --- a/components/higher-order/with-global-events/test/index.js +++ b/components/higher-order/with-global-events/test/index.js @@ -57,11 +57,13 @@ describe( 'withGlobalEvents', () => { } } ); - function mountEnhancedComponent( props ) { + function mountEnhancedComponent( props = {} ) { const EnhancedComponent = withGlobalEvents( { resize: 'handleResize', } )( OriginalComponent ); + props.ref = () => {}; + wrapper = TestRenderer.create( Hello ); } From 0590e3937ccc16a13665482a4d390d247af23803 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 21:58:10 +0200 Subject: [PATCH 60/67] Minor JSDoc improvement --- components/modal/frame.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/modal/frame.js b/components/modal/frame.js index 153659c8da23bd..be9b44dde7c442 100644 --- a/components/modal/frame.js +++ b/components/modal/frame.js @@ -62,7 +62,7 @@ class ModalFrame extends Component { /** * Callback function called when a key is pressed. * - * @param {Object} event Key down event. + * @param {KeyboardEvent} event Key down event. */ handleKeyDown( event ) { if ( event.keyCode === ESCAPE ) { From 6f57d08b050ab35a0380e85beb76c50251814967 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 22:07:25 +0200 Subject: [PATCH 61/67] Removed styles from defaultProps --- components/modal/index.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/components/modal/index.js b/components/modal/index.js index 4396b252b50c6a..6fac2164a09ac5 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -162,10 +162,6 @@ Modal.defaultProps = { focusOnMount: true, shouldCloseOnEsc: true, shouldCloseOnClickOutside: true, - style: { - content: null, - overlay: null, - }, /* accessibility */ aria: { labelledby: 'modal-heading', From c547404dd09640c4da3f0984ff9ea51e604bcf3c Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 22:07:38 +0200 Subject: [PATCH 62/67] Documentation improvements --- components/modal/README.md | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/components/modal/README.md b/components/modal/README.md index 024eeb195d3a5f..d8cbdd4c3de225 100644 --- a/components/modal/README.md +++ b/components/modal/README.md @@ -12,7 +12,6 @@ Render a screen overlay with a modal on top. - { this.state.isOpen ? - - - - : null } + { this.state.isOpen ? + + + + : null } ); } From 4eba6c71afc5e700630f4b0b5e386804bd5f4622 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 22:19:12 +0200 Subject: [PATCH 63/67] don't add labelledBy attribute to modal frame when no title is present --- components/modal/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/modal/index.js b/components/modal/index.js index 6fac2164a09ac5..bf9cd0dda75901 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -133,7 +133,7 @@ class Modal extends Component { ) } onRequestClose={ onRequestClose } aria={ { - labelledby: this.headingId, + labelledby: title ? this.headingId : null, describedby: aria.describedby, } } { ...otherProps } > From 9ee5b83912631c5110dcd1916465c613b59083e8 Mon Sep 17 00:00:00 2001 From: Alexander Botteram Date: Tue, 3 Jul 2018 22:31:00 +0200 Subject: [PATCH 64/67] Don't add unique id to headingId when aria.labelledby prop is provided, also added aria.labelledby change logic --- components/modal/index.js | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/components/modal/index.js b/components/modal/index.js index bf9cd0dda75901..fec7d638acc2ed 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -27,7 +27,7 @@ class Modal extends Component { this.prepareDOM(); - this.headingId = uniqueId( props.aria.labelledby ); + this.setHeadingId( props ); } /** @@ -48,6 +48,16 @@ class Modal extends Component { parentElement.appendChild( this.node ); } + /** + * Sets the heading id to the aria.labelledby prop or a unique id when + * the prop is unavailable. + * + * @param {Object} props The component's props. + */ + setHeadingId( props ) { + this.headingId = props.aria.labelledby || uniqueId( 'modal-heading-' ); + } + /** * Removes the specific mounting point for this modal from the DOM. */ @@ -68,6 +78,17 @@ class Modal extends Component { } } + /** + * Updates headingId when the aria.labelledby prop has changed. + * + * @param {Object} nextProps The component's next props. + */ + componentWillReceiveProps( nextProps ) { + if ( this.props.aria.labelledby !== nextProps.aria.labelledby ) { + this.setHeadingId( nextProps ); + } + } + /** * Removes the modal's node from the DOM. Also calls closeLastModal when this is * the last modal to be closed. @@ -164,7 +185,7 @@ Modal.defaultProps = { shouldCloseOnClickOutside: true, /* accessibility */ aria: { - labelledby: 'modal-heading', + labelledby: null, describedby: null, }, }; From 1a0bf757a943528c94cdac51911dd71cd139034d Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 3 Jul 2018 20:04:19 -0400 Subject: [PATCH 65/67] Components: Reorder component lifecycle as first class members --- components/modal/index.js | 70 +++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/components/modal/index.js b/components/modal/index.js index fec7d638acc2ed..10f7b2e561995a 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -30,41 +30,6 @@ class Modal extends Component { this.setHeadingId( props ); } - /** - * Prepares the DOM for the modals to be rendered. - * - * Every modal is mounted in a separate div appended to a parent div - * that is appended to the document body. - * - * The parent div will be created if it does not yet exist, and the - * separate div for this specific modal will be appended to that. - */ - prepareDOM() { - if ( ! parentElement ) { - parentElement = document.createElement( 'div' ); - document.body.appendChild( parentElement ); - } - this.node = document.createElement( 'div' ); - parentElement.appendChild( this.node ); - } - - /** - * Sets the heading id to the aria.labelledby prop or a unique id when - * the prop is unavailable. - * - * @param {Object} props The component's props. - */ - setHeadingId( props ) { - this.headingId = props.aria.labelledby || uniqueId( 'modal-heading-' ); - } - - /** - * Removes the specific mounting point for this modal from the DOM. - */ - cleanDOM() { - parentElement.removeChild( this.node ); - } - /** * Appends the modal's node to the DOM, so the portal can render the * modal in it. Also calls the openFirstModal when this is the first modal to be @@ -103,6 +68,41 @@ class Modal extends Component { this.cleanDOM(); } + /** + * Prepares the DOM for the modals to be rendered. + * + * Every modal is mounted in a separate div appended to a parent div + * that is appended to the document body. + * + * The parent div will be created if it does not yet exist, and the + * separate div for this specific modal will be appended to that. + */ + prepareDOM() { + if ( ! parentElement ) { + parentElement = document.createElement( 'div' ); + document.body.appendChild( parentElement ); + } + this.node = document.createElement( 'div' ); + parentElement.appendChild( this.node ); + } + + /** + * Sets the heading id to the aria.labelledby prop or a unique id when + * the prop is unavailable. + * + * @param {Object} props The component's props. + */ + setHeadingId( props ) { + this.headingId = props.aria.labelledby || uniqueId( 'modal-heading-' ); + } + + /** + * Removes the specific mounting point for this modal from the DOM. + */ + cleanDOM() { + parentElement.removeChild( this.node ); + } + /** * Prepares the DOM for this modal and any additional modal to be mounted. * From 87787069ee3cd908a30a9e3ca94016a0c0bfc211 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 3 Jul 2018 20:35:11 -0400 Subject: [PATCH 66/67] Components: Use withInstanceId to generate modal heading id --- components/modal/index.js | 38 +++++++++++--------------------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/components/modal/index.js b/components/modal/index.js index 10f7b2e561995a..7eb62fb91a92f2 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -12,10 +12,11 @@ import { Component, createPortal } from '@wordpress/element'; /** * Internal dependencies */ +import './style.scss'; import ModalFrame from './frame'; import ModalHeader from './header'; import * as ariaHelper from './aria-helper'; -import './style.scss'; +import withInstanceId from '../with-instance-id'; // Used to count the number of open modals. let parentElement, @@ -26,8 +27,6 @@ class Modal extends Component { super( props ); this.prepareDOM(); - - this.setHeadingId( props ); } /** @@ -43,17 +42,6 @@ class Modal extends Component { } } - /** - * Updates headingId when the aria.labelledby prop has changed. - * - * @param {Object} nextProps The component's next props. - */ - componentWillReceiveProps( nextProps ) { - if ( this.props.aria.labelledby !== nextProps.aria.labelledby ) { - this.setHeadingId( nextProps ); - } - } - /** * Removes the modal's node from the DOM. Also calls closeLastModal when this is * the last modal to be closed. @@ -86,16 +74,6 @@ class Modal extends Component { parentElement.appendChild( this.node ); } - /** - * Sets the heading id to the aria.labelledby prop or a unique id when - * the prop is unavailable. - * - * @param {Object} props The component's props. - */ - setHeadingId( props ) { - this.headingId = props.aria.labelledby || uniqueId( 'modal-heading-' ); - } - /** * Removes the specific mounting point for this modal from the DOM. */ @@ -139,9 +117,15 @@ class Modal extends Component { closeButtonLabel, children, aria, + instanceId, ...otherProps } = this.props; + const headingId = ( + aria.labelledby || + 'components-modal-header-' + instanceId + ); + return createPortal(
@@ -162,7 +146,7 @@ class Modal extends Component { closeLabel={ closeButtonLabel } onClose={ onRequestClose } title={ title } - headingId={ this.headingId } + headingId={ headingId } icon={ icon } />
@@ -190,4 +174,4 @@ Modal.defaultProps = { }, }; -export default Modal; +export default withInstanceId( Modal ); From 6a43d9fdc6b0f3d7746b2f3d0efa0e56784b3a01 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 3 Jul 2018 20:53:04 -0400 Subject: [PATCH 67/67] Components: Fix modal withInstanceId import reference --- components/modal/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/modal/index.js b/components/modal/index.js index 7eb62fb91a92f2..63c719431efe77 100644 --- a/components/modal/index.js +++ b/components/modal/index.js @@ -2,7 +2,7 @@ * External dependencies */ import classnames from 'classnames'; -import { noop, uniqueId } from 'lodash'; +import { noop } from 'lodash'; /** * WordPress dependencies @@ -16,7 +16,7 @@ import './style.scss'; import ModalFrame from './frame'; import ModalHeader from './header'; import * as ariaHelper from './aria-helper'; -import withInstanceId from '../with-instance-id'; +import withInstanceId from '../higher-order/with-instance-id'; // Used to count the number of open modals. let parentElement,