From e5d0c439ac4630ce7ff6bc04bde4f996431daef2 Mon Sep 17 00:00:00 2001 From: Jon Q Date: Fri, 27 Mar 2020 11:52:17 -0400 Subject: [PATCH] Adding a11y and RTL support to AlignmentMatrixControl Also refactors isRTL / useRTL from the rtl utils. --- .../src/alignment-matrix-control/cell.js | 21 +++ .../src/alignment-matrix-control/icon.js | 21 ++- .../src/alignment-matrix-control/index.js | 70 +++++--- .../styles/alignment-matrix-control-styles.js | 10 +- .../alignment-matrix-control/test/index.js | 40 +++++ .../alignment-matrix-control/test/utils.js | 160 ++++++++++++++++++ .../test/utils.test.js | 55 ------ .../src/alignment-matrix-control/utils.js | 31 +++- .../components/src/range-control/index.js | 4 +- packages/components/src/utils/rtl.js | 12 +- 10 files changed, 331 insertions(+), 93 deletions(-) create mode 100644 packages/components/src/alignment-matrix-control/cell.js create mode 100644 packages/components/src/alignment-matrix-control/test/index.js create mode 100644 packages/components/src/alignment-matrix-control/test/utils.js delete mode 100644 packages/components/src/alignment-matrix-control/test/utils.test.js diff --git a/packages/components/src/alignment-matrix-control/cell.js b/packages/components/src/alignment-matrix-control/cell.js new file mode 100644 index 00000000000000..091b1deede7193 --- /dev/null +++ b/packages/components/src/alignment-matrix-control/cell.js @@ -0,0 +1,21 @@ +/** + * Internal dependencies + */ +import { + Cell as CellView, + Point, +} from './styles/alignment-matrix-control-styles'; + +export default function Cell( { isActive = false, value, ...props } ) { + return ( + + + + ); +} diff --git a/packages/components/src/alignment-matrix-control/icon.js b/packages/components/src/alignment-matrix-control/icon.js index c747394815a930..c6fd7d66434ab8 100644 --- a/packages/components/src/alignment-matrix-control/icon.js +++ b/packages/components/src/alignment-matrix-control/icon.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import classnames from 'classnames'; + /** * Internal dependencies */ @@ -9,6 +14,7 @@ import { } from './styles/alignment-matrix-control-icon-styles'; export default function AlignmentMatrixControlIcon( { + className, height, size: sizeProp = 24, value = 'center', @@ -18,12 +24,23 @@ export default function AlignmentMatrixControlIcon( { const alignIndex = getAlignmentIndex( value ); const size = sizeProp || width || height; + const classes = classnames( + 'component-alignment-matrix-control-icon', + className + ); + return ( - + { ALIGNMENTS.map( ( align, index ) => { const isActive = alignIndex === index; + return ( - + ); diff --git a/packages/components/src/alignment-matrix-control/index.js b/packages/components/src/alignment-matrix-control/index.js index 2b5e00bbab9704..12c923a6b91150 100644 --- a/packages/components/src/alignment-matrix-control/index.js +++ b/packages/components/src/alignment-matrix-control/index.js @@ -2,10 +2,13 @@ * External dependencies */ import { noop } from 'lodash'; +import classnames from 'classnames'; /** * WordPress dependencies */ +import { __ } from '@wordpress/i18n'; +import { useInstanceId } from '@wordpress/compose'; import { useEffect, useRef } from '@wordpress/element'; import { UP, DOWN, LEFT, RIGHT } from '@wordpress/keycodes'; @@ -19,22 +22,27 @@ import { getAlignmentValueFromIndex, getNextIndexFromDirection, } from './utils'; -import { Root, Cell, Point } from './styles/alignment-matrix-control-styles'; +import Cell from './cell'; +import { Root } from './styles/alignment-matrix-control-styles'; import { useControlledState } from '../utils/hooks'; +import { useRTL } from '../utils/rtl'; import AlignmentMatrixControlIcon from './icon'; -// TODO: Account for RTL alignments export default function AlignmentMatrixControl( { + className, + label = __( 'Alignment Matrix Control' ), hasFocusBorder = true, onChange = noop, onKeyDown = noop, value = 'center', ...props } ) { + const isRTL = useRTL(); const [ alignIndex, setAlignIndex ] = useControlledState( getAlignmentIndex( value ) ); const nodeRef = useRef(); + const instanceId = useInstanceId( AlignmentMatrixControl ); const handleOnChange = ( nextIndex, changeProps ) => { const alignName = getAlignmentValueFromIndex( nextIndex ); @@ -46,41 +54,42 @@ export default function AlignmentMatrixControl( { const handleOnKeyDown = ( event ) => { const { keyCode } = event; let nextIndex; + let direction; onKeyDown( event ); switch ( keyCode ) { case UP: event.preventDefault(); - nextIndex = getNextIndexFromDirection( - alignIndex, - DIRECTION.UP - ); + direction = DIRECTION.UP; + + nextIndex = getNextIndexFromDirection( alignIndex, direction ); handleOnChange( nextIndex, { event } ); + break; case DOWN: event.preventDefault(); - nextIndex = getNextIndexFromDirection( - alignIndex, - DIRECTION.DOWN - ); + direction = DIRECTION.DOWN; + + nextIndex = getNextIndexFromDirection( alignIndex, direction ); handleOnChange( nextIndex, { event } ); + break; case LEFT: event.preventDefault(); - nextIndex = getNextIndexFromDirection( - alignIndex, - DIRECTION.LEFT - ); + direction = isRTL ? DIRECTION.RIGHT : DIRECTION.LEFT; + + nextIndex = getNextIndexFromDirection( alignIndex, direction ); handleOnChange( nextIndex, { event } ); + break; case RIGHT: event.preventDefault(); - nextIndex = getNextIndexFromDirection( - alignIndex, - DIRECTION.RIGHT - ); + direction = isRTL ? DIRECTION.LEFT : DIRECTION.RIGHT; + + nextIndex = getNextIndexFromDirection( alignIndex, direction ); handleOnChange( nextIndex, { event } ); + break; default: break; @@ -111,23 +120,40 @@ export default function AlignmentMatrixControl( { }; }, [ handleOnKeyDown ] ); + const id = `alignment-matrix-control-${ instanceId }`; + const activeCellId = `${ id }-${ alignIndex }`; + + const classes = classnames( + 'component-alignment-matrix-control', + className + ); + return ( { ALIGNMENTS.map( ( align, index ) => { const isActive = alignIndex === index; + const cellId = `${ id }-${ index }`; + const cellValue = getAlignmentValueFromIndex( index ); + return ( - - + value={ cellValue } + /> ); } ) } diff --git a/packages/components/src/alignment-matrix-control/styles/alignment-matrix-control-styles.js b/packages/components/src/alignment-matrix-control/styles/alignment-matrix-control-styles.js index 8c17e1673db443..faf82d20ccfa01 100644 --- a/packages/components/src/alignment-matrix-control/styles/alignment-matrix-control-styles.js +++ b/packages/components/src/alignment-matrix-control/styles/alignment-matrix-control-styles.js @@ -37,8 +37,8 @@ export const Root = styled.div` grid-template-rows: repeat( 3, calc( var( --width ) / 3 ) ); width: var( --width, 92px ); - ${rootBase}; - ${rootBorder}; + ${rootBase} + ${rootBorder} `; const pointActive = ( { isActive } ) => { @@ -66,15 +66,15 @@ export const pointBase = ( props ) => { margin: auto; transition: all 120ms linear; - ${reduceMotion( 'transition' )}; - ${pointActive( props )}; + ${reduceMotion( 'transition' )} + ${pointActive( props )} `; }; export const Point = styled.span` height: 6px; width: 6px; - ${pointBase}; + ${pointBase} `; export const Cell = styled.span` diff --git a/packages/components/src/alignment-matrix-control/test/index.js b/packages/components/src/alignment-matrix-control/test/index.js new file mode 100644 index 00000000000000..05be599d4897e2 --- /dev/null +++ b/packages/components/src/alignment-matrix-control/test/index.js @@ -0,0 +1,40 @@ +/** + * External dependencies + */ +import { render, unmountComponentAtNode } from 'react-dom'; +import { act } from 'react-dom/test-utils'; + +/** + * Internal dependencies + */ +import AlignmentMatrixControl from '../'; + +let container = null; + +beforeEach( () => { + container = document.createElement( 'div' ); + document.body.appendChild( container ); +} ); + +afterEach( () => { + unmountComponentAtNode( container ); + container.remove(); + container = null; +} ); + +const getControl = () => { + return container.querySelector( '.component-alignment-matrix-control' ); +}; + +describe( 'AlignmentMatrixControl', () => { + describe( 'Basic rendering', () => { + it( 'should render', () => { + act( () => { + render( , container ); + } ); + const control = getControl(); + + expect( control ).toBeTruthy(); + } ); + } ); +} ); diff --git a/packages/components/src/alignment-matrix-control/test/utils.js b/packages/components/src/alignment-matrix-control/test/utils.js new file mode 100644 index 00000000000000..b44c6b1d92e1c3 --- /dev/null +++ b/packages/components/src/alignment-matrix-control/test/utils.js @@ -0,0 +1,160 @@ +/** + * Internal dependencies + */ +import { + ALIGNMENT_VALUES, + DIRECTION, + getAlignmentIndex, + isAlignmentValid, + getAlignmentValueFromIndex, + getNextIndexFromDirection, +} from '../utils'; + +describe( 'AlignmentMatrixControl', () => { + describe( 'isAlignmentValid', () => { + it( 'should return true for correctly matching alignment values', () => { + // x and y values + expect( isAlignmentValid( 'center top' ) ).toBe( true ); + expect( isAlignmentValid( 'top center' ) ).toBe( true ); + expect( isAlignmentValid( 'center bottom' ) ).toBe( true ); + // single center value + expect( isAlignmentValid( 'center' ) ).toBe( true ); + // x + y center value + expect( isAlignmentValid( 'center center' ) ).toBe( true ); + } ); + + it( 'should return false for invalid alignment values', () => { + expect( isAlignmentValid( 'top bottom' ) ).toBe( false ); + expect( isAlignmentValid( 'left right' ) ).toBe( false ); + expect( isAlignmentValid( 'near mid' ) ).toBe( false ); + expect( isAlignmentValid( 'mid mid' ) ).toBe( false ); + expect( isAlignmentValid( 'mid' ) ).toBe( false ); + } ); + + it( 'should only consider first 2 (max) values', () => { + expect( isAlignmentValid( 'center top right bottom' ) ).toBe( + false + ); + expect( isAlignmentValid( 'top center left right bottom' ) ).toBe( + false + ); + } ); + + it( 'should remap middle to center', () => { + expect( isAlignmentValid( 'middle' ) ).toBe( true ); + expect( isAlignmentValid( 'middle middle' ) ).toBe( true ); + expect( isAlignmentValid( 'top middle' ) ).toBe( true ); + expect( isAlignmentValid( 'middle bottom' ) ).toBe( true ); + } ); + } ); + + describe( 'getAlignmentIndex', () => { + it( 'should return correct index given an alignment value', () => { + ALIGNMENT_VALUES.forEach( ( alignment, index ) => { + expect( getAlignmentIndex( alignment ) ).toBe( index ); + } ); + } ); + } ); + + describe( 'getAlignmentValueFromIndex', () => { + it( 'should return correct value based on index', () => { + ALIGNMENT_VALUES.forEach( ( alignment, index ) => { + expect( getAlignmentValueFromIndex( index ) ).toBe( alignment ); + } ); + } ); + } ); + + describe( 'getNextIndexFromDirection', () => { + describe( 'Basic movements', () => { + it( 'should move index "up"', () => { + const currentIndex = 4; + + const nextDirection = getNextIndexFromDirection( + currentIndex, + DIRECTION.UP + ); + + expect( nextDirection ).toBe( 1 ); + } ); + + it( 'should move index "DOWN"', () => { + const currentIndex = 4; + + const nextDirection = getNextIndexFromDirection( + currentIndex, + DIRECTION.DOWN + ); + + expect( nextDirection ).toBe( 7 ); + } ); + + it( 'should move index "LEFT"', () => { + const currentIndex = 4; + + const nextDirection = getNextIndexFromDirection( + currentIndex, + DIRECTION.LEFT + ); + + expect( nextDirection ).toBe( 3 ); + } ); + + it( 'should move index "RIGHT"', () => { + const currentIndex = 4; + + const nextDirection = getNextIndexFromDirection( + currentIndex, + DIRECTION.RIGHT + ); + + expect( nextDirection ).toBe( 5 ); + } ); + } ); + + describe( 'Edge movements', () => { + it( 'should not move index "up" at at an edge', () => { + const currentIndex = 2; + + const nextDirection = getNextIndexFromDirection( + currentIndex, + DIRECTION.UP + ); + + expect( nextDirection ).toBe( 2 ); + } ); + + it( 'should not move index "down" at at an edge', () => { + const currentIndex = 7; + + const nextDirection = getNextIndexFromDirection( + currentIndex, + DIRECTION.DOWN + ); + + expect( nextDirection ).toBe( 7 ); + } ); + + it( 'should not move index "LEFT" at at an edge', () => { + const currentIndex = 3; + + const nextDirection = getNextIndexFromDirection( + currentIndex, + DIRECTION.LEFT + ); + + expect( nextDirection ).toBe( 3 ); + } ); + + it( 'should not move index "RIGHT" at at an edge', () => { + const currentIndex = 5; + + const nextDirection = getNextIndexFromDirection( + currentIndex, + DIRECTION.RIGHT + ); + + expect( nextDirection ).toBe( 5 ); + } ); + } ); + } ); +} ); diff --git a/packages/components/src/alignment-matrix-control/test/utils.test.js b/packages/components/src/alignment-matrix-control/test/utils.test.js deleted file mode 100644 index 65373269e1c8fe..00000000000000 --- a/packages/components/src/alignment-matrix-control/test/utils.test.js +++ /dev/null @@ -1,55 +0,0 @@ -/** - * Internal dependencies - */ -import { - ALIGNMENT_VALUES, - getAlignmentIndex, - isAlignmentValid, -} from '../utils'; - -describe( 'AlignmentMatrixControl', () => { - describe( 'isAlignmentValid', () => { - it( 'should return true for correctly matching alignment values', () => { - // x and y values - expect( isAlignmentValid( 'center top' ) ).toBe( true ); - expect( isAlignmentValid( 'top center' ) ).toBe( true ); - expect( isAlignmentValid( 'center bottom' ) ).toBe( true ); - // single center value - expect( isAlignmentValid( 'center' ) ).toBe( true ); - // x + y center value - expect( isAlignmentValid( 'center center' ) ).toBe( true ); - } ); - - it( 'should return false for invalid alignment values', () => { - expect( isAlignmentValid( 'top bottom' ) ).toBe( false ); - expect( isAlignmentValid( 'left right' ) ).toBe( false ); - expect( isAlignmentValid( 'near mid' ) ).toBe( false ); - expect( isAlignmentValid( 'mid mid' ) ).toBe( false ); - expect( isAlignmentValid( 'mid' ) ).toBe( false ); - } ); - - it( 'should only consider first 2 (max) values', () => { - expect( isAlignmentValid( 'center top right bottom' ) ).toBe( - false - ); - expect( isAlignmentValid( 'top center left right bottom' ) ).toBe( - false - ); - } ); - - it( 'should remap middle to center', () => { - expect( isAlignmentValid( 'middle' ) ).toBe( true ); - expect( isAlignmentValid( 'middle middle' ) ).toBe( true ); - expect( isAlignmentValid( 'top middle' ) ).toBe( true ); - expect( isAlignmentValid( 'middle bottom' ) ).toBe( true ); - } ); - } ); - - describe( 'getAlignmentIndex', () => { - it( 'should return correct index given an alignment value', () => { - ALIGNMENT_VALUES.forEach( ( alignment, index ) => { - expect( getAlignmentIndex( alignment ) ).toBe( index ); - } ); - } ); - } ); -} ); diff --git a/packages/components/src/alignment-matrix-control/utils.js b/packages/components/src/alignment-matrix-control/utils.js index dbb97e06966fb9..b21eeac4f2e868 100644 --- a/packages/components/src/alignment-matrix-control/utils.js +++ b/packages/components/src/alignment-matrix-control/utils.js @@ -3,6 +3,11 @@ */ import { isEqual } from 'lodash'; +/** + * Internal dependencies + */ +import { getRTL } from '../utils/rtl'; + export const DIRECTION = { UP: 'up', DOWN: 'down', @@ -51,7 +56,9 @@ export function transformAlignment( alignments = [] ) { value = value .map( ( v ) => v.toLowerCase() ) // Supports remapping of 'middle' to 'center' - .map( ( v ) => v.replace( 'middle', 'center' ) ); + .map( ( v ) => v.replace( 'middle', 'center' ) ) + // Flips for RTL + .map( transformAlignmentRTL ); // Handles cases were only 'center' or ['center'] is provided if ( value.length === 1 && value[ 0 ] === 'center' ) { @@ -60,6 +67,28 @@ export function transformAlignment( alignments = [] ) { return value.sort(); } +/** + * Transforms alignment values to respect RTL. + * + * @param {string} value The alignment value to transform. + * + * @return {string} The flipped alignment value (if RTL). + */ +function transformAlignmentRTL( value ) { + const isRTL = getRTL(); + const token = '$TMP'; + + if ( ! isRTL ) return value; + + return ( + value + // Temporarily swap left for token + .replace( 'left', token ) + .replace( 'right', 'left' ) + // Swap tokenized left for actual value + .replace( token, 'right' ) + ); +} /** * Checks if a value is a valid alignment. diff --git a/packages/components/src/range-control/index.js b/packages/components/src/range-control/index.js index d718dec1a229a0..263abed8617a4e 100644 --- a/packages/components/src/range-control/index.js +++ b/packages/components/src/range-control/index.js @@ -37,7 +37,7 @@ import { Thumb, Wrapper, } from './styles/range-control-styles'; -import { useRtl } from '../utils/rtl'; +import { useRTL } from '../utils/rtl'; const BaseRangeControl = forwardRef( ( @@ -70,7 +70,7 @@ const BaseRangeControl = forwardRef( }, ref ) => { - const isRTL = useRtl(); + const isRTL = useRTL(); const sliderValue = valueProp !== undefined ? valueProp : initialPosition; diff --git a/packages/components/src/utils/rtl.js b/packages/components/src/utils/rtl.js index bc312455baa3a6..88a3f651af96c1 100644 --- a/packages/components/src/utils/rtl.js +++ b/packages/components/src/utils/rtl.js @@ -14,7 +14,7 @@ const UPPER_RIGHT_REGEXP = new RegExp( /Right/g ); * * @return {boolean} Whether document is RTL. */ -function getRtl() { +export function getRTL() { return !! ( document && document.documentElement.dir === 'rtl' ); } @@ -23,8 +23,8 @@ function getRtl() { * * @return {boolean} Whether document is RTL. */ -export function useRtl() { - return getRtl(); +export function useRTL() { + return getRTL(); } /** @@ -83,12 +83,12 @@ export const convertLTRToRTL = ( ltrStyles = {} ) => { */ export function rtl( ltrStyles = {}, rtlStyles ) { return () => { - const isRtl = getRtl(); + const isRTL = getRTL(); if ( rtlStyles ) { - return isRtl ? css( rtlStyles ) : css( ltrStyles ); + return isRTL ? css( rtlStyles ) : css( ltrStyles ); } - return isRtl ? css( convertLTRToRTL( ltrStyles ) ) : css( ltrStyles ); + return isRTL ? css( convertLTRToRTL( ltrStyles ) ) : css( ltrStyles ); }; }