List view: Refactor ARIA attributes #48461
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,7 +53,6 @@ function ListViewBlock( { | |
| path, | ||
| isExpanded, | ||
| selectedClientIds, | ||
| isSyncedBranch, | ||
| } ) { | ||
| const cellRef = useRef( null ); | ||
|
|
@@ -90,6 +89,7 @@ function ListViewBlock( { | |
| const { toggleBlockHighlight } = useDispatch( blockEditorStore ); | ||
|
|
||
| const blockInformation = useBlockDisplayInformation( clientId ); | ||
| const blockTitle = blockInformation?.title || __( 'Untitled' ); | ||
| const blockName = useSelect( | ||
| ( select ) => select( blockEditorStore ).getBlockName( clientId ), | ||
| [ clientId ] | ||
|
|
@@ -111,28 +111,19 @@ function ListViewBlock( { | |
| level | ||
| ); | ||
|
|
||
|
There was a problem hiding this comment. I notice that the condition to check if There was a problem hiding this comment. If that is the case, please check the variable directly below this line. It was not included in this conditional and still used There was a problem hiding this comment. In the case of the below There was a problem hiding this comment. I'm not too sure if that check is still needed, though, but because #38775 introduced the check, it'd probaly be good to verify before removing the check. There was a problem hiding this comment. Ah, yeah, I totally missed that sneaky conditional on that line. NVM. |
||
| const blockAriaLabel = isLocked | ||
| ? sprintf( | ||
| // translators: %s: The title of the block. This string indicates a link to select the locked block. | ||
| __( '%s (locked)' ), | ||
| blockTitle | ||
| ) | ||
| : blockTitle; | ||
|
|
||
| const settingsAriaLabel = sprintf( | ||
| // translators: %s: The title of the block. | ||
| __( 'Options for %s' ), | ||
| blockTitle | ||
| ); | ||
|
|
||
| const { isTreeGridMounted, expand, collapse, BlockSettingsMenu } = | ||
| useListViewContext(); | ||
|
|
@@ -249,18 +240,13 @@ function ListViewBlock( { | |
| id={ `list-view-block-${ clientId }` } | ||
| data-block={ clientId } | ||
| data-expanded={ canExpand ? isExpanded : undefined } | ||
andrewserong marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ref={ rowRef } | ||
| > | ||
| <TreeGridCell | ||
| className="block-editor-list-view-block__contents-cell" | ||
| colSpan={ colSpan } | ||
| ref={ cellRef } | ||
| aria-selected={ !! isSelected || forceSelectionContentLock } | ||
andrewserong marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| > | ||
| { ( { ref, tabIndex, onFocus } ) => ( | ||
| <div className="block-editor-list-view-block__contents-container"> | ||
|
|
@@ -277,9 +263,10 @@ function ListViewBlock( { | |
| currentlyEditingBlockInCanvas ? 0 : tabIndex | ||
| } | ||
| onFocus={ onFocus } | ||
| isExpanded={ canExpand ? isExpanded : undefined } | ||
|
There was a problem hiding this comment. Changed to match the others. |
||
| selectedClientIds={ selectedClientIds } | ||
| ariaLabel={ blockAriaLabel } | ||
| ariaDescribedBy={ descriptionId } | ||
| /> | ||
| <div | ||
| className="block-editor-list-view-block-select-button__description" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -91,7 +91,8 @@ function UnforwardedTreeGrid( | |
| const canExpandCollapse = 0 === currentColumnIndex; | ||
| const cannotFocusNextColumn = | ||
| canExpandCollapse && | ||
| ( activeRow.getAttribute( 'data-expanded' ) === 'false' || | ||
|
There was a problem hiding this comment. Had to add this attribute check here or else we'll never know if it is okay to expand or collapse with the keyboard. Should probably document an example in README. Likely need to update unit tests. There was a problem hiding this comment. Hey @alexstine , would you be able to work on a follow-up where you add info to the README, or (even better) add a Storybook example? There was a problem hiding this comment. @ciampo Storybook is tricky for me to use so doubt I would be much help making an example that might not be testable by me. I only use Storybook when stuff is not testable any other way because accessibility is not great and components used in-practice are much easier to test. For the README example, let me open a follow-up. That should be easy enough. There was a problem hiding this comment. Sounds reasonable, thank you! I'll see if I can help and add the Storybook example myself |
||
| 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' | ||
| ) { | ||
| onExpandRow( activeRow ); | ||
| event.preventDefault(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why this component was no longer receiving this prop.