Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autocomplete: Assign ARIA attributes as combobox, with TinyMCE node mirroring #2789

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 56 additions & 13 deletions blocks/editable/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
identity,
find,
defer,
pickBy,
noop,
} from 'lodash';
import { nodeListToReact } from 'dom-react';
Expand Down Expand Up @@ -74,6 +75,8 @@ export default class Editable extends Component {
);
}

this.bindMirrorRef = this.bindNodeRef.bind( this, 'mirror' );
this.bindEditorRef = this.bindNodeRef.bind( this, 'editor' );
this.onInit = this.onInit.bind( this );
this.getSettings = this.getSettings.bind( this );
this.onSetup = this.onSetup.bind( this );
Expand All @@ -89,13 +92,19 @@ export default class Editable extends Component {
this.onBeforePastePreProcess = this.onBeforePastePreProcess.bind( this );
this.onPaste = this.onPaste.bind( this );

this.nodeRefs = {};

this.state = {
formats: {},
empty: ! value || ! value.length,
selectedNodeId: 0,
};
}

bindNodeRef( name, node ) {
this.nodeRefs[ name ] = node;
}

getSettings( settings ) {
return ( this.props.getSettings || identity )( {
...settings,
Expand Down Expand Up @@ -143,6 +152,9 @@ export default class Editable extends Component {

onInit() {
this.updateFocus();

// Start mirroring attributes after editor initializes
this.nodeRefs.editor.mirrorNode( this.nodeRefs.mirror );
}

onFocus() {
Expand Down Expand Up @@ -594,7 +606,6 @@ export default class Editable extends Component {
render() {
const {
tagName: Tagname = 'div',
style,
value,
focus,
wrapperClassname,
Expand All @@ -606,6 +617,19 @@ export default class Editable extends Component {
keepPlaceholderOnFocus = false,
} = this.props;

// Assign as props to editor node those not explicitly handled by
// Editable. Since event handlers are proxied by TinyMCE, ignore any
// props prefixed by `on` (see: `proxyPropHandler`).
const editorProps = pickBy( this.props, ( propValue, propKey ) => (
! this.constructor.propTypes.hasOwnProperty( propKey ) &&
! /^on[A-Z]/.test( propKey )
) );

editorProps.className = classnames( 'blocks-editable__tinymce', className );
if ( ! editorProps[ 'aria-label' ] ) {
editorProps[ 'aria-label' ] = placeholder;
}

// Generating a key that includes `tagName` ensures that if the tag
// changes, we unmount and destroy the previous TinyMCE element, then
// mount and initialize a new child element in its place.
Expand Down Expand Up @@ -635,30 +659,49 @@ export default class Editable extends Component {
{ formatToolbar }
</div>
}
<div
data-mirror
aria-hidden
ref={ this.bindMirrorRef }
{ ...editorProps } />
{ isPlaceholderVisible &&
<Tagname data-placeholder aria-hidden { ...editorProps }>
{ MultilineTag ? <MultilineTag>{ placeholder }</MultilineTag> : placeholder }
</Tagname>
}
<TinyMCE
ref={ this.bindEditorRef }
tagName={ Tagname }
getSettings={ this.getSettings }
onSetup={ this.onSetup }
style={ style }
defaultValue={ value }
isPlaceholderVisible={ isPlaceholderVisible }
label={ placeholder }
className={ className }
key={ key }
additionalProps={ editorProps }
/>
{ isPlaceholderVisible &&
<Tagname
className={ classnames( 'blocks-editable__tinymce', className ) }
style={ style }
>
{ MultilineTag ? <MultilineTag>{ placeholder }</MultilineTag> : placeholder }
</Tagname>
}
</div>
);
}
}

Editable.propTypes = {
className: noop,
focus: noop,
formattingControls: noop,
getSettings: noop,
inlineToolbar: noop,
keepPlaceholderOnFocus: noop,
multiline: noop,
onChange: noop,
onFocus: noop,
onMerge: noop,
onReplace: noop,
onSetup: noop,
placeholder: noop,
tagName: noop,
value: noop,
wrapperClassname: noop,
};

Editable.contextTypes = {
onUndo: noop,
};
36 changes: 20 additions & 16 deletions blocks/editable/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,26 @@
color: $blue-medium-500;
}

&[data-mirror] {
display: none;
}

&[data-placeholder] {
opacity: 0.5;
pointer-events: none;

& + .blocks-editable__tinymce {
position: absolute;
top: 0;
width: 100%;
margin-top: 0;

& > p {
margin-top: 0;
}
}
}

&:focus a[data-mce-selected] {
padding: 0 2px;
margin: 0 -2px;
Expand All @@ -46,22 +66,6 @@
&:focus code[data-mce-selected] {
background: $light-gray-400;
}

&[data-is-placeholder-visible="true"] {
position: absolute;
top: 0;
width: 100%;
margin-top: 0;

& > p {
margin-top: 0;
}
}

& + .blocks-editable__tinymce {
opacity: 0.5;
pointer-events: none;
}
}

.has-drop-cap .blocks-editable__tinymce:not( :focus ) {
Expand Down
62 changes: 32 additions & 30 deletions blocks/editable/tinymce.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
* External dependencies
*/
import tinymce from 'tinymce';
import { isEqual } from 'lodash';
import classnames from 'classnames';

/**
* WordPress dependencies
Expand All @@ -15,6 +13,17 @@ export default class TinyMCE extends Component {
this.initialize();
}

componentWillUnmount() {
if ( this.observer ) {
this.observer.disconnect();
}

if ( this.editor ) {
this.editor.destroy();
delete this.editor;
}
}

shouldComponentUpdate() {
// We must prevent rerenders because TinyMCE will modify the DOM, thus
// breaking React's ability to reconcile changes.
Expand All @@ -23,31 +32,26 @@ export default class TinyMCE extends Component {
return false;
}

componentWillReceiveProps( nextProps ) {
const name = 'data-is-placeholder-visible';
const isPlaceholderVisible = String( !! nextProps.isPlaceholderVisible );

if ( this.editorNode.getAttribute( name ) !== isPlaceholderVisible ) {
this.editorNode.setAttribute( name, isPlaceholderVisible );
}

if ( ! isEqual( this.props.style, nextProps.style ) ) {
this.editorNode.setAttribute( 'style', '' );
Object.assign( this.editorNode.style, nextProps.style );
}

if ( ! isEqual( this.props.className, nextProps.className ) ) {
this.editorNode.className = classnames( nextProps.className, 'blocks-editable__tinymce' );
}
}

componentWillUnmount() {
if ( ! this.editor ) {
return;
}
mirrorNode( node ) {
// Since React reconciliation is disabled for the TinyMCE node, we sync
// attribute changes using a mutation observer on a node within the
// parent Editable which receives reconciliation from Editable props.
this.observer = new window.MutationObserver( ( mutations ) => {
mutations.forEach( ( mutation ) => {
const { attributeName } = mutation;
const nextValue = node.getAttribute( attributeName );

if ( null === nextValue ) {
this.editorNode.removeAttribute( attributeName );
} else {
this.editorNode.setAttribute( attributeName, nextValue );
}
} );
} );

this.editor.destroy();
delete this.editor;
this.observer.observe( node, {
attributes: true,
} );
}

initialize() {
Expand Down Expand Up @@ -83,7 +87,7 @@ export default class TinyMCE extends Component {
}

render() {
const { tagName = 'div', style, defaultValue, label, className } = this.props;
const { tagName = 'div', defaultValue, additionalProps } = this.props;

// If a default value is provided, render it into the DOM even before
// TinyMCE finishes initializing. This avoids a short delay by allowing
Expand All @@ -97,9 +101,7 @@ export default class TinyMCE extends Component {
ref: ( node ) => this.editorNode = node,
contentEditable: true,
suppressContentEditableWarning: true,
className: classnames( className, 'blocks-editable__tinymce' ),
style,
'aria-label': label,
...additionalProps,
}, children );
}
}
19 changes: 14 additions & 5 deletions components/autocomplete/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ import { keycodes } from '@wordpress/utils';
import './style.scss';
import Button from '../button';
import Popover from '../popover';
import withInstanceId from '../higher-order/with-instance-id';

const { ENTER, ESCAPE, UP, DOWN } = keycodes;

class Autocomplete extends Component {
export class Autocomplete extends Component {
static getInitialState() {
return {
isOpen: false,
Expand Down Expand Up @@ -180,10 +181,12 @@ class Autocomplete extends Component {
}

render() {
const { children, className } = this.props;
const { children, className, instanceId } = this.props;
const { isOpen, selectedIndex } = this.state;
const classes = classnames( 'components-autocomplete__popover', className );
const filteredOptions = this.getFilteredOptions();
const listBoxId = `components-autocomplete-listbox-${ instanceId }`;
const activeId = `components-autocomplete-item-${ instanceId }-${ selectedIndex }`;

// Blur is applied to the wrapper node, since if the child is Editable,
// the event will not have `relatedTarget` assigned.
Expand All @@ -196,6 +199,10 @@ class Autocomplete extends Component {
{ cloneElement( Children.only( children ), {
onInput: this.search,
onKeyDown: this.setSelectedIndex,
role: 'combobox',
'aria-expanded': isOpen,
'aria-activedescendant': isOpen ? activeId : null,
'aria-owns': isOpen ? listBoxId : null,
} ) }
<Popover
isOpen={ isOpen && filteredOptions.length > 0 }
Expand All @@ -204,13 +211,15 @@ class Autocomplete extends Component {
className={ classes }
>
<ul
role="menu"
id={ listBoxId }
role="listbox"
className="components-autocomplete__results"
>
{ filteredOptions.map( ( option, index ) => (
<li
key={ option.value }
role="menuitem"
id={ `components-autocomplete-item-${ instanceId }-${ index }` }
role="option"
className={ classnames( 'components-autocomplete__result', {
'is-selected': index === selectedIndex,
} ) }
Expand All @@ -227,4 +236,4 @@ class Autocomplete extends Component {
}
}

export default Autocomplete;
export default withInstanceId( Autocomplete );
14 changes: 11 additions & 3 deletions components/autocomplete/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { keycodes } from '@wordpress/utils';
/**
* Internal dependencies
*/
import Autocomplete from '../';
import { Autocomplete } from '../';

const { ENTER, ESCAPE, UP, DOWN, SPACE } = keycodes;

Expand Down Expand Up @@ -53,13 +53,17 @@ describe( 'Autocomplete', () => {
</Autocomplete>
);
const popover = wrapper.find( 'Popover' );
const clone = wrapper.find( '[data-ok]' );

expect( wrapper.state( 'isOpen' ) ).toBe( false );
expect( popover.prop( 'focusOnOpen' ) ).toBe( false );
expect( popover.hasClass( 'my-autocomplete' ) ).toBe( true );
expect( popover.hasClass( 'components-autocomplete__popover' ) ).toBe( true );
expect( wrapper.hasClass( 'components-autocomplete' ) ).toBe( true );
expect( wrapper.find( '[data-ok]' ) ).toHaveLength( 1 );
expect( clone ).toHaveLength( 1 );
expect( clone.prop( 'aria-expanded' ) ).toBe( false );
expect( clone.prop( 'aria-activedescendant' ) ).toBe( null );
expect( clone.prop( 'aria-owns' ) ).toBe( null );
} );

it( 'opens on absent trigger prefix search', () => {
Expand All @@ -68,8 +72,9 @@ describe( 'Autocomplete', () => {
<div contentEditable />
</Autocomplete>
);
const clone = wrapper.find( '[contentEditable]' );

wrapper.find( '[contentEditable]' ).simulate( 'input', {
clone.simulate( 'input', {
target: {
textContent: 'b',
},
Expand All @@ -80,6 +85,9 @@ describe( 'Autocomplete', () => {
expect( wrapper.state( 'search' ) ).toEqual( /b/i );
expect( wrapper.find( 'Popover' ).prop( 'isOpen' ) ).toBe( true );
expect( wrapper.find( '.components-autocomplete__result' ) ).toHaveLength( 1 );
expect( clone.prop( 'aria-expanded' ) ).toBe( false );
expect( clone.prop( 'aria-activedescendant' ) ).toBe( null );
expect( clone.prop( 'aria-owns' ) ).toBe( null );
} );

it( 'does not render popover as open if no results', () => {
Expand Down