Components: Improve Disabled component (disabled attribute applicable…

…, tabindex removal, pointer-events) (#5748) * Components: Only apply disabled attribute to eligible nodes * Components: Remove tabindex attribute from disabled nodes * Components: Disable pointer events for disabled content
WordPress · Apr 11, 2018 · 004cb8b · 004cb8b
1 parent c17b668
commit 004cb8b
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 7 deletions.
diff --git a/components/disabled/index.js b/components/disabled/index.js
@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import { debounce } from 'lodash';
+import { includes, debounce } from 'lodash';
 
 /**
  * WordPress dependencies
@@ -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 );
@@ -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' );
 			}

diff --git a/components/disabled/style.scss b/components/disabled/style.scss
@@ -1,5 +1,6 @@
 .components-disabled {
 	position: relative;
+	pointer-events: none;
 
 	&:after {
 		content: '';

diff --git a/components/disabled/test/index.js b/components/disabled/test/index.js
@@ -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', () => {
@@ -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: