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

New: output block position for screen readers. Update: refactor the way the screen reader information on block button is delivered. #29060

Closed
wants to merge 46 commits into from
Closed
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
bf6325c
i
Feb 16, 2021
4d711e4
Try to import getBlockCount.
Feb 16, 2021
112ba24
Try to import getBlockCount.
Feb 16, 2021
06e53a6
Use existing selector.
Feb 16, 2021
d8fb0e2
Pass in rootClientId to getBlockCount.
Feb 16, 2021
bf04b90
Pass our new total arg in to the accessible label function.
Feb 16, 2021
b467a77
Attempt to fix tests.
Feb 16, 2021
7e119f9
Use description text to output instead of aria-label.
Feb 16, 2021
bb8ac20
A fix to button component to use proper ID passed in to span.
Feb 16, 2021
93e5256
Define custom unique description ID constructed with clientId of block.
Feb 16, 2021
0f4dc5a
Try to output accessible Block label after block title output.
Feb 16, 2021
bd2f11c
Render span instead of div.
Feb 16, 2021
8f8e0c0
Get rid of duplicate title. Add spacing instead of : .
Feb 16, 2021
22979b8
Auto format.
Feb 17, 2021
41259f4
Merge branch 'master' of https://github.com/wordpress/gutenberg into …
Feb 17, 2021
3da7129
Test case.
Feb 17, 2021
6f86d03
Modify the accessible description function with parent title.
Feb 17, 2021
219e94e
Change text.
Feb 17, 2021
4bae102
Formatting fixes and additional comments.
Feb 17, 2021
5b65ebc
No verify.
Feb 17, 2021
4b312f9
Try to sync package-lock.json with master.
Feb 17, 2021
3ef07e1
Refresh.
Feb 17, 2021
f5a163e
Revert package-lock.json to upstream master.
Feb 17, 2021
15d5381
Use createInterpolateElement for title output.
Feb 17, 2021
1e4bf55
Fix title block tests.
Feb 17, 2021
040516e
Fix title bugs. Output parentTitle in SR block description via sprint…
Feb 18, 2021
35e3e53
Merge branch 'master' of https://github.com/wordpress/gutenberg into …
Feb 18, 2021
857265d
Remove mods to button component.
Feb 18, 2021
2fdf221
Pass rootClientId from selector callback.
Feb 18, 2021
780f2c6
Merge branch 'master' of https://github.com/wordpress/gutenberg
Feb 18, 2021
b6108ef
Remove unused sprintf.
Feb 19, 2021
accb462
Merge branch 'master' of https://github.com/wordpress/gutenberg
Feb 22, 2021
f040379
Merge branch 'master' of https://github.com/wordpress/gutenberg into …
Feb 23, 2021
ba88fb9
Fix invalid sprintf arg.
Feb 24, 2021
ac76c53
Merge branch 'master' of https://github.com/wordpress/gutenberg into …
Feb 24, 2021
94387c7
Fix parentTitle output for blocks with label.
Feb 24, 2021
7fcb514
Give total variable a better descriptive name blocksTotal.
Mar 5, 2021
025c4ad
Merge branch 'master' of https://github.com/wordpress/gutenberg into …
Mar 5, 2021
96b20fb
Merge branch 'master' of https://github.com/wordpress/gutenberg
Mar 23, 2021
4f59b3f
Refresh.
Mar 24, 2021
0de9d39
Fix conflict.
Mar 24, 2021
a0c8034
Merge branch 'trunk' of https://github.com/wordpress/gutenberg into n…
Mar 26, 2021
32f297c
Correct error that popped up during merge from trunk.
Mar 26, 2021
8b4d2ce
Revert changes to title.
Mar 30, 2021
0a37674
Merge branch 'trunk' of https://github.com/wordpress/gutenberg into n…
Mar 30, 2021
2e925b0
Add title back to description.
Mar 30, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { focus } from '@wordpress/dom';
*/
import BlockTitle from '../block-title';
import { store as blockEditorStore } from '../../store';
import useBlockDisplayInformation from '../use-block-display-information';

