-
Notifications
You must be signed in to change notification settings - Fork 4.5k
DataViews: Add data picker component #70971
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
Open
talldan
wants to merge
32
commits into
trunk
Choose a base branch
from
add/data-picker-component
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+796
−5
Open
Changes from all commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
5fd0746
Create DataPicker component as a sibling to DataViews
talldan f6f7115
Update prop handling
talldan 6d5e837
Add foundational parts for DataPicker component
talldan f1783e7
Add some initial styles a parts of the dataview UI (search, filters, …
talldan b1b623f
Add aria properties
talldan 2ab695e
Refactor layout to separate component
talldan 8ff8c83
Implement basic active descendent handling
talldan 99313ba
Refactor active index
talldan 1de7571
Fix Voiceover moving focus outside listbox
talldan df96268
Add focus ring
talldan 08ee052
Fix incorrect active descendent when paginating
talldan f078310
Beginnings of selection
talldan 8ae5450
Make the description an aria-description
talldan 8c40af1
Add correct padding around grid
talldan 4e6bf58
Improve selection style
talldan 1d13052
Allow selection across pagination
talldan 1cd771b
Match grid sizing of grid dataview
talldan 7877ee9
Use ariakit instead of custom active descendent implementation
talldan 0838c64
Use focus-visible for focus rings
talldan c7ef973
Clarify comment
talldan dbfdfb4
Only show focus outline when an active descendant is not set
talldan a72f23c
Handle multi-selection
talldan 0df3f0b
Add aria label to listbox
talldan 85febc1
Add controls
talldan 0801cd4
Use aria-selected instead of aria-checked, the latter is not announce…
talldan e190e90
Add no results / loading indication
talldan 291ad34
Disable finish button when no items selected
talldan cfb2180
Add more footer furnishings
talldan 9ec9fdc
Rearrange footer
talldan 20815ec
Only show select all checkbox for multiselections
talldan 6dbe93b
Revert export of dataviews types
talldan 9992c14
Adopt responsive image changes
talldan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
267 changes: 267 additions & 0 deletions
267
packages/dataviews/src/components/datapicker/grid-layout.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,267 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import clsx from 'clsx'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { | ||
Composite, | ||
__experimentalVStack as VStack, | ||
CheckboxControl, | ||
Spinner, | ||
} from '@wordpress/components'; | ||
import { __ } from '@wordpress/i18n'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import type { | ||
NormalizedField, | ||
ViewPickerGrid, | ||
ViewBaseProps, | ||
} from '../../types'; | ||
import type { SetSelection } from '../../private-types'; | ||
import { useContext } from '@wordpress/element'; | ||
import DataViewsContext from '../dataviews-context'; | ||
|
||
type DataPickerGridLayoutProps< Item > = { | ||
multiple: boolean; | ||
label: string; | ||
data: ViewBaseProps< Item >[ 'data' ]; | ||
fields: ViewBaseProps< Item >[ 'fields' ]; | ||
getItemId: ViewBaseProps< Item >[ 'getItemId' ]; | ||
isLoading?: boolean; | ||
onChangeView: ViewBaseProps< Item >[ 'onChangeView' ]; | ||
view: ViewPickerGrid; | ||
selection: string[]; | ||
onChangeSelection: SetSelection; | ||
setSize: number; | ||
startPosition: number; | ||
}; | ||
|
||
export default function DataPickerGridLayout< Item >( { | ||
multiple, | ||
label, | ||
data, | ||
fields, | ||
getItemId, | ||
isLoading, | ||
view, | ||
selection, | ||
onChangeSelection, | ||
setSize, | ||
startPosition, | ||
}: DataPickerGridLayoutProps< Item > ) { | ||
const { resizeObserverRef } = useContext( DataViewsContext ); | ||
|
||
const hasData = !! data?.length; | ||
|
||
const titleField = fields.find( | ||
( field ) => field.id === view?.titleField | ||
); | ||
const mediaField = fields.find( | ||
( field ) => field.id === view?.mediaField | ||
); | ||
const descriptionField = fields.find( | ||
( field ) => field.id === view?.descriptionField | ||
); | ||
|
||
const usedPreviewSize = view.layout?.previewSize; | ||
/* | ||
* This is the maximum width that an image can achieve in the grid. The reasoning is: | ||
* The biggest min image width available is 430px (see /dataviews-layouts/grid/preview-size-picker.tsx). | ||
* Because the grid is responsive, once there is room for another column, the images shrink to accommodate it. | ||
* So each image will never grow past 2*430px plus a little more to account for the gaps. | ||
*/ | ||
const size = '900px'; | ||
|
||
return ( | ||
<> | ||
{ hasData && ( | ||
<Composite | ||
virtualFocus | ||
orientation="horizontal" | ||
render={ | ||
<ul | ||
ref={ | ||
resizeObserverRef as React.RefObject< HTMLUListElement > | ||
} | ||
style={ { | ||
gridTemplateColumns: | ||
usedPreviewSize && | ||
`repeat(auto-fill, minmax(${ usedPreviewSize }px, 1fr))`, | ||
} } | ||
className="dataviews-picker-grid" | ||
aria-label={ label } | ||
role="listbox" | ||
aria-multiselectable={ multiple } | ||
tabIndex={ 0 } | ||
aria-busy={ isLoading } | ||
/> | ||
} | ||
> | ||
{ data.map( ( item, index ) => { | ||
const position = startPosition + index; | ||
const itemId = getItemId( item ); | ||
const className = clsx( 'dataviews-picker-grid__card', { | ||
'is-selected': selection.includes( itemId ), | ||
} ); | ||
return ( | ||
<GridItem | ||
key={ itemId } | ||
className={ className } | ||
multiple={ multiple } | ||
view={ view } | ||
selection={ selection } | ||
onChangeSelection={ onChangeSelection } | ||
getItemId={ getItemId } | ||
item={ item } | ||
titleField={ titleField } | ||
mediaField={ mediaField } | ||
descriptionField={ descriptionField } | ||
setSize={ setSize } | ||
position={ position } | ||
config={ { | ||
sizes: size, | ||
} } | ||
/> | ||
); | ||
} ) } | ||
</Composite> | ||
) } | ||
{ | ||
// Render empty state. | ||
! hasData && ( | ||
<div | ||
className={ clsx( { | ||
'dataviews-loading': isLoading, | ||
'dataviews-no-results': ! isLoading, | ||
} ) } | ||
> | ||
<p>{ isLoading ? <Spinner /> : __( 'No results' ) }</p> | ||
</div> | ||
) | ||
} | ||
</> | ||
); | ||
} | ||
|
||
type GridItemProps< Item > = { | ||
className: string; | ||
multiple: boolean; | ||
item: Item; | ||
view: ViewPickerGrid; | ||
selection: string[]; | ||
onChangeSelection: SetSelection; | ||
getItemId: ( item: Item ) => string; | ||
titleField: NormalizedField< Item > | undefined; | ||
mediaField: NormalizedField< Item > | undefined; | ||
descriptionField: NormalizedField< Item > | undefined; | ||
setSize: number; | ||
position: number; | ||
config: { | ||
sizes: string; | ||
}; | ||
}; | ||
|
||
function GridItem< Item >( { | ||
className, | ||
multiple, | ||
item, | ||
view, | ||
selection, | ||
onChangeSelection, | ||
getItemId, | ||
titleField, | ||
mediaField, | ||
descriptionField, | ||
setSize, | ||
position, | ||
config, | ||
}: GridItemProps< Item > ) { | ||
const { showTitle = true, showMedia = true, showDescription = true } = view; | ||
const renderedMediaField = | ||
showMedia && mediaField?.render ? ( | ||
<mediaField.render | ||
item={ item } | ||
field={ mediaField } | ||
config={ config } | ||
/> | ||
) : null; | ||
const renderedTitleField = | ||
showTitle && titleField?.render ? ( | ||
<titleField.render item={ item } field={ titleField } /> | ||
) : null; | ||
const renderedDescriptionField = | ||
showDescription && descriptionField?.render ? ( | ||
<descriptionField.render item={ item } field={ descriptionField } /> | ||
) : null; | ||
|
||
const itemId = getItemId( item ); | ||
const isSelected = selection.includes( getItemId( item ) ); | ||
|
||
const descriptionId = `dataviews-picker-grid-item-${ itemId }-description`; | ||
|
||
return ( | ||
<Composite.Item | ||
render={ ( props ) => ( | ||
<VStack | ||
{ ...props } | ||
children={ props.children } | ||
as="li" | ||
spacing={ 0 } | ||
className={ className } | ||
role="option" | ||
aria-posinset={ position } | ||
aria-setsize={ setSize } | ||
aria-selected={ isSelected } | ||
aria-describedby={ descriptionId } | ||
onClick={ () => { | ||
if ( isSelected ) { | ||
onChangeSelection( | ||
selection.filter( | ||
( selectionId ) => itemId !== selectionId | ||
) | ||
); | ||
return; | ||
} | ||
|
||
if ( multiple ) { | ||
onChangeSelection( [ ...selection, itemId ] ); | ||
} else { | ||
onChangeSelection( [ itemId ] ); | ||
} | ||
} } | ||
/> | ||
) } | ||
> | ||
<div className="dataviews-picker-grid__media"> | ||
{ renderedMediaField } | ||
</div> | ||
<CheckboxControl | ||
// This checkbox is decorative to ensure that there are no extra tab stops | ||
// in the grid. | ||
// To make that happen, it's hidden from screen readers, has a tabIndex of -1 | ||
// and has pointer-events: none in its css. | ||
aria-hidden | ||
tabIndex={ -1 } | ||
__nextHasNoMarginBottom | ||
className="dataviews-picker-grid__selection-checkbox" | ||
checked={ isSelected } | ||
onChange={ () => {} } | ||
/> | ||
<div className="dataviews-picker-grid__title-field"> | ||
{ renderedTitleField } | ||
</div> | ||
<div | ||
id={ descriptionId } | ||
className="dataviews-picker-grid__description-field" | ||
aria-hidden | ||
> | ||
{ renderedDescriptionField } | ||
</div> | ||
</Composite.Item> | ||
); | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Does this need to be different that "grid"? I'm trying to understand what are the main differences between the two components, what should be shared or not...?
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.
The markup, semantics and keyboard navigation are quite different. Also different props.
The parts that are most similar are some of the styles, like the grid columns and responsiveness.
Possibly we could try using the same view, but consider selection a different mode of the view that when active adds the different roles, keyboard nav and so on. It'd need to do quite a lot, like remove all the tabbable elements within items. 🤔
Uh oh!
There was an error while loading. Please reload this page.
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.
Yes, this is what I kind of was thinking about, maybe a "layout" can decide whether it can be used in "DataViews", "DataPicker" or both.
Now it doesn't mean that it can't be two separate components, but I feel like the entry point should at least be the same no?
Now, I do think this PR is a bit "fundamental" in the sense that introduces a new component / new behavior entirely for DataViews package. I would love as much feedback as possible as I do have uncertainties myself. cc @WordPress/gutenberg-core @mtias @aduth
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.
If we were to reuse layouts, why not just use DataViews with a "picker" mode instead of creating a dedicated component? Most (maybe all?) of the custom behaviour we need to introduce here concerns the layout structure, semantics and interaction, as well as the structure and interaction of the items, which is also defined inside the layouts.
For the sake of not adding more complexity to the current view layouts, I like the idea of adding a new "picker" layout - or several, but I'm not convinced picker needs more than one layout component. Certainly its HTML shouldn't change because it needs the listbox semantics, so if we want to customise the display I think it would be better to do it purely with CSS variations.