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.

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

Breadcrumb: add accessibility label #19597

Merged
merged 5 commits into from
Jan 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ function BlockPopover( {
{ shouldShowBreadcrumb && (
<BlockBreadcrumb
clientId={ clientId }
rootClientId={ rootClientId }
moverDirection={ moverDirection }
data-align={ align }
/>
) }
Expand Down
38 changes: 4 additions & 34 deletions packages/block-editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -87,7 +60,6 @@ function BlockListBlock( {
isSelectionEnabled,
className,
name,
index,
isValid,
attributes,
initialPosition,
Expand Down Expand Up @@ -134,7 +106,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

Expand Down Expand Up @@ -314,7 +287,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={
Expand Down Expand Up @@ -362,7 +335,6 @@ const applyWithSelect = withSelect(
getSettings,
hasSelectedInnerBlock,
getTemplateLock,
getBlockIndex,
__unstableGetBlockWithoutInnerBlocks,
isNavigationMode,
} = select( 'core/block-editor' );
Expand All @@ -375,7 +347,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.
Expand All @@ -401,7 +372,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
Expand Down
24 changes: 21 additions & 3 deletions packages/block-editor/src/components/block-list/breadcrumb.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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( () => {
Expand All @@ -42,13 +56,17 @@ function BlockBreadcrumb( { clientId, ...props } ) {
}
}

const blockType = getBlockType( name );
const label = getAccessibleBlockLabel( blockType, attributes, index + 1, moverDirection );

return (
<div className="block-editor-block-list__breadcrumb" { ...props }>
<Toolbar>
<Button
ref={ ref }
onClick={ () => setNavigationMode( false ) }
onKeyDown={ onKeyDown }
label={ label }
>
<BlockTitle clientId={ clientId } />
</Button>
Expand Down
2 changes: 0 additions & 2 deletions packages/block-editor/src/components/block-list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ const forceSyncUpdates = ( WrappedComponent ) => ( props ) => {
function BlockList( {
className,
rootClientId,
__experimentalMoverDirection: moverDirection = 'vertical',
isDraggable,
renderAppender,
__experimentalUIParts = {},
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions packages/e2e-test-utils/src/transform-block-to.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }` );
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ 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' );

await tabThroughBlockMoverControl();
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' );
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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)' );
Expand All @@ -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( () => {
Expand Down