/**
* Returns true if the user is using windows.
Expand All @@ -51,6 +52,7 @@ function selector( select ) {
getNextBlockClientId,
hasBlockMovingClientId,
getBlockIndex,
getBlockCount,
getBlockRootClientId,
getClientIdsOfDescendants,
canInsertBlockType,
Expand All @@ -70,6 +72,7 @@ function selector( select ) {
),
hasBlockMovingClientId,
getBlockIndex,
getBlockCount,
getBlockRootClientId,
getClientIdsOfDescendants,
canInsertBlockType,
Expand All @@ -93,25 +96,39 @@ function BlockSelectionButton( { clientId, rootClientId, blockElement } ) {
const {
__unstableGetBlockWithoutInnerBlocks,
getBlockIndex,
getBlockCount,
hasBlockMovingClientId,
getBlockListSettings,
getBlockRootClientId,
} = select( blockEditorStore );
const index = getBlockIndex( clientId, rootClientId );
const total = getBlockCount( rootClientId );
const passedRootClientId = getBlockRootClientId( clientId );
const { name, attributes } = __unstableGetBlockWithoutInnerBlocks(
clientId
);
const blockMovingMode = hasBlockMovingClientId();
return {
index,
total,
name,
attributes,
blockMovingMode,
orientation: getBlockListSettings( rootClientId )?.orientation,
passedRootClientId,
};
},
[ clientId, rootClientId ]
);
const { index, name, attributes, blockMovingMode, orientation } = selected;
const {
index,
total,
name,
attributes,
blockMovingMode,
orientation,
passedRootClientId,
} = selected;
const { setNavigationMode, removeBlock } = useDispatch( blockEditorStore );
const ref = useRef();

Expand Down Expand Up @@ -242,11 +259,20 @@ function BlockSelectionButton( { clientId, rootClientId, blockElement } ) {
}

const blockType = getBlockType( name );
let parentTitle;
const blockInformation = useBlockDisplayInformation( passedRootClientId );
if ( blockInformation !== null ) {
parentTitle = blockInformation.title
? blockInformation.title
: undefined;
}
const label = getAccessibleBlockLabel(
blockType,
attributes,
index + 1,
orientation
orientation,
total,
alexstine marked this conversation as resolved.
Show resolved Hide resolved
parentTitle
);

const classNames = classnames(
Expand All @@ -262,7 +288,7 @@ function BlockSelectionButton( { clientId, rootClientId, blockElement } ) {
ref={ ref }
onClick={ () => setNavigationMode( false ) }
onKeyDown={ onKeyDown }
label={ label }
describedBy={ label }
>
<BlockTitle clientId={ clientId } />
alexstine marked this conversation as resolved.
Show resolved Hide resolved
</Button>
Expand Down
14 changes: 13 additions & 1 deletion packages/block-editor/src/components/block-title/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@ import { truncate } from 'lodash';
* WordPress dependencies
*/
import { useSelect } from '@wordpress/data';
import { __, sprintf } from '@wordpress/i18n';
import { createInterpolateElement } from '@wordpress/element';
import {
getBlockType,
__experimentalGetBlockLabel as getBlockLabel,
isReusableBlock,
} from '@wordpress/blocks';
import { VisuallyHidden } from '@wordpress/components';

/**
* Internal dependencies
Expand Down Expand Up @@ -73,5 +76,14 @@ export default function BlockTitle( { clientId } ) {
if ( label !== blockType.title ) {
return truncate( label, { length: 35 } );
}
return blockInformation.title;
return createInterpolateElement(
sprintf(
/* translators: 1: The block title. */
__( '%s <VisuallyHidden>Block.</VisuallyHidden>' ),
alexstine marked this conversation as resolved.
Show resolved Hide resolved
blockInformation.title
),
{
VisuallyHidden: <VisuallyHidden as="span" />,
}
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ describe( 'BlockTitle', () => {

const wrapper = shallow( <BlockTitle clientId="id-name-exists" /> );

expect( wrapper.text() ).toBe( 'Block Title' );
expect( wrapper.text() ).toBe( 'Block Title Block.' );
} );

