From 00345103b38c36e735c96ef14876f28d03ada504 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Sun, 26 Feb 2023 14:19:28 -0600 Subject: [PATCH 01/19] Fix ARIA attributes. --- .../list-view/block-select-button.js | 7 ++++- .../src/components/list-view/block.js | 30 +++++++------------ 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/packages/block-editor/src/components/list-view/block-select-button.js b/packages/block-editor/src/components/list-view/block-select-button.js index 39baf78d5a7a88..b912e3a7f62c69 100644 --- a/packages/block-editor/src/components/list-view/block-select-button.js +++ b/packages/block-editor/src/components/list-view/block-select-button.js @@ -35,6 +35,9 @@ function ListViewBlockSelectButton( onDragStart, onDragEnd, draggable, + ariaLabel, + ariaExpanded, + ariaDescribedBy, }, ref ) { @@ -76,7 +79,9 @@ function ListViewBlockSelectButton( onDragEnd={ onDragEnd } draggable={ draggable } href={ `#block-${ clientId }` } - aria-hidden={ true } + aria-label={ ariaLabel } + aria-expanded={ ariaExpanded } + aria-describedby={ ariaDescribedBy } > diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js index bf024b1fb758bf..9a06e5bfe752b4 100644 --- a/packages/block-editor/src/components/list-view/block.js +++ b/packages/block-editor/src/components/list-view/block.js @@ -54,7 +54,6 @@ function ListViewBlock( { path, isExpanded, selectedClientIds, - preventAnnouncement, isSyncedBranch, } ) { const cellRef = useRef( null ); @@ -112,20 +111,13 @@ function ListViewBlock( { level ); - let blockAriaLabel = __( 'Link' ); - if ( blockInformation ) { - blockAriaLabel = isLocked - ? sprintf( - // translators: %s: The title of the block. This string indicates a link to select the locked block. - __( '%s link (locked)' ), - blockInformation.title - ) - : sprintf( - // translators: %s: The title of the block. This string indicates a link to select the block. - __( '%s link' ), - blockInformation.title - ); - } + const blockAriaLabel = isLocked + ? sprintf( + // translators: %s: The title of the block. This string indicates a link to select the locked block. + __( '%s (locked)' ), + blockInformation.title + ) + : blockInformation.title; const settingsAriaLabel = blockInformation ? sprintf( @@ -245,17 +237,13 @@ function ListViewBlock( { id={ `list-view-block-${ clientId }` } data-block={ clientId } isExpanded={ canExpand ? isExpanded : undefined } - aria-selected={ !! isSelected || forceSelectionContentLock } ref={ rowRef } > { ( { ref, tabIndex, onFocus } ) => (
@@ -272,7 +260,9 @@ function ListViewBlock( { onFocus={ onFocus } isExpanded={ isExpanded } selectedClientIds={ selectedClientIds } - preventAnnouncement={ preventAnnouncement } + ariaLabel={ blockAriaLabel } + ariaExpanded={ canExpand ? isExpanded : undefined } + ariaDescribedBy={ descriptionId } />
Date: Sun, 26 Feb 2023 14:20:18 -0600 Subject: [PATCH 02/19] Add assertive for selection read. --- .../src/components/list-view/use-block-selection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/list-view/use-block-selection.js b/packages/block-editor/src/components/list-view/use-block-selection.js index 59aaaeacb01d40..4d410d6d393f2f 100644 --- a/packages/block-editor/src/components/list-view/use-block-selection.js +++ b/packages/block-editor/src/components/list-view/use-block-selection.js @@ -145,7 +145,7 @@ export default function useBlockSelection() { } if ( label ) { - speak( label ); + speak( label, 'assertive' ); } }, [ From 55f43ac49379283e7fe77a7d6ab93387d359eb32 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 3 Apr 2023 19:37:04 -0500 Subject: [PATCH 03/19] Remove useless useEffect. Refactor expanded to use isExpanded attribute. --- .../list-view/block-select-button.js | 4 ++-- .../src/components/list-view/block.js | 23 +++---------------- 2 files changed, 5 insertions(+), 22 deletions(-) diff --git a/packages/block-editor/src/components/list-view/block-select-button.js b/packages/block-editor/src/components/list-view/block-select-button.js index b912e3a7f62c69..21759d205845aa 100644 --- a/packages/block-editor/src/components/list-view/block-select-button.js +++ b/packages/block-editor/src/components/list-view/block-select-button.js @@ -35,8 +35,8 @@ function ListViewBlockSelectButton( onDragStart, onDragEnd, draggable, + isExpanded, ariaLabel, - ariaExpanded, ariaDescribedBy, }, ref @@ -80,8 +80,8 @@ function ListViewBlockSelectButton( draggable={ draggable } href={ `#block-${ clientId }` } aria-label={ ariaLabel } - aria-expanded={ ariaExpanded } aria-describedby={ ariaDescribedBy } + aria-expanded={ isExpanded } > diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js index f06b047a5d2152..903073e10075ec 100644 --- a/packages/block-editor/src/components/list-view/block.js +++ b/packages/block-editor/src/components/list-view/block.js @@ -13,13 +13,7 @@ import { } from '@wordpress/components'; import { useInstanceId } from '@wordpress/compose'; import { moreVertical } from '@wordpress/icons'; -import { - useState, - useRef, - useEffect, - useCallback, - memo, -} from '@wordpress/element'; +import { useState, useRef, useCallback, memo } from '@wordpress/element'; import { useDispatch, useSelect } from '@wordpress/data'; import { sprintf, __ } from '@wordpress/i18n'; @@ -126,8 +120,7 @@ function ListViewBlock( { ) : __( 'Options' ); - const { isTreeGridMounted, expand, collapse, BlockSettingsMenu } = - useListViewContext(); + const { expand, collapse, BlockSettingsMenu } = useListViewContext(); const hasSiblings = siblingBlockCount > 0; const hasRenderedMovers = showBlockMovers && hasSiblings; @@ -141,15 +134,6 @@ function ListViewBlock( { { 'is-visible': isHovered || isFirstSelectedBlock } ); - // If ListView has experimental features related to the Persistent List View, - // only focus the selected list item on mount; otherwise the list would always - // try to steal the focus from the editor canvas. - useEffect( () => { - if ( ! isTreeGridMounted && isSelected ) { - cellRef.current.focus(); - } - }, [] ); - const onMouseEnter = useCallback( () => { setIsHovered( true ); toggleBlockHighlight( clientId, true ); @@ -265,10 +249,9 @@ function ListViewBlock( { currentlyEditingBlockInCanvas ? 0 : tabIndex } onFocus={ onFocus } - isExpanded={ isExpanded } + isExpanded={ canExpand ? isExpanded : undefined } selectedClientIds={ selectedClientIds } ariaLabel={ blockAriaLabel } - ariaExpanded={ canExpand ? isExpanded : undefined } ariaDescribedBy={ descriptionId } />
Date: Mon, 3 Apr 2023 20:31:14 -0500 Subject: [PATCH 04/19] Add prop to disable aria-expanded on tree grid tr so I can override on the link. --- packages/block-editor/src/components/list-view/leaf.js | 1 + packages/components/src/tree-grid/index.tsx | 9 +++++++-- packages/components/src/tree-grid/row.tsx | 3 ++- packages/components/src/tree-grid/types.ts | 4 ++++ 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/list-view/leaf.js b/packages/block-editor/src/components/list-view/leaf.js index 63c154fd620379..9159d6b0e77cf1 100644 --- a/packages/block-editor/src/components/list-view/leaf.js +++ b/packages/block-editor/src/components/list-view/leaf.js @@ -51,6 +51,7 @@ const ListViewLeaf = forwardRef( level={ level } positionInSet={ position } setSize={ rowCount } + disableAriaExpanded={ true } { ...props } > { children } diff --git a/packages/components/src/tree-grid/index.tsx b/packages/components/src/tree-grid/index.tsx index 7087bafc86f70b..6a258eb9b1e319 100644 --- a/packages/components/src/tree-grid/index.tsx +++ b/packages/components/src/tree-grid/index.tsx @@ -91,7 +91,8 @@ function UnforwardedTreeGrid( const canExpandCollapse = 0 === currentColumnIndex; const cannotFocusNextColumn = canExpandCollapse && - activeRow.getAttribute( 'aria-expanded' ) === 'false' && + ( activeRow.getAttribute( 'data-expanded' ) === 'false' || + activeRow.getAttribute( 'aria-expanded' ) === 'false' ) && keyCode === RIGHT; if ( ( [ LEFT, RIGHT ] as number[] ).includes( keyCode ) ) { @@ -112,6 +113,8 @@ function UnforwardedTreeGrid( // Left: // If a row is focused, and it is expanded, collapses the current row. if ( + activeRow.getAttribute( 'data-expanded' ) === + 'true' || activeRow.getAttribute( 'aria-expanded' ) === 'true' ) { onCollapseRow( activeRow ); @@ -151,8 +154,10 @@ function UnforwardedTreeGrid( // Right: // If a row is focused, and it is collapsed, expands the current row. if ( + activeRow.getAttribute( 'data-expanded' ) === + 'false' || activeRow.getAttribute( 'aria-expanded' ) === - 'false' + 'false' ) { onExpandRow( activeRow ); event.preventDefault(); diff --git a/packages/components/src/tree-grid/row.tsx b/packages/components/src/tree-grid/row.tsx index a4b69cd46e1797..553b3422762957 100644 --- a/packages/components/src/tree-grid/row.tsx +++ b/packages/components/src/tree-grid/row.tsx @@ -16,6 +16,7 @@ function UnforwardedTreeGridRow( positionInSet, setSize, isExpanded, + disableAriaExpanded, ...props }: WordPressComponentProps< TreeGridRowProps, 'tr', false >, ref: React.ForwardedRef< HTMLTableRowElement > @@ -28,7 +29,7 @@ function UnforwardedTreeGridRow( aria-level={ level } aria-posinset={ positionInSet } aria-setsize={ setSize } - aria-expanded={ isExpanded } + aria-expanded={ disableAriaExpanded ? undefined : isExpanded } > { children } diff --git a/packages/components/src/tree-grid/types.ts b/packages/components/src/tree-grid/types.ts index 47c2739f43dc7c..2f5d7e4a39c8f8 100644 --- a/packages/components/src/tree-grid/types.ts +++ b/packages/components/src/tree-grid/types.ts @@ -24,6 +24,10 @@ export type TreeGridRowProps = { * it has no other built-in behavior. */ isExpanded?: boolean; + /** + * An optional value that designates whether a row should have aria-expanded attribute or if this should be managed in the parent component. + */ + disableAriaExpanded?: boolean; }; type RovingTabIndexItemPassThruProps = { From 4c30383e0a16f612483c7bdce6e0fed201d82120 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Tue, 4 Apr 2023 13:54:06 +1000 Subject: [PATCH 05/19] Try to fix failing e2e test --- .../specs/editor/various/multi-block-selection.spec.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/e2e/specs/editor/various/multi-block-selection.spec.js b/test/e2e/specs/editor/various/multi-block-selection.spec.js index 3e03a0c2805620..d72ab2c6bf7dbf 100644 --- a/test/e2e/specs/editor/various/multi-block-selection.spec.js +++ b/test/e2e/specs/editor/various/multi-block-selection.spec.js @@ -916,8 +916,8 @@ test.describe( 'Multi-block selection', () => { const listView = page.getByRole( 'treegrid', { name: 'Block navigation structure', } ); - const navButtons = listView.getByRole( 'gridcell', { - name: 'Paragraph link', + const navButtons = listView.getByRole( 'link', { + name: 'Paragraph', } ); await navButtons.nth( 1 ).click(); @@ -954,9 +954,7 @@ test.describe( 'Multi-block selection', () => { // Move focus to the list view link to prepare for the keyboard navigation. await navButtons.nth( 3 ).click(); - await expect( - navButtons.nth( 3 ).getByRole( 'link', { includeHidden: true } ) - ).toBeFocused(); + await expect( navButtons.nth( 3 ) ).toBeFocused(); // Press Up twice to highlight the second block. await pageUtils.pressKeys( 'ArrowUp', { times: 2 } ); From 4c2b450dbc16c80d44bf65c834c01be0412c3149 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Tue, 4 Apr 2023 15:51:15 +1000 Subject: [PATCH 06/19] Update list view tests --- .../specs/editor/various/list-view.spec.js | 114 +++++++++--------- 1 file changed, 55 insertions(+), 59 deletions(-) diff --git a/test/e2e/specs/editor/various/list-view.spec.js b/test/e2e/specs/editor/various/list-view.spec.js index 906159c944d1e7..f9a1770437a4ce 100644 --- a/test/e2e/specs/editor/various/list-view.spec.js +++ b/test/e2e/specs/editor/various/list-view.spec.js @@ -27,7 +27,8 @@ test.describe( 'List View', () => { // The last inserted block should be selected. await expect( listView.getByRole( 'gridcell', { - name: 'Paragraph link', + name: 'Paragraph', + exact: true, selected: true, } ) ).toBeVisible(); @@ -43,10 +44,12 @@ test.describe( 'List View', () => { // Drag the paragraph above the heading. const paragraphBlockItem = listView.getByRole( 'gridcell', { - name: 'Paragraph link', + name: 'Paragraph', + exact: true, } ); const headingBlockItem = listView.getByRole( 'gridcell', { - name: 'Heading link', + name: 'Heading', + exact: true, } ); await paragraphBlockItem.dragTo( headingBlockItem, { x: 0, y: 0 } ); @@ -80,7 +83,8 @@ test.describe( 'List View', () => { // The last inserted block should be selected. await expect( listView.getByRole( 'gridcell', { - name: 'Paragraph link', + name: 'Paragraph', + exact: true, selected: true, } ) ).toBeVisible(); @@ -88,11 +92,9 @@ test.describe( 'List View', () => { // Go to the image block in List View. await pageUtils.pressKeys( 'ArrowUp', { times: 2 } ); await expect( - listView - .getByRole( 'gridcell', { - name: 'Image link', - } ) - .getByRole( 'link', { includeHidden: true } ) + listView.getByRole( 'link', { + name: 'Image', + } ) ).toBeFocused(); // Select the image block in the canvas. @@ -110,9 +112,7 @@ test.describe( 'List View', () => { await page.keyboard.press( 'Backspace' ); // List View should have two rows. - await expect( - listView.getByRole( 'gridcell', { name: /link/i } ) - ).toHaveCount( 2 ); + await expect( listView.getByRole( 'row' ) ).toHaveCount( 2 ); } ); // Check for regression of https://github.com/WordPress/gutenberg/issues/39026. @@ -135,7 +135,8 @@ test.describe( 'List View', () => { // The last inserted block should be selected. await expect( listView.getByRole( 'gridcell', { - name: 'Paragraph link', + name: 'Paragraph', + exact: true, selected: true, } ) ).toBeVisible(); @@ -176,7 +177,8 @@ test.describe( 'List View', () => { // The last inserted block should be selected. await expect( listView.getByRole( 'gridcell', { - name: 'Paragraph link', + name: 'Paragraph', + exact: true, selected: true, } ) ).toBeVisible(); @@ -184,11 +186,9 @@ test.describe( 'List View', () => { // Select the image block in List View. await pageUtils.pressKeys( 'ArrowUp', { times: 2 } ); await expect( - listView - .getByRole( 'gridcell', { - name: 'Image link', - } ) - .getByRole( 'link', { includeHidden: true } ) + listView.getByRole( 'link', { + name: 'Image', + } ) ).toBeFocused(); await page.keyboard.press( 'Enter' ); @@ -229,7 +229,8 @@ test.describe( 'List View', () => { // The last inserted block should be selected. await expect( listView.getByRole( 'gridcell', { - name: 'Heading link', + name: 'Heading', + exact: true, selected: true, } ) ).toBeVisible(); @@ -238,7 +239,8 @@ test.describe( 'List View', () => { await pageUtils.pressKeys( 'shift+ArrowUp' ); await expect( listView.getByRole( 'gridcell', { - name: 'Image link', + name: 'Image', + exact: true, selected: true, } ) ).toBeVisible(); @@ -278,8 +280,8 @@ test.describe( 'List View', () => { // Things start off expanded. await expect( - listView.getByRole( 'gridcell', { - name: 'Cover link', + listView.getByRole( 'link', { + name: 'Cover', expanded: true, } ) ).toBeVisible(); @@ -287,28 +289,27 @@ test.describe( 'List View', () => { // The child paragraph block should be selected. await expect( listView.getByRole( 'gridcell', { - name: 'Paragraph link', + name: 'Paragraph', + exact: true, selected: true, } ) ).toBeVisible(); // Collapse the Cover block. await listView - .getByRole( 'gridcell', { name: 'Cover link' } ) + .getByRole( 'gridcell', { name: 'Cover', exact: true } ) .getByTestId( 'list-view-expander', { includeHidden: true } ) // Force the click to bypass the visibility check. The expander is // intentionally aria-hidden. See the implementation for details. .click( { force: true } ); // Check that we're collapsed. - await expect( - listView.getByRole( 'gridcell', { name: /link/i } ) - ).toHaveCount( 1 ); + await expect( listView.getByRole( 'row' ) ).toHaveCount( 1 ); // Click the Cover block List View item. await listView - .getByRole( 'gridcell', { - name: 'Cover link', + .getByRole( 'link', { + name: 'Cover', expanded: false, } ) .click(); @@ -322,7 +323,8 @@ test.describe( 'List View', () => { // The child paragraph block in List View should be selected. await expect( listView.getByRole( 'gridcell', { - name: 'Paragraph link', + name: 'Paragraph', + exact: true, selected: true, } ) ).toBeVisible(); @@ -349,7 +351,8 @@ test.describe( 'List View', () => { // The last inserted block should be selected. await expect( listView.getByRole( 'gridcell', { - name: 'Group link', + name: 'Group', + exact: true, selected: true, } ) ).toBeVisible(); @@ -357,22 +360,19 @@ test.describe( 'List View', () => { // Press Home to go to the first inserted block (image). await page.keyboard.press( 'Home' ); await expect( - listView - .getByRole( 'gridcell', { - name: 'Image link', - } ) - .getByRole( 'link', { includeHidden: true } ) + listView.getByRole( 'link', { + name: 'Image', + } ) ).toBeFocused(); // Press End followed by Arrow Up to go to the second to last block (columns). await page.keyboard.press( 'End' ); await page.keyboard.press( 'ArrowUp' ); await expect( - listView - .getByRole( 'gridcell', { - name: 'Columns link', - } ) - .getByRole( 'link', { includeHidden: true } ) + listView.getByRole( 'link', { + name: 'Columns', + exact: true, + } ) ).toBeFocused(); // Navigate the right column to image block options button via Home key. @@ -417,18 +417,17 @@ test.describe( 'List View', () => { // The paragraph item should be selected. await expect( listView.getByRole( 'gridcell', { - name: 'Paragraph link', + name: 'Paragraph', + exact: true, selected: true, } ) ).toBeVisible(); // Navigate to the image block item. await page.keyboard.press( 'ArrowUp' ); - const imageItem = listView - .getByRole( 'gridcell', { - name: 'Image link', - } ) - .getByRole( 'link', { includeHidden: true } ); + const imageItem = listView.getByRole( 'link', { + name: 'Image', + } ); await expect( imageItem ).toBeFocused(); @@ -527,7 +526,8 @@ test.describe( 'List View', () => { // The last inserted block should be selected. await expect( listView.getByRole( 'gridcell', { - name: 'Paragraph link', + name: 'Paragraph', + exact: true, selected: true, } ) ).toBeVisible(); @@ -535,11 +535,9 @@ test.describe( 'List View', () => { // Go to the image block in List View. await pageUtils.pressKeys( 'ArrowUp', { times: 2 } ); await expect( - listView - .getByRole( 'gridcell', { - name: 'Image link', - } ) - .getByRole( 'link', { includeHidden: true } ) + listView.getByRole( 'link', { + name: 'Image', + } ) ).toBeFocused(); // Select the image block in the canvas. @@ -554,11 +552,9 @@ test.describe( 'List View', () => { // Triggering the List View shortcut should result in the image block gaining focus. await pageUtils.pressKeys( 'access+o' ); await expect( - listView - .getByRole( 'gridcell', { - name: 'Image link', - } ) - .getByRole( 'link', { includeHidden: true } ) + listView.getByRole( 'link', { + name: 'Image', + } ) ).toBeFocused(); } ); } ); From 08967aeafad8389645ccece72cf6ca8989617977 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 4 Apr 2023 10:01:23 -0500 Subject: [PATCH 07/19] Restore removed useEffect. --- .../src/components/list-view/block.js | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js index 903073e10075ec..9bf0c58107f4eb 100644 --- a/packages/block-editor/src/components/list-view/block.js +++ b/packages/block-editor/src/components/list-view/block.js @@ -13,7 +13,13 @@ import { } from '@wordpress/components'; import { useInstanceId } from '@wordpress/compose'; import { moreVertical } from '@wordpress/icons'; -import { useState, useRef, useCallback, memo } from '@wordpress/element'; +import { + useState, + useRef, + useEffect, + useCallback, + memo, +} from '@wordpress/element'; import { useDispatch, useSelect } from '@wordpress/data'; import { sprintf, __ } from '@wordpress/i18n'; @@ -120,7 +126,8 @@ function ListViewBlock( { ) : __( 'Options' ); - const { expand, collapse, BlockSettingsMenu } = useListViewContext(); + const { isTreeGridMounted, expand, collapse, BlockSettingsMenu } = + useListViewContext(); const hasSiblings = siblingBlockCount > 0; const hasRenderedMovers = showBlockMovers && hasSiblings; @@ -134,6 +141,15 @@ function ListViewBlock( { { 'is-visible': isHovered || isFirstSelectedBlock } ); + // If ListView has experimental features related to the Persistent List View, + // only focus the selected list item on mount; otherwise the list would always + // try to steal the focus from the editor canvas. + useEffect( () => { + if ( ! isTreeGridMounted && isSelected ) { + cellRef.current.focus(); + } + }, [] ); + const onMouseEnter = useCallback( () => { setIsHovered( true ); toggleBlockHighlight( clientId, true ); From 54797d5a82418610c44d03d8094c1b92b0d5ddc8 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 4 Apr 2023 10:04:25 -0500 Subject: [PATCH 08/19] Revert TreeGrid changes. --- packages/components/src/tree-grid/index.tsx | 9 ++------- packages/components/src/tree-grid/row.tsx | 3 +-- packages/components/src/tree-grid/types.ts | 4 ---- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/packages/components/src/tree-grid/index.tsx b/packages/components/src/tree-grid/index.tsx index 6a258eb9b1e319..7087bafc86f70b 100644 --- a/packages/components/src/tree-grid/index.tsx +++ b/packages/components/src/tree-grid/index.tsx @@ -91,8 +91,7 @@ function UnforwardedTreeGrid( const canExpandCollapse = 0 === currentColumnIndex; const cannotFocusNextColumn = canExpandCollapse && - ( activeRow.getAttribute( 'data-expanded' ) === 'false' || - activeRow.getAttribute( 'aria-expanded' ) === 'false' ) && + activeRow.getAttribute( 'aria-expanded' ) === 'false' && keyCode === RIGHT; if ( ( [ LEFT, RIGHT ] as number[] ).includes( keyCode ) ) { @@ -113,8 +112,6 @@ function UnforwardedTreeGrid( // Left: // If a row is focused, and it is expanded, collapses the current row. if ( - activeRow.getAttribute( 'data-expanded' ) === - 'true' || activeRow.getAttribute( 'aria-expanded' ) === 'true' ) { onCollapseRow( activeRow ); @@ -154,10 +151,8 @@ function UnforwardedTreeGrid( // Right: // If a row is focused, and it is collapsed, expands the current row. if ( - activeRow.getAttribute( 'data-expanded' ) === - 'false' || activeRow.getAttribute( 'aria-expanded' ) === - 'false' + 'false' ) { onExpandRow( activeRow ); event.preventDefault(); diff --git a/packages/components/src/tree-grid/row.tsx b/packages/components/src/tree-grid/row.tsx index 553b3422762957..a4b69cd46e1797 100644 --- a/packages/components/src/tree-grid/row.tsx +++ b/packages/components/src/tree-grid/row.tsx @@ -16,7 +16,6 @@ function UnforwardedTreeGridRow( positionInSet, setSize, isExpanded, - disableAriaExpanded, ...props }: WordPressComponentProps< TreeGridRowProps, 'tr', false >, ref: React.ForwardedRef< HTMLTableRowElement > @@ -29,7 +28,7 @@ function UnforwardedTreeGridRow( aria-level={ level } aria-posinset={ positionInSet } aria-setsize={ setSize } - aria-expanded={ disableAriaExpanded ? undefined : isExpanded } + aria-expanded={ isExpanded } > { children } diff --git a/packages/components/src/tree-grid/types.ts b/packages/components/src/tree-grid/types.ts index 2f5d7e4a39c8f8..47c2739f43dc7c 100644 --- a/packages/components/src/tree-grid/types.ts +++ b/packages/components/src/tree-grid/types.ts @@ -24,10 +24,6 @@ export type TreeGridRowProps = { * it has no other built-in behavior. */ isExpanded?: boolean; - /** - * An optional value that designates whether a row should have aria-expanded attribute or if this should be managed in the parent component. - */ - disableAriaExpanded?: boolean; }; type RovingTabIndexItemPassThruProps = { From 0833362856a12e7a55ed70edb17d3bf8c0b759b3 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 4 Apr 2023 10:07:13 -0500 Subject: [PATCH 09/19] Finish revert. --- packages/block-editor/src/components/list-view/leaf.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/block-editor/src/components/list-view/leaf.js b/packages/block-editor/src/components/list-view/leaf.js index 9159d6b0e77cf1..63c154fd620379 100644 --- a/packages/block-editor/src/components/list-view/leaf.js +++ b/packages/block-editor/src/components/list-view/leaf.js @@ -51,7 +51,6 @@ const ListViewLeaf = forwardRef( level={ level } positionInSet={ position } setSize={ rowCount } - disableAriaExpanded={ true } { ...props } > { children } From 115cfdc05e11c669ebe6d93da0b31d61949852c5 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 4 Apr 2023 10:17:42 -0500 Subject: [PATCH 10/19] Eliminate aria-expanded on main tr. --- packages/block-editor/src/components/list-view/block.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js index 9bf0c58107f4eb..840a3b9845dbde 100644 --- a/packages/block-editor/src/components/list-view/block.js +++ b/packages/block-editor/src/components/list-view/block.js @@ -241,7 +241,7 @@ function ListViewBlock( { id={ `list-view-block-${ clientId }` } data-block={ clientId } data-expanded={ canExpand ? isExpanded : undefined } - isExpanded={ canExpand ? isExpanded : undefined } + isExpanded={ undefined } ref={ rowRef } > Date: Tue, 4 Apr 2023 10:27:06 -0500 Subject: [PATCH 11/19] Add data-expanded back to TreeGrid. Need to detect when to expand via keyboard. --- packages/components/src/tree-grid/index.tsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/components/src/tree-grid/index.tsx b/packages/components/src/tree-grid/index.tsx index 7087bafc86f70b..6a258eb9b1e319 100644 --- a/packages/components/src/tree-grid/index.tsx +++ b/packages/components/src/tree-grid/index.tsx @@ -91,7 +91,8 @@ function UnforwardedTreeGrid( const canExpandCollapse = 0 === currentColumnIndex; const cannotFocusNextColumn = canExpandCollapse && - activeRow.getAttribute( 'aria-expanded' ) === 'false' && + ( activeRow.getAttribute( 'data-expanded' ) === 'false' || + activeRow.getAttribute( 'aria-expanded' ) === 'false' ) && keyCode === RIGHT; if ( ( [ LEFT, RIGHT ] as number[] ).includes( keyCode ) ) { @@ -112,6 +113,8 @@ function UnforwardedTreeGrid( // Left: // If a row is focused, and it is expanded, collapses the current row. if ( + activeRow.getAttribute( 'data-expanded' ) === + 'true' || activeRow.getAttribute( 'aria-expanded' ) === 'true' ) { onCollapseRow( activeRow ); @@ -151,8 +154,10 @@ function UnforwardedTreeGrid( // Right: // If a row is focused, and it is collapsed, expands the current row. if ( + activeRow.getAttribute( 'data-expanded' ) === + 'false' || activeRow.getAttribute( 'aria-expanded' ) === - 'false' + 'false' ) { onExpandRow( activeRow ); event.preventDefault(); From b56270b1efbdd87ab7c81577ea15e8b28c2c1d95 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 4 Apr 2023 21:53:28 -0500 Subject: [PATCH 12/19] Fix expander. --- packages/block-editor/src/components/list-view/style.scss | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/list-view/style.scss b/packages/block-editor/src/components/list-view/style.scss index a04572da98e5f3..76ba2647e2996c 100644 --- a/packages/block-editor/src/components/list-view/style.scss +++ b/packages/block-editor/src/components/list-view/style.scss @@ -93,7 +93,7 @@ &.is-branch-selected.is-first-selected td:last-child { border-top-right-radius: $radius-block-ui; } - &[aria-expanded="false"] { + &[data-expanded="false"] { &.is-branch-selected.is-first-selected td:first-child { border-top-left-radius: $radius-block-ui; } @@ -380,7 +380,7 @@ $block-navigation-max-indent: 8; } // Point downwards when open. -.block-editor-list-view-leaf[aria-expanded="true"] .block-editor-list-view__expander svg { +.block-editor-list-view-leaf[data-expanded="true"] .block-editor-list-view__expander svg { visibility: visible; transition: transform 0.2s ease; transform: rotate(90deg); @@ -388,7 +388,7 @@ $block-navigation-max-indent: 8; } // Point rightwards when closed -.block-editor-list-view-leaf[aria-expanded="false"] .block-editor-list-view__expander svg { +.block-editor-list-view-leaf[data-expanded="false"] .block-editor-list-view__expander svg { visibility: visible; transform: rotate(0deg); transition: transform 0.2s ease; From 90f94255f0c246b43192e2098f7875fe55e5c678 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 4 Apr 2023 21:53:50 -0500 Subject: [PATCH 13/19] Move isExpanded undefined to tr directly. --- packages/block-editor/src/components/list-view/block.js | 1 - packages/block-editor/src/components/list-view/leaf.js | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js index 840a3b9845dbde..1883d518f8ecbd 100644 --- a/packages/block-editor/src/components/list-view/block.js +++ b/packages/block-editor/src/components/list-view/block.js @@ -241,7 +241,6 @@ function ListViewBlock( { id={ `list-view-block-${ clientId }` } data-block={ clientId } data-expanded={ canExpand ? isExpanded : undefined } - isExpanded={ undefined } ref={ rowRef } > { children } From eed5466d21cc9c117556edefbc4fcf9e208af490 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 4 Apr 2023 22:57:06 -0500 Subject: [PATCH 14/19] Fix E2E. --- test/e2e/specs/editor/blocks/columns.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/specs/editor/blocks/columns.spec.js b/test/e2e/specs/editor/blocks/columns.spec.js index d220250ab0a93c..0e510b0cfb4b1e 100644 --- a/test/e2e/specs/editor/blocks/columns.spec.js +++ b/test/e2e/specs/editor/blocks/columns.spec.js @@ -26,7 +26,7 @@ test.describe( 'Columns', () => { // block column add await page .locator( - 'role=treegrid[name="Block navigation structure"i] >> role=gridcell[name="Column link"i]' + 'role=treegrid[name="Block navigation structure"i] >> role=gridcell[name="Column"i]' ) .first() .click(); From 17cdc5b52c952af197b10c535862b04fb6f085be Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 5 Apr 2023 14:41:13 +1000 Subject: [PATCH 15/19] Retain dark color for button when expanded --- .../block-editor/src/components/list-view/style.scss | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/block-editor/src/components/list-view/style.scss b/packages/block-editor/src/components/list-view/style.scss index 76ba2647e2996c..3784f3be63eaa1 100644 --- a/packages/block-editor/src/components/list-view/style.scss +++ b/packages/block-editor/src/components/list-view/style.scss @@ -15,6 +15,18 @@ // Use position relative for row animation. position: relative; + .components-button { + // When a row is expanded, retain the dark color. + &[aria-expanded="true"] { + color: $gray-900; + } + + // Ensure that on hover, the admin color is still used. + &:hover { + color: var(--wp-admin-theme-color); + } + } + // The background has to be applied to the td, not tr, or border-radius won't work. &.is-selected td { background: var(--wp-admin-theme-color); From 3c9068c8240445efc388d8a15aa7284a3721a1c1 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Wed, 5 Apr 2023 12:25:06 -0500 Subject: [PATCH 16/19] Add title fallback. --- .../src/components/list-view/block.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js index 1883d518f8ecbd..63e3a9dede7744 100644 --- a/packages/block-editor/src/components/list-view/block.js +++ b/packages/block-editor/src/components/list-view/block.js @@ -89,6 +89,7 @@ function ListViewBlock( { const { toggleBlockHighlight } = useDispatch( blockEditorStore ); const blockInformation = useBlockDisplayInformation( clientId ); + const blockTitle = blockInformation?.title || __( 'Untitled Block' ); const blockName = useSelect( ( select ) => select( blockEditorStore ).getBlockName( clientId ), [ clientId ] @@ -114,17 +115,15 @@ function ListViewBlock( { ? sprintf( // translators: %s: The title of the block. This string indicates a link to select the locked block. __( '%s (locked)' ), - blockInformation.title + blockTitle ) - : blockInformation.title; + : blockTitle; - const settingsAriaLabel = blockInformation - ? sprintf( - // translators: %s: The title of the block. - __( 'Options for %s block' ), - blockInformation.title - ) - : __( 'Options' ); + const settingsAriaLabel = sprintf( + // translators: %s: The title of the block. + __( 'Options for %s block' ), + blockTitle + ); const { isTreeGridMounted, expand, collapse, BlockSettingsMenu } = useListViewContext(); From 714f33164bdb6f62414e28728e6164823f9d94bf Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Wed, 5 Apr 2023 12:31:50 -0500 Subject: [PATCH 17/19] Save some verbosity on block options button. --- packages/block-editor/src/components/list-view/block.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js index 63e3a9dede7744..fb8054bee7986a 100644 --- a/packages/block-editor/src/components/list-view/block.js +++ b/packages/block-editor/src/components/list-view/block.js @@ -89,7 +89,7 @@ function ListViewBlock( { const { toggleBlockHighlight } = useDispatch( blockEditorStore ); const blockInformation = useBlockDisplayInformation( clientId ); - const blockTitle = blockInformation?.title || __( 'Untitled Block' ); + const blockTitle = blockInformation?.title || __( 'Untitled' ); const blockName = useSelect( ( select ) => select( blockEditorStore ).getBlockName( clientId ), [ clientId ] @@ -121,7 +121,7 @@ function ListViewBlock( { const settingsAriaLabel = sprintf( // translators: %s: The title of the block. - __( 'Options for %s block' ), + __( 'Options for %s' ), blockTitle ); From 87b8c8c412ea30a0773b03799bc9a61f60480ed3 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Wed, 5 Apr 2023 12:36:45 -0500 Subject: [PATCH 18/19] Fix E2E. --- test/e2e/specs/editor/various/list-view.spec.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/e2e/specs/editor/various/list-view.spec.js b/test/e2e/specs/editor/various/list-view.spec.js index f9a1770437a4ce..fb0228dd88f8c8 100644 --- a/test/e2e/specs/editor/various/list-view.spec.js +++ b/test/e2e/specs/editor/various/list-view.spec.js @@ -143,7 +143,7 @@ test.describe( 'List View', () => { // Remove the Paragraph block via its options menu in List View. await listView - .getByRole( 'button', { name: 'Options for Paragraph block' } ) + .getByRole( 'button', { name: 'Options for Paragraph' } ) .click(); await page .getByRole( 'menuitem', { name: /Remove Paragraph/i } ) @@ -194,7 +194,7 @@ test.describe( 'List View', () => { // Remove the Image block via its options menu in List View. await listView - .getByRole( 'button', { name: 'Options for Image block' } ) + .getByRole( 'button', { name: 'Options for Image' } ) .click(); await page.getByRole( 'menuitem', { name: /Remove Image/i } ).click(); @@ -247,7 +247,7 @@ test.describe( 'List View', () => { // Remove both blocks. await listView - .getByRole( 'button', { name: 'Options for Image block' } ) + .getByRole( 'button', { name: 'Options for Image' } ) .click(); await page.getByRole( 'menuitem', { name: /Remove blocks/i } ).click(); @@ -379,13 +379,13 @@ test.describe( 'List View', () => { await page.keyboard.press( 'ArrowRight' ); await page.keyboard.press( 'Home' ); await expect( - listView.getByRole( 'button', { name: 'Options for Image block' } ) + listView.getByRole( 'button', { name: 'Options for Image' } ) ).toBeFocused(); // Navigate the right column to group block options button. await page.keyboard.press( 'End' ); await expect( - listView.getByRole( 'button', { name: 'Options for Group block' } ) + listView.getByRole( 'button', { name: 'Options for Group' } ) ).toBeFocused(); } ); From e7a577b6262aa5be478e79e190e3eefe1e47d2e0 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Wed, 5 Apr 2023 12:54:48 -0500 Subject: [PATCH 19/19] Add components changelog entry. --- packages/components/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 725156f85dd713..52082b2a71ad38 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- `TreeGrid`: Modify keyboard navigation code to use a data-expanded attribute if aria-expanded is to be controlled outside of the TreeGrid component ([#48461](https://github.com/WordPress/gutenberg/pull/48461)). + ## 23.7.0 (2023-03-29) - `ImageSizeControl`: Remove the "Image Dimensions" label ([#49414](https://github.com/WordPress/gutenberg/pull/49414)).