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.

Sign up for GitHub

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

Jump to bottom

Components: Improve Disabled component (disabled attribute applicable, tabindex removal, pointer-events) #5748

Merged
merged 3 commits into from
Apr 11, 2018
Merged

Components: Improve Disabled component (disabled attribute applicable, tabindex removal, pointer-events) #5748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
87fcf3d
Components: Only apply disabled attribute to eligible nodes
aduth Mar 22, 2018
8e45803
Components: Remove tabindex attribute from disabled nodes
aduth Mar 22, 2018
0ee5358
Components: Disable pointer events for disabled content
aduth Mar 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 25 additions & 2 deletions components/disabled/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { debounce } from 'lodash';
import { includes, debounce } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -14,6 +14,25 @@ import { focus } from '@wordpress/utils';
*/
import './style.scss';

/**
* Names of control nodes which qualify for disabled behavior.
*
* See WHATWG HTML Standard: 4.10.18.5: "Enabling and disabling form controls: the disabled attribute".
*
* @link https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#enabling-and-disabling-form-controls:-the-disabled-attribute
*
* @type {string[]}
*/
const DISABLED_ELIGIBLE_NODE_NAMES = [
'BUTTON',
'FIELDSET',
'INPUT',
'OPTGROUP',
'OPTION',
'SELECT',
'TEXTAREA',
];

class Disabled extends Component {
constructor() {
super( ...arguments );
Expand Down Expand Up @@ -48,10 +67,14 @@ class Disabled extends Component {

disable() {
focus.focusable.find( this.node ).forEach( ( focusable ) => {
if ( ! focusable.hasAttribute( 'disabled' ) ) {
if ( includes( DISABLED_ELIGIBLE_NODE_NAMES, focusable.nodeName ) ) {
focusable.setAttribute( 'disabled', '' );
}

if ( focusable.hasAttribute( 'tabindex' ) ) {
focusable.removeAttribute( 'tabindex' );
}

if ( focusable.hasAttribute( 'contenteditable' ) ) {
focusable.setAttribute( 'contenteditable', 'false' );
}
Expand Down
1 change: 1 addition & 0 deletions components/disabled/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
.components-disabled {
position: relative;
pointer-events: none;

&:after {
content: '';
Expand Down
19 changes: 14 additions & 5 deletions components/disabled/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,18 @@ describe( 'Disabled', () => {
window.MutationObserver = MutationObserver;
} );

const Form = () => <form><input /><div contentEditable /></form>;
const Form = () => <form><input /><div contentEditable tabIndex="0" /></form>;

it( 'will disable all fields', () => {
const wrapper = mount( <Disabled><Form /></Disabled> );

expect( wrapper.find( 'input' ).getDOMNode().hasAttribute( 'disabled' ) ).toBe( true );
expect( wrapper.find( '[contentEditable]' ).getDOMNode().getAttribute( 'contenteditable' ) ).toBe( 'false' );
const input = wrapper.find( 'input' ).getDOMNode();
const div = wrapper.find( '[contentEditable]' ).getDOMNode();

expect( input.hasAttribute( 'disabled' ) ).toBe( true );
expect( div.getAttribute( 'contenteditable' ) ).toBe( 'false' );
expect( div.hasAttribute( 'tabindex' ) ).toBe( false );
expect( div.hasAttribute( 'disabled' ) ).toBe( false );
} );

it( 'should cleanly un-disable via reconciliation', () => {
Expand All @@ -73,8 +78,12 @@ describe( 'Disabled', () => {
const wrapper = mount( <MaybeDisable /> );
wrapper.setProps( { isDisabled: false } );

expect( wrapper.find( 'input' ).getDOMNode().hasAttribute( 'disabled' ) ).toBe( false );
expect( wrapper.find( '[contentEditable]' ).getDOMNode().getAttribute( 'contenteditable' ) ).toBe( 'true' );
const input = wrapper.find( 'input' ).getDOMNode();
const div = wrapper.find( '[contentEditable]' ).getDOMNode();

expect( input.hasAttribute( 'disabled' ) ).toBe( false );
expect( div.getAttribute( 'contenteditable' ) ).toBe( 'true' );
expect( div.hasAttribute( 'tabindex' ) ).toBe( true );
} );

// Ideally, we'd have two more test cases here:
Expand Down