Merge pull request #2680 from WordPress/update/color-picker-a11y-vali…

…d-code Color picker improvements, first round
WordPress · Sep 29, 2017 · a596ceb · a596ceb
2 parents e7f1223 + a4d10f7
commit a596ceb
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 20 deletions.
diff --git a/blocks/color-palette/index.js b/blocks/color-palette/index.js
@@ -23,16 +23,24 @@ class ColorPalette extends Component {
 		this.state = {
 			opened: false,
 		};
-		this.openPicker = this.openPicker.bind( this );
-		this.closePicker = this.closePicker.bind( this );
+		this.togglePicker = this.togglePicker.bind( this );
+		this.closeOnClickOutside = this.closeOnClickOutside.bind( this );
+		this.bindToggleNode = this.bindToggleNode.bind( this );
 	}
 
-	openPicker() {
-		this.setState( { opened: true } );
+	togglePicker() {
+		this.setState( ( state ) => ( { opened: ! state.opened } ) );
 	}
 
-	closePicker() {
-		this.setState( { opened: false } );
+	closeOnClickOutside( event ) {
+		const { opened } = this.state;
+		if ( opened && ! this.toggleNode.contains( event.target ) ) {
+			this.togglePicker();
+		}
+	}
+
+	bindToggleNode( node ) {
+		this.toggleNode = node;
 	}
 
 	render() {
@@ -42,38 +50,42 @@ class ColorPalette extends Component {
 				{ colors.map( ( color ) => {
 					const style = { color: color };
 					const className = classnames( 'blocks-color-palette__item', { 'is-active': value === color } );
-					/* Disable reason: aria-current seems like a valid aria-prop and well-suited for this */
-					/* eslint-disable jsx-a11y/aria-props */
+
 					return (
 						<div key={ color } className="blocks-color-palette__item-wrapper">
 							<button
+								type="button"
 								className={ className }
 								style={ style }
 								onClick={ () => onChange( value === color ? undefined : color ) }
 								aria-label={ sprintf( __( 'Color: %s' ), color ) }
-								aria-current={ value === color && __( 'Selected color' ) }
+								aria-pressed={ value === color }
 							/>
 						</div>
 					);
-					/* eslint-enable jsx-a11y/aria-props */
 				} ) }
 
 				<div className="blocks-color-palette__item-wrapper blocks-color-palette__custom-color">
 					<button
+						type="button"
+						aria-expanded={ this.state.opened }
 						className="blocks-color-palette__item"
-						onClick={ this.openPicker }
-						aria-label={ __( 'Open custom color picker' ) }
-					/>
+						onClick={ this.togglePicker }
+						ref={ this.bindToggleNode }
+						aria-label={ __( 'Custom color picker' ) }
+					>
+						<span className="blocks-color-palette__custom-color-gradient" />
+					</button>
 					<Popover
 						isOpen={ this.state.opened }
-						onClose={ this.closePicker }
+						onClickOutside={ this.closeOnClickOutside }
 						className="blocks-color-palette__picker"
 					>
 						<ChromePicker
 							color={ value }
 							onChangeComplete={ ( color ) => {
 								onChange( color.hex );
-								this.closePicker();
+								this.togglePicker();
 							} }
 							style={ { width: '100%' } }
 							disableAlpha
@@ -82,8 +94,12 @@ class ColorPalette extends Component {
 				</div>
 
 				<div className="blocks-color-palette__item-wrapper blocks-color-palette__clear-color">
-					<button className="blocks-color-palette__item" onClick={ () => onChange( undefined ) }>
-						<div className="blocks-color-palette__clear-color-line" />
+					<button
+						className="blocks-color-palette__item"
+						onClick={ () => onChange( undefined ) }
+						aria-label={ __( 'Remove color' ) }
+					>
+						<span className="blocks-color-palette__clear-color-line" />
 					</button>
 				</div>
 			</div>

diff --git a/blocks/color-palette/style.scss b/blocks/color-palette/style.scss
@@ -11,6 +11,7 @@ $color-palette-circle-spacing: 14px;
 	width: $color-palette-circle-size;
 	margin-right: $color-palette-circle-spacing;
 	margin-bottom: $color-palette-circle-spacing;
+	vertical-align: top;
 	transform: scale( 1 );
 	transition: 100ms transform ease;
 	&:hover {
@@ -20,6 +21,7 @@ $color-palette-circle-spacing: 14px;
 
 .blocks-color-palette__item {
 	display: inline-block;
+	vertical-align: top;
 	height: 100%;
 	width: 100%;
 	border: none;
@@ -51,11 +53,10 @@ $color-palette-circle-spacing: 14px;
 .blocks-color-palette__clear-color .blocks-color-palette__item {
 	color: $white;
 	background: $white;
-	overflow: hidden;
 }
 
 .blocks-color-palette__clear-color-line {
-	cursor: pointer;
+	display: block;
 	position: absolute;
 	border: 2px solid $alert-red;
 	border-radius: 50%;
@@ -81,10 +82,20 @@ $color-palette-circle-spacing: 14px;
 .blocks-color-palette__custom-color .blocks-color-palette__item {
 	position: relative;
 	box-shadow: none;
+}
+
+.blocks-color-palette__custom-color .blocks-color-palette__custom-color-gradient {
+	display: block;
+	width: 100%;
+	height: 100%;
+	position: absolute;
+	top: 0;
+	left: 0;
+	border-radius: 50%;
 	overflow: hidden;
 }
 
-.blocks-color-palette__custom-color .blocks-color-palette__item:before {
+.blocks-color-palette__custom-color .blocks-color-palette__custom-color-gradient:before {
 	box-sizing: border-box;
 	content: '';
 	filter: blur( 6px ) saturate( 0.7 ) brightness( 1.1 );