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.

Sign up for GitHub

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

List View: Add multi-select support for shift + Home and End keys #39272

Merged
merged 8 commits into from
Mar 23, 2022
Merged

List View: Add multi-select support for shift + Home and End keys #39272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fedaf18
List View: Add multi-select support for shift + Home and End keys
andrewserong Mar 8, 2022
058b8a6
Skip announcing deselected blocks when using Home and End keys for th…
andrewserong Mar 8, 2022
2692f44
Try setting preventAnnouncement at the parent list view, and pass dow…
andrewserong Mar 18, 2022
159b2a2
Remove preventAnnouncement and move block description up to the grid …
andrewserong Mar 21, 2022
a8219d3
Add link to block title label, restore aria-expanded at the button level
andrewserong Mar 22, 2022
39f6a76
Update e2e tests to look for aria-selected attributes
andrewserong Mar 22, 2022
4dc20b9
Try removing code that prevents the announcement of deselected blocks
andrewserong Mar 22, 2022
ad382ac
Add back in skipping announcing deselected blocks
andrewserong Mar 23, 2022
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
31 changes: 2 additions & 29 deletions packages/block-editor/src/components/list-view/block-select-button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,14 @@ import classnames from 'classnames';
/**
* WordPress dependencies
*/
import { Button, VisuallyHidden } from '@wordpress/components';
import { useInstanceId } from '@wordpress/compose';
import { Button } from '@wordpress/components';
import { forwardRef } from '@wordpress/element';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import BlockIcon from '../block-icon';
import useBlockDisplayInformation from '../use-block-display-information';
import { getBlockPositionDescription } from './utils';
import BlockTitle from '../block-title';
import ListViewExpander from './expander';
import { SPACE, ENTER } from '@wordpress/keycodes';
Expand All @@ -25,29 +22,17 @@ function ListViewBlockSelectButton(
{
className,
block: { clientId },
isSelected,
onClick,
onToggleExpanded,
position,
siblingBlockCount,
level,
tabIndex,
onFocus,
onDragStart,
onDragEnd,
draggable,
isExpanded,
},
ref
) {
const blockInformation = useBlockDisplayInformation( clientId );
const instanceId = useInstanceId( ListViewBlockSelectButton );
const descriptionId = `list-view-block-select-button__${ instanceId }`;
const blockPositionDescription = getBlockPositionDescription(
position,
siblingBlockCount,
level
);

// The `href` attribute triggers the browser's native HTML drag operations.
// When the link is dragged, the element's outerHTML is set in DataTransfer object as text/html.
Expand All @@ -73,15 +58,14 @@ function ListViewBlockSelectButton(
) }
onClick={ onClick }
onKeyDown={ onKeyDownHandler }
aria-describedby={ descriptionId }
ref={ ref }
tabIndex={ tabIndex }
onFocus={ onFocus }
onDragStart={ onDragStartHandler }
onDragEnd={ onDragEnd }
draggable={ draggable }
href={ `#block-${ clientId }` }
aria-expanded={ isExpanded }
andrewserong marked this conversation as resolved.
Show resolved Hide resolved
aria-hidden={ true }
andrewserong marked this conversation as resolved.
Show resolved Hide resolved
>
<ListViewExpander onClick={ onToggleExpanded } />
<BlockIcon icon={ blockInformation?.icon } showColors />
Expand All @@ -91,18 +75,7 @@ function ListViewBlockSelectButton(
{ blockInformation.anchor }
</span>
) }
{ isSelected && (
<VisuallyHidden>
{ __( '(selected block)' ) }
</VisuallyHidden>
) }
</Button>
<div
className="block-editor-list-view-block-select-button__description"
id={ descriptionId }
>
{ blockPositionDescription }
</div>
</>
);
}
Expand Down
54 changes: 44 additions & 10 deletions packages/block-editor/src/components/list-view/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
__experimentalTreeGridCell as TreeGridCell,
__experimentalTreeGridItem as TreeGridItem,
} from '@wordpress/components';
import { useInstanceId } from '@wordpress/compose';
import { moreVertical } from '@wordpress/icons';
import {
useState,
Expand All @@ -32,6 +33,7 @@ import {
import ListViewBlockContents from './block-contents';
import BlockSettingsDropdown from '../block-settings-menu/block-settings-dropdown';
import { useListViewContext } from './context';
import { getBlockPositionDescription } from './utils';
import { store as blockEditorStore } from '../../store';
import useBlockDisplayInformation from '../use-block-display-information';

Expand All @@ -49,13 +51,39 @@ function ListViewBlock( {
path,
isExpanded,
selectedClientIds,
preventAnnouncement,
} ) {
const cellRef = useRef( null );
const [ isHovered, setIsHovered ] = useState( false );
const { clientId } = block;

const { toggleBlockHighlight } = useDispatch( blockEditorStore );

const blockInformation = useBlockDisplayInformation( clientId );
const instanceId = useInstanceId( ListViewBlock );
const descriptionId = `list-view-block-select-button__${ instanceId }`;
const blockPositionDescription = getBlockPositionDescription(
position,
siblingBlockCount,
level
);

const blockAriaLabel = blockInformation
? sprintf(
// translators: %s: The title of the block. This string indicates a link to select the block.
__( '%s link' ),
blockInformation.title
)
: __( 'Link' );

const settingsAriaLabel = blockInformation
? sprintf(
// translators: %s: The title of the block.
__( 'Options for %s block' ),
blockInformation.title
)
: __( 'Options' );

const {
__experimentalFeatures: withExperimentalFeatures,
__experimentalPersistentListViewFeatures: withExperimentalPersistentListViewFeatures,
Expand Down Expand Up @@ -155,15 +183,6 @@ function ListViewBlock( {
'has-single-cell': hideBlockActions,
} );

const blockInformation = useBlockDisplayInformation( clientId );
const settingsAriaLabel = blockInformation
? sprintf(
// translators: %s: The title of the block.
__( 'Options for %s block' ),
blockInformation.title
)
: __( 'Options' );

// Only include all selected blocks if the currently clicked on block
// is one of the selected blocks. This ensures that if a user attempts
// to alter a block that isn't part of the selection, they're still able
Expand All @@ -186,11 +205,16 @@ function ListViewBlock( {
id={ `list-view-block-${ clientId }` }
data-block={ clientId }
isExpanded={ isExpanded }
aria-selected={ !! isSelected }
>
<TreeGridCell
className="block-editor-list-view-block__contents-cell"
colSpan={ colSpan }
ref={ cellRef }
aria-label={ blockAriaLabel }
aria-selected={ !! isSelected }
aria-expanded={ isExpanded }
aria-describedby={ descriptionId }
>
{ ( { ref, tabIndex, onFocus } ) => (
<div className="block-editor-list-view-block__contents-container">
Expand All @@ -207,7 +231,14 @@ function ListViewBlock( {
onFocus={ onFocus }
isExpanded={ isExpanded }
selectedClientIds={ selectedClientIds }
preventAnnouncement={ preventAnnouncement }
/>
<div
className="block-editor-list-view-block-select-button__description"
id={ descriptionId }
>
{ blockPositionDescription }
</div>
</div>
) }
</TreeGridCell>
Expand Down Expand Up @@ -244,7 +275,10 @@ function ListViewBlock( {
) }

{ showBlockActions && (
<TreeGridCell className={ listViewBlockSettingsClassName }>
<TreeGridCell
className={ listViewBlockSettingsClassName }
aria-selected={ !! isSelected }
>
{ ( { ref, tabIndex, onFocus } ) => (
<BlockSettingsDropdown
clientIds={ dropdownClientIds }
Expand Down
17 changes: 15 additions & 2 deletions packages/block-editor/src/components/list-view/use-block-selection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { speak } from '@wordpress/a11y';
import { __, sprintf } from '@wordpress/i18n';
import { useDispatch, useSelect } from '@wordpress/data';
import { useCallback } from '@wordpress/element';
import { UP, DOWN } from '@wordpress/keycodes';
import { UP, DOWN, HOME, END } from '@wordpress/keycodes';
import { store as blocksStore } from '@wordpress/blocks';

/**
Expand Down Expand Up @@ -49,7 +49,10 @@ export default function useBlockSelection() {

const isKeyPress =
event.type === 'keydown' &&
( event.keyCode === UP || event.keyCode === DOWN );
( event.keyCode === UP ||
event.keyCode === DOWN ||
event.keyCode === HOME ||
event.keyCode === END );

// Handle clicking on a block when no blocks are selected, and return early.
if (
Expand Down Expand Up @@ -114,6 +117,16 @@ export default function useBlockSelection() {
// the total number of blocks deselected is greater than one.
const updatedSelectedBlocks = getSelectedBlockClientIds();

// If the selection is greater than 1 and the Home or End keys
// were used to generate the selection, then skip announcing the
// deselected blocks.
if (
( event.keyCode === HOME || event.keyCode === END ) &&
updatedSelectedBlocks.length > 1
) {
return;
}
andrewserong marked this conversation as resolved.
Show resolved Hide resolved

Copy link
Contributor

Choose a reason for hiding this comment

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

In testing on both Windows with NVDA+Firefox and macOS with VoiceOver+Firefox, this change doesn't seem to make any difference. The last selected block of the multi-selection is always announced, whether the multi-selection happens with Home/End keys or just Shift+Arrow Up/Down. And if for @alexstine the change seemed to make things worse, I think it's safe to remove it altogether.

Copy link
Contributor

Choose a reason for hiding this comment

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

This change is good when selecting with Arrows but when using Home/End, it should only announce the number of selected/deselected. Why? Let's look at this practically.

  • I press Shift+Down Arrow to select a block.
  • I hear the total number of blocks selected plus the block I just selected.
  • If I press End, it will select say 5 blocks but then I only hear the last block get announced.

See how this can get confusing?

When having these Home/End selections, it should just announce the total as in reality you selected multiple blocks at once vs. one block at a time.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see what you mean. When pressing End, the last block in the selection gets focused; that must be why it's being announced. I wonder if we can instead keep focus on the first block in the selection - and if there won't be any negative side-effects to that 😅

Copy link
Contributor Author

@andrewserong andrewserong Mar 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for taking a closer look here! I was wondering if it's possible for us to conditionally add aria-hidden set to true in the block select button component for just the case of having switched focus while holding shift plus Home / End, to prevent the block from being announced. I might have a play.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think focus should always follow the selection. Otherwise when you press up or down again it becomes disorienting.

Looking at how Finder works, it's similar to what Alex describes (though you have to use shift+option+arrow key instead of home or end to select to the start or end). It'll only announce '6 rows selected' and it won't announce the file that is focused (even though focus does move).

Is there any way to suppress the innate announcement that screenreaders make when focus moves?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was also wondering if there's any advantage in using roles like aria-multiselectable:
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-multiselectable

It didn't seem to do anything when I tested adding it, but maybe it needs to be used in combination with aria-selected.

The docs seem to imply that's ok with a tree, but don't say if that extends to a treegrid.

const selectionDiff = difference(
selectedBlocks,
updatedSelectedBlocks
Expand Down
44 changes: 25 additions & 19 deletions packages/e2e-tests/specs/editor/various/list-view.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ describe( 'List view', () => {
await pressKeyWithModifier( 'access', 'o' );

// The last inserted paragraph block should be selected in List View.
await page.waitForXPath(
'//a[contains(., "Paragraph(selected block)")]'
await page.waitForSelector(
'.block-editor-list-view-block__contents-cell[aria-selected="true"][aria-label^="Paragraph"]'
);

// Go to the image block in list view.
Expand Down Expand Up @@ -115,8 +115,8 @@ describe( 'List view', () => {
await openListView();

// The last inserted paragraph block should be selected in List View.
await page.waitForXPath(
'//a[contains(., "Paragraph(selected block)")]'
await page.waitForSelector(
'.block-editor-list-view-block__contents-cell[aria-selected="true"][aria-label^="Paragraph"]'
);

// Paragraph options button.
Expand All @@ -134,8 +134,8 @@ describe( 'List view', () => {
await paragraphRemoveButton.click();

// Heading block should be selected as previous block.
await page.waitForXPath(
'//a[contains(., "Heading(selected block)")]'
await page.waitForSelector(
'.block-editor-list-view-block__contents-cell[aria-selected="true"][aria-label^="Heading"]'
);
} );

Expand All @@ -150,16 +150,18 @@ describe( 'List view', () => {
await openListView();

// The last inserted paragraph block should be selected in List View.
await page.waitForXPath(
'//a[contains(., "Paragraph(selected block)")]'
await page.waitForSelector(
'.block-editor-list-view-block__contents-cell[aria-selected="true"][aria-label^="Paragraph"]'
);

// Go to the image block in list view.
await pressKeyTimes( 'ArrowUp', 2 );
await pressKeyTimes( 'Enter', 1 );

// Image block should have selected.
await page.waitForXPath( '//a[contains(., "Image(selected block)")]' );
await page.waitForSelector(
'.block-editor-list-view-block__contents-cell[aria-selected="true"][aria-label^="Image"]'
);

// Image options dropdown.
const imageOptionsButton = await page.waitForSelector(
Expand All @@ -176,8 +178,8 @@ describe( 'List view', () => {
await imageRemoveButton.click();

// Heading block should be selected as next block.
await page.waitForXPath(
'//a[contains(., "Heading(selected block)")]'
await page.waitForSelector(
'.block-editor-list-view-block__contents-cell[aria-selected="true"][aria-label^="Heading"]'
);
} );

Expand All @@ -194,8 +196,8 @@ describe( 'List view', () => {
await openListView();

// The last inserted heading block should be selected in List View.
const headingBlock = await page.waitForXPath(
'//a[contains(., "Heading(selected block)")]'
const headingBlock = await page.waitForSelector(
'.block-editor-list-view-block__contents-cell[aria-selected="true"][aria-label^="Heading"]'
);

await headingBlock.click();
Expand All @@ -204,10 +206,12 @@ describe( 'List view', () => {
await pressKeyWithModifier( 'shift', 'ArrowUp' );

// Both Image and Heading blocks should have selected.
await page.waitForXPath(
'//a[contains(., "Heading(selected block)")]'
await page.waitForSelector(
'.block-editor-list-view-block__contents-cell[aria-selected="true"][aria-label^="Heading"]'
);
await page.waitForSelector(
'.block-editor-list-view-block__contents-cell[aria-selected="true"][aria-label^="Image"]'
);
await page.waitForXPath( '//a[contains(., "Image(selected block)")]' );

const imageOptionsButton = await page.waitForSelector(
'tr.block-editor-list-view-leaf:first-child button[aria-label="Options for Image block"]'
Expand All @@ -224,8 +228,8 @@ describe( 'List view', () => {
await blocksRemoveButton.click();

// Newly created default paragraph block should be selected.
await page.waitForXPath(
'//a[contains(., "Paragraph(selected block)")]'
await page.waitForSelector(
'.block-editor-list-view-block__contents-cell[aria-selected="true"][aria-label^="Paragraph"]'
);
} );

Expand Down Expand Up @@ -289,7 +293,9 @@ describe( 'List view', () => {
await pressKeyWithModifier( 'access', 'o' );

// The last inserted group block should be selected in list view.
await page.waitForXPath( '//a[contains(., "Group(selected block)")]' );
await page.waitForSelector(
'.block-editor-list-view-block__contents-cell[aria-selected="true"][aria-label^="Group"]'
);

// Press Home to go to the first inserted block (image).
await page.keyboard.press( 'Home' );
Expand Down