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

Jump to bottom

[Dataviews] Table layout: Ensure focus is not lost on interaction #57340

Merged
merged 14 commits into from
Jan 5, 2024
Merged

[Dataviews] Table layout: Ensure focus is not lost on interaction #57340

Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
14030cf
Ensuring focus is persisted for table view actions
andrewhayward Dec 22, 2023
5cce5a3
Merge branch 'trunk' into dataviews/table-view-focus-persistence
andrewhayward Dec 22, 2023
37681a9
Adding extra delay to focus header menu fallback
andrewhayward Dec 22, 2023
38fc7e4
Undoing merge artefact
andrewhayward Dec 22, 2023
adc78cb
Simplifying `onHide` call
andrewhayward Jan 3, 2024
42c8eb9
Using `Map` for menus refs
andrewhayward Jan 3, 2024
0c24f6c
Ensuring fallback always has an index
andrewhayward Jan 3, 2024
1df86fe
Simplifying `aria-busy` logic
andrewhayward Jan 3, 2024
0815fe3
Switching `queueMicrotask` for `useEffect`
andrewhayward Jan 3, 2024
0464d0f
Smarter description block class name
andrewhayward Jan 3, 2024
563c3f0
Removing data persistence
andrewhayward Jan 3, 2024
6c1eb95
Removing related persistency changes
andrewhayward Jan 3, 2024
10727d3
Removing optional chaining of `onHide`
andrewhayward Jan 4, 2024
812eb31
Handling `nextHeaderMenuToFocus` a little more efficiently
andrewhayward Jan 5, 2024
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
177 changes: 122 additions & 55 deletions packages/dataviews/src/view-table.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* External dependencies
*/
import classNames from 'classnames';

/**
* WordPress dependencies
*/
Expand All @@ -9,7 +14,15 @@ import {
Icon,
privateApis as componentsPrivateApis,
} from '@wordpress/components';
import { Children, Fragment } from '@wordpress/element';
import {
Children,
Fragment,
forwardRef,
useEffect,
useId,
useRef,
useState,
} from '@wordpress/element';

