From e08720a43e50d715d9bb9fa097c9af4da67b406a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 16 Jan 2020 11:53:37 +0100 Subject: [PATCH] Breadcrumb: add accessibility label (#19597) * Breadcrumb: add accessibility label * Polish * Clean up * Remove mover prop * Fix selector --- .../components/block-list/block-popover.js | 2 + .../src/components/block-list/block.js | 38 ++----------------- .../src/components/block-list/breadcrumb.js | 24 ++++++++++-- .../src/components/block-list/index.js | 2 - .../e2e-test-utils/src/transform-block-to.js | 6 +-- .../various/keyboard-navigable-blocks.test.js | 16 ++++---- 6 files changed, 38 insertions(+), 50 deletions(-) diff --git a/packages/block-editor/src/components/block-list/block-popover.js b/packages/block-editor/src/components/block-list/block-popover.js index 0d2563547df02a..453b56b69f17dc 100644 --- a/packages/block-editor/src/components/block-list/block-popover.js +++ b/packages/block-editor/src/components/block-list/block-popover.js @@ -160,6 +160,8 @@ function BlockPopover( { { shouldShowBreadcrumb && ( ) } diff --git a/packages/block-editor/src/components/block-list/block.js b/packages/block-editor/src/components/block-list/block.js index 933d9c5787a08b..899287b7c815b2 100644 --- a/packages/block-editor/src/components/block-list/block.js +++ b/packages/block-editor/src/components/block-list/block.js @@ -21,9 +21,9 @@ import { isReusableBlock, isUnmodifiedDefaultBlock, getUnregisteredTypeHandlerName, - __experimentalGetAccessibleBlockLabel as getAccessibleBlockLabel, } from '@wordpress/blocks'; import { withFilters } from '@wordpress/components'; +import { __, sprintf } from '@wordpress/i18n'; import { withDispatch, withSelect, @@ -45,36 +45,9 @@ import { isInsideRootBlock } from '../../utils/dom'; import useMovingAnimation from './moving-animation'; import { Context } from './root-container'; -/** - * A debounced version of getAccessibleBlockLabel, avoids unnecessary updates to the aria-label attribute - * when typing in some blocks, like the paragraph. - * - * @param {Object} blockType The block type object representing the block's definition. - * @param {Object} attributes The block's attribute values. - * @param {number} index The index of the block in the block list. - * @param {string} moverDirection A string representing whether the movers are displayed vertically or horizontally. - * @param {number} delay The debounce delay. - */ -const useDebouncedAccessibleBlockLabel = ( blockType, attributes, index, moverDirection, delay ) => { - const [ blockLabel, setBlockLabel ] = useState( '' ); - - useEffect( () => { - const timeoutId = setTimeout( () => { - setBlockLabel( getAccessibleBlockLabel( blockType, attributes, index + 1, moverDirection ) ); - }, delay ); - - return () => { - clearTimeout( timeoutId ); - }; - }, [ blockType, attributes, index, moverDirection, delay ] ); - - return blockLabel; -}; - function BlockListBlock( { mode, isFocusMode, - moverDirection, isLocked, clientId, isSelected, @@ -87,7 +60,6 @@ function BlockListBlock( { isSelectionEnabled, className, name, - index, isValid, attributes, initialPosition, @@ -131,7 +103,8 @@ function BlockListBlock( { const onBlockError = () => setErrorState( true ); const blockType = getBlockType( name ); - const blockAriaLabel = useDebouncedAccessibleBlockLabel( blockType, attributes, index, moverDirection, 400 ); + // translators: %s: Type of block (i.e. Text, Image etc) + const blockLabel = sprintf( __( 'Block: %s' ), blockType.title ); // Handing the focus of the block on creation and update @@ -312,7 +285,7 @@ function BlockListBlock( { // Only allow selection to be started from a selected block. onMouseLeave={ isSelected ? onMouseLeave : undefined } tabIndex="0" - aria-label={ blockAriaLabel } + aria-label={ blockLabel } role="group" { ...wrapperProps } style={ @@ -358,7 +331,6 @@ const applyWithSelect = withSelect( getSettings, hasSelectedInnerBlock, getTemplateLock, - getBlockIndex, __unstableGetBlockWithoutInnerBlocks, isNavigationMode, } = select( 'core/block-editor' ); @@ -371,7 +343,6 @@ const applyWithSelect = withSelect( // "ancestor" is the more appropriate label due to "deep" check const isAncestorOfSelectedBlock = hasSelectedInnerBlock( clientId, checkDeep ); - const index = getBlockIndex( clientId, rootClientId ); // The fallback to `{}` is a temporary fix. // This function should never be called when a block is not present in the state. @@ -397,7 +368,6 @@ const applyWithSelect = withSelect( isLocked: !! templateLock, isFocusMode: focusMode && isLargeViewport, isNavigationMode: isNavigationMode(), - index, isRTL, // Users of the editor.BlockListBlock filter used to be able to access the block prop diff --git a/packages/block-editor/src/components/block-list/breadcrumb.js b/packages/block-editor/src/components/block-list/breadcrumb.js index 283b818b3b907b..2f0c4f28bd9180 100644 --- a/packages/block-editor/src/components/block-list/breadcrumb.js +++ b/packages/block-editor/src/components/block-list/breadcrumb.js @@ -2,9 +2,13 @@ * WordPress dependencies */ import { Toolbar, Button } from '@wordpress/components'; -import { useDispatch } from '@wordpress/data'; +import { useSelect, useDispatch } from '@wordpress/data'; import { useEffect, useRef } from '@wordpress/element'; import { BACKSPACE, DELETE } from '@wordpress/keycodes'; +import { + getBlockType, + __experimentalGetAccessibleBlockLabel as getAccessibleBlockLabel, +} from '@wordpress/blocks'; /** * Internal dependencies @@ -21,12 +25,22 @@ import BlockTitle from '../block-title'; * * @return {WPComponent} The component to be rendered. */ -function BlockBreadcrumb( { clientId, ...props } ) { - const ref = useRef(); +function BlockBreadcrumb( { clientId, rootClientId, moverDirection, ...props } ) { + const selected = useSelect( ( select ) => { + const { + __unstableGetBlockWithoutInnerBlocks, + getBlockIndex, + } = select( 'core/block-editor' ); + const index = getBlockIndex( clientId, rootClientId ); + const { name, attributes } = __unstableGetBlockWithoutInnerBlocks( clientId ); + return { index, name, attributes }; + }, [ clientId, rootClientId ] ); + const { index, name, attributes } = selected; const { setNavigationMode, removeBlock, } = useDispatch( 'core/block-editor' ); + const ref = useRef(); // Focus the breadcrumb in navigation mode. useEffect( () => { @@ -42,6 +56,9 @@ function BlockBreadcrumb( { clientId, ...props } ) { } } + const blockType = getBlockType( name ); + const label = getAccessibleBlockLabel( blockType, attributes, index + 1, moverDirection ); + return (
@@ -49,6 +66,7 @@ function BlockBreadcrumb( { clientId, ...props } ) { ref={ ref } onClick={ () => setNavigationMode( false ) } onKeyDown={ onKeyDown } + label={ label } > diff --git a/packages/block-editor/src/components/block-list/index.js b/packages/block-editor/src/components/block-list/index.js index 5e2e9459215415..1fd0f8afa9fc50 100644 --- a/packages/block-editor/src/components/block-list/index.js +++ b/packages/block-editor/src/components/block-list/index.js @@ -35,7 +35,6 @@ const forceSyncUpdates = ( WrappedComponent ) => ( props ) => { function BlockList( { className, rootClientId, - __experimentalMoverDirection: moverDirection = 'vertical', isDraggable, renderAppender, __experimentalUIParts = {}, @@ -99,7 +98,6 @@ function BlockList( { rootClientId={ rootClientId } clientId={ clientId } isDraggable={ isDraggable } - moverDirection={ moverDirection } isMultiSelecting={ isMultiSelecting } // This prop is explicitely computed and passed down // to avoid being impacted by the async mode diff --git a/packages/e2e-test-utils/src/transform-block-to.js b/packages/e2e-test-utils/src/transform-block-to.js index 9cc5bc80b17bb9..2bf0b5d559a354 100644 --- a/packages/e2e-test-utils/src/transform-block-to.js +++ b/packages/e2e-test-utils/src/transform-block-to.js @@ -18,7 +18,7 @@ export async function transformBlockTo( name ) { await insertButton.click(); // Wait for the transformed block to appear. - const BLOCK_SELECTOR = '[contains(@class, "block-editor-block-list__block")]'; - const BLOCK_NAME_SELECTOR = `[contains(@aria-label, "${ name } Block")]`; - await page.waitForXPath( `//*${ BLOCK_SELECTOR }${ BLOCK_NAME_SELECTOR }` ); + const BLOCK_SELECTOR = '.block-editor-block-list__block'; + const BLOCK_NAME_SELECTOR = `[aria-label="Block: ${ name }"]`; + await page.waitForSelector( `${ BLOCK_SELECTOR }${ BLOCK_NAME_SELECTOR }` ); } diff --git a/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js b/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js index bdf0c34f962837..c2830d5e473b77 100644 --- a/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js +++ b/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js @@ -22,7 +22,7 @@ const navigateToContentEditorTop = async () => { await pressKeyWithModifier( 'ctrl', '`' ); }; -const tabThroughParagraphBlock = async ( paragraphText, blockIndex ) => { +const tabThroughParagraphBlock = async ( paragraphText ) => { await page.keyboard.press( 'Tab' ); await expect( await getActiveLabel() ).toBe( 'Add block' ); @@ -30,7 +30,7 @@ const tabThroughParagraphBlock = async ( paragraphText, blockIndex ) => { await tabThroughBlockToolbar(); await page.keyboard.press( 'Tab' ); - await expect( await getActiveLabel() ).toBe( `Paragraph Block. Row ${ blockIndex + 1 }. ${ paragraphText }` ); + await expect( await getActiveLabel() ).toBe( 'Block: Paragraph' ); await page.keyboard.press( 'Tab' ); await expect( await getActiveLabel() ).toBe( 'Paragraph block' ); @@ -94,12 +94,12 @@ describe( 'Order of block keyboard navigation', () => { await page.mouse.move( 10, 10 ); await navigateToContentEditorTop(); - await tabThroughParagraphBlock( 'Paragraph 1', 1 ); + await tabThroughParagraphBlock( 'Paragraph 1' ); // Repeat the same steps to ensure that there is no change introduced in how the focus is handled. // This prevents the previous regression explained in: https://github.com/WordPress/gutenberg/issues/11773. await navigateToContentEditorTop(); - await tabThroughParagraphBlock( 'Paragraph 1', 1 ); + await tabThroughParagraphBlock( 'Paragraph 1' ); } ); it( 'allows tabbing in navigation mode if no block is selected', async () => { @@ -122,10 +122,10 @@ describe( 'Order of block keyboard navigation', () => { } ) ).toBe( 'Add title' ); await page.keyboard.press( 'Tab' ); - await expect( await getActiveLabel() ).toBe( 'Paragraph' ); + await expect( await getActiveLabel() ).toBe( 'Paragraph Block. Row 1. 0' ); await page.keyboard.press( 'Tab' ); - await expect( await getActiveLabel() ).toBe( 'Paragraph' ); + await expect( await getActiveLabel() ).toBe( 'Paragraph Block. Row 2. 1' ); await page.keyboard.press( 'Tab' ); await expect( await getActiveLabel() ).toBe( 'Document (selected)' ); @@ -147,10 +147,10 @@ describe( 'Order of block keyboard navigation', () => { } ); await pressKeyWithModifier( 'shift', 'Tab' ); - await expect( await getActiveLabel() ).toBe( 'Paragraph' ); + await expect( await getActiveLabel() ).toBe( 'Paragraph Block. Row 2. 1' ); await pressKeyWithModifier( 'shift', 'Tab' ); - await expect( await getActiveLabel() ).toBe( 'Paragraph' ); + await expect( await getActiveLabel() ).toBe( 'Paragraph Block. Row 1. 0' ); await pressKeyWithModifier( 'shift', 'Tab' ); await expect( await page.evaluate( () => {