it( 'renders label if it is set', () => {
Expand Down
84 changes: 57 additions & 27 deletions packages/blocks/src/api/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,19 +161,24 @@ export function getBlockLabel( blockType, attributes, context = 'visual' ) {
* @param {Object} attributes The values of the block's attributes.
* @param {?number} position The position of the block in the block list.
* @param {string} [direction='vertical'] The direction of the block layout.
* @param {number} total The total number of blocks.
* @param {string} parentTitle In the event of a child block, this param gets the title of the parent block.
*
* @return {string} The block label.
*/
export function getAccessibleBlockLabel(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little surprised there aren't unit tests for this function. Probably my fault as I think I initially wrote it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, just realise there are unit tests. So these will need to be updated, with new test cases added for the additional branches of code. Happy to help with that, either directly or by providing guidance, let me know.

There are docs for testing, but not sure how good they are.

blockType,
attributes,
position,
direction = 'vertical'
direction = 'vertical',
total,
parentTitle
Copy link
Contributor

@guarani guarani Mar 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is used in a React Native file, which will need updating as well:

const accessibilityLabel = getAccessibleBlockLabel(
blockType,
attributes,
order + 1
);

The new total parameter here I understand corresponds to the block count, which comes from:

export const getGlobalBlockCount = createSelector(

) {
// `title` is already localized, `label` is a user-supplied value.
const { title } = blockType;
const label = getBlockLabel( blockType, attributes, 'accessibility' );
const hasPosition = position !== undefined;
const hasParentTitle = parentTitle !== undefined;

// getBlockLabel returns the block title as a fallback when there's no label,
// if it did return the title, this function needs to avoid adding the
Expand All @@ -184,53 +189,78 @@ export function getAccessibleBlockLabel(
if ( hasPosition && direction === 'vertical' ) {
if ( hasLabel ) {
return sprintf(
/* translators: accessibility text. 1: The block title. 2: The block row number. 3: The block label.. */
__( '%1$s Block. Row %2$d. %3$s' ),
title,
/* translators: accessibility text. 1: The block row number. 2: The total number of blocks. 3: The block label.. */
__( '%1$d of %2$d. %3$s' ),
position,
total,
label
);
} else if ( hasLabel && hasParentTitle ) {
return sprintf(
/* translators: accessibility text. 1: The block row number. 2: The total number of blocks. 3: The parent block title. 4: The block label.. */
__( '%1$d of %2$d. Child of %3$s block. %4$s' ),
position,
total,
parentTitle,
label
);
}

if ( hasParentTitle ) {
return sprintf(
/* translators: accessibility text. 1: The block row number. 2: The total number of blocks. 3: The parent block title. */
__( '%1$d of %2$d. Child of %3$s block.' ),
position,
total,
parentTitle
);
}
return sprintf(
/* translators: accessibility text. 1: The block title. 2: The block row number. */
__( '%1$s Block. Row %2$d' ),
title,
position
/* translators: accessibility text. 1: The block row number. 2: The total number of blocks. */
__( '%1$d of %2$d.' ),
position,
total
);
} else if ( hasPosition && direction === 'horizontal' ) {
if ( hasLabel ) {
return sprintf(
/* translators: accessibility text. 1: The block title. 2: The block column number. 3: The block label.. */
__( '%1$s Block. Column %2$d. %3$s' ),
title,
/* translators: accessibility text. 1: The block column number. 2: The total number of blocks. 3: The block label.. */
__( 'Column %1$d of %2$d. %4$s' ),
alexstine marked this conversation as resolved.
Show resolved Hide resolved
position,
total,
label
);
} else if ( hasLabel && hasParentTitle ) {
return sprintf(
/* translators: accessibility text. 1: The block column number. 2: The total number of blocks. 3: The block parent title. 4: The block label.. */
__( 'Column %1$d of %2$d. Child of %3$s block. %4$s' ),
position,
total,
parentTitle,
label
);
}

if ( hasParentTitle ) {
return sprintf(
/* translators: accessibility text. 1: The block column number. 2: The total number of blocks. 3: The block parent title. */
__( 'Column %1$d of %2$d. Child of %3$s block.' ),
position,
total,
parentTitle
);
}
return sprintf(
/* translators: accessibility text. 1: The block title. 2: The block column number. */
__( '%1$s Block. Column %2$d' ),
title,
position
/* translators: accessibility text. 1: The block column number. 2: The total number of blocks. */
__( 'Column %1$d of %2$d.' ),
position,
total
);
}

if ( hasLabel ) {
return sprintf(
/* translators: accessibility text. %1: The block title. %2: The block label. */
__( '%1$s Block. %2$s' ),
title,
label
);
return label;
}

return sprintf(
/* translators: accessibility text. %s: The block title. */
__( '%s Block' ),
title
);
}

/**
Expand Down