/**
* Internal dependencies
Expand Down Expand Up @@ -39,7 +52,10 @@ const sanitizeOperators = ( field ) => {
);
};

function HeaderMenu( { field, view, onChangeView } ) {
const HeaderMenu = forwardRef( function HeaderMenu(
oandregal marked this conversation as resolved.
Show resolved Hide resolved
{ field, view, onChangeView, onHide },
ref
) {
const isHidable = field.enableHiding !== false;

const isSortable = field.enableSorting !== false;
Expand Down Expand Up @@ -74,6 +90,7 @@ function HeaderMenu( { field, view, onChangeView } ) {
size="compact"
className="dataviews-table-header-button"
style={ { padding: 0 } }
ref={ ref }
>
{ field.header }
{ isSorted && (
Expand Down Expand Up @@ -122,6 +139,7 @@ function HeaderMenu( { field, view, onChangeView } ) {
<DropdownMenuItem
prefix={ <Icon icon={ unseen } /> }
onClick={ () => {
onHide( field );
onChangeView( {
...view,
hiddenFields: view.hiddenFields.concat(
Expand Down Expand Up @@ -260,7 +278,7 @@ function HeaderMenu( { field, view, onChangeView } ) {
</WithSeparators>
</DropdownMenu>
);
}
} );

function WithSeparators( { children } ) {
return Children.toArray( children )
Expand All @@ -283,62 +301,106 @@ function ViewTable( {
isLoading = false,
deferredRendering,
} ) {
const headerMenuRefs = useRef( new Map() );
const headerMenuToFocusRef = useRef();
const [ nextHeaderMenuToFocus, setNextHeaderMenuToFocus ] = useState();

useEffect( () => {
if ( headerMenuToFocusRef.current ) {
headerMenuToFocusRef.current.focus();
headerMenuToFocusRef.current = undefined;
}
} );

const asyncData = useAsyncList( data );
const tableNoticeId = useId();

if ( nextHeaderMenuToFocus ) {
// If we need to force focus, we short-circuit rendering here
// to prevent any additional work while we handle that.
// Clearing out the focus directive is necessary to make sure
// future renders don't cause unexpected focus jumps.
headerMenuToFocusRef.current = nextHeaderMenuToFocus;
setNextHeaderMenuToFocus();
return;
}

const onHide = ( field ) => {
const hidden = headerMenuRefs.current.get( field.id );
const fallback = headerMenuRefs.current.get( hidden.fallback );
setNextHeaderMenuToFocus( fallback?.node );
};
const visibleFields = fields.filter(
( field ) =>
! view.hiddenFields.includes( field.id ) &&
! [ view.layout.mediaField, view.layout.primaryField ].includes(
field.id
)
);
const shownData = useAsyncList( data );
const usedData = deferredRendering ? shownData : data;
const usedData = deferredRendering ? asyncData : data;
const hasData = !! usedData?.length;
if ( isLoading ) {
// TODO:Add spinner or progress bar..
return (
<div className="dataviews-loading">
<h3>{ __( 'Loading' ) }</h3>
</div>
);
}
const sortValues = { asc: 'ascending', desc: 'descending' };

return (
<div className="dataviews-table-view-wrapper">
{ hasData && (
<table className="dataviews-table-view">
<thead>
<tr>
{ visibleFields.map( ( field ) => (
<th
key={ field.id }
style={ {
width: field.width || undefined,
minWidth: field.minWidth || undefined,
maxWidth: field.maxWidth || undefined,
<table
className="dataviews-table-view"
aria-busy={ isLoading }
aria-describedby={ tableNoticeId }
andrewhayward marked this conversation as resolved.
Show resolved Hide resolved
>
<thead>
<tr>
{ visibleFields.map( ( field, index ) => (
<th
key={ field.id }
style={ {
width: field.width || undefined,
minWidth: field.minWidth || undefined,
maxWidth: field.maxWidth || undefined,
} }
data-field-id={ field.id }
aria-sort={
view.sort?.field === field.id &&
sortValues[ view.sort.direction ]
}
scope="col"
>
<HeaderMenu
ref={ ( node ) => {
if ( node ) {
headerMenuRefs.current.set(
field.id,
{
node,
fallback:
visibleFields[
index > 0
? index - 1
: 1
]?.id,
}
);
} else {
headerMenuRefs.current.delete(
field.id
);
}
} }
data-field-id={ field.id }
aria-sort={
view.sort?.field === field.id &&
sortValues[ view.sort.direction ]
}
scope="col"
>
<HeaderMenu
field={ field }
view={ view }
onChangeView={ onChangeView }
/>
</th>
) ) }
{ !! actions?.length && (
<th data-field-id="actions">
{ __( 'Actions' ) }
</th>
) }
</tr>
</thead>
<tbody>
{ usedData.map( ( item ) => (
field={ field }
view={ view }
onChangeView={ onChangeView }
onHide={ onHide }
/>
</th>
) ) }
{ !! actions?.length && (
<th data-field-id="actions">{ __( 'Actions' ) }</th>
) }
</tr>
</thead>
<tbody>
{ hasData &&
usedData.map( ( item ) => (
<tr key={ getItemId( item ) }>
{ visibleFields.map( ( field ) => (
<td
Expand Down Expand Up @@ -366,14 +428,19 @@ function ViewTable( {
) }
</tr>
) ) }
</tbody>
</table>
) }
{ ! hasData && (
<div className="dataviews-no-results">
<p>{ __( 'No results' ) }</p>
</div>
) }
</tbody>
</table>
<div
className={ classNames( 'dataviews-table-status', {
'dataviews-loading': isLoading,
'dataviews-no-results': ! hasData && ! isLoading,
} ) }
andrewhayward marked this conversation as resolved.
Show resolved Hide resolved
id={ tableNoticeId }
>
{ ! hasData && (
<p>{ isLoading ? __( 'Loading…' ) : __( 'No results' ) }</p>
) }
</div>
</div>
);
}
Expand Down