-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
DataViews: iterate on list view #56746
Changes from 24 commits
d7b6a1a
047a1c7
17b00d5
448cd12
4ab95f6
122b743
494a1c9
658e444
a0254f1
623151f
98eee14
e70540b
58b2a57
e2fbf38
6910c9f
b822691
4d6994b
d0aaf51
b081b15
771f5f4
32dcc5d
c0e1a36
525c93c
a8447bb
1f72b36
9013662
ed554c5
4d3a7f2
5621413
4d0011d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,14 +43,14 @@ export default function ViewGrid( { data, fields, view, actions, getItemId } ) { | |
className="dataviews-view-grid__card" | ||
> | ||
<div className="dataviews-view-grid__media"> | ||
{ mediaField?.render( { item, view } ) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can access the |
||
{ mediaField?.render( { item } ) } | ||
</div> | ||
<HStack | ||
className="dataviews-view-grid__primary-field" | ||
justify="space-between" | ||
> | ||
<FlexBlock> | ||
{ primaryField?.render( { item, view } ) } | ||
{ primaryField?.render( { item } ) } | ||
</FlexBlock> | ||
<ItemActions | ||
item={ item } | ||
|
@@ -65,7 +65,6 @@ export default function ViewGrid( { data, fields, view, actions, getItemId } ) { | |
{ visibleFields.map( ( field ) => { | ||
const renderedValue = field.render( { | ||
item, | ||
view, | ||
} ); | ||
if ( ! renderedValue ) { | ||
return null; | ||
|
@@ -80,7 +79,7 @@ export default function ViewGrid( { data, fields, view, actions, getItemId } ) { | |
{ field.header } | ||
</div> | ||
<div className="dataviews-view-grid__field-value"> | ||
{ field.render( { item, view } ) } | ||
{ field.render( { item } ) } | ||
</div> | ||
</VStack> | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,96 @@ | ||
/** | ||
* Internal dependencies | ||
* External dependencies | ||
*/ | ||
import ViewTable from './view-table'; | ||
import classNames from 'classnames'; | ||
|
||
export default function ViewList( props ) { | ||
// To do: change to email-like preview list. | ||
return <ViewTable { ...props } />; | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useAsyncList } from '@wordpress/compose'; | ||
import { | ||
__experimentalHStack as HStack, | ||
__experimentalVStack as VStack, | ||
} from '@wordpress/components'; | ||
import { ENTER } from '@wordpress/keycodes'; | ||
|
||
export default function ViewList( { | ||
view, | ||
fields, | ||
data, | ||
getItemId, | ||
onSelectionChange, | ||
selection, | ||
} ) { | ||
const shownData = useAsyncList( data, { step: 3 } ); | ||
const mediaField = fields.find( | ||
( field ) => field.id === view.layout.mediaField | ||
); | ||
const primaryField = fields.find( | ||
( field ) => field.id === view.layout.primaryField | ||
); | ||
const visibleFields = fields.filter( | ||
( field ) => | ||
! view.hiddenFields.includes( field.id ) && | ||
! [ view.layout.primaryField, view.layout.mediaField ].includes( | ||
field.id | ||
) | ||
); | ||
|
||
const onEnter = ( item ) => ( event ) => { | ||
const { keyCode } = event; | ||
if ( keyCode === ENTER ) { | ||
onSelectionChange( [ item ] ); | ||
} | ||
}; | ||
|
||
return ( | ||
<ul className="dataviews-list-view"> | ||
{ shownData.map( ( item, index ) => { | ||
return ( | ||
<li key={ getItemId?.( item ) || index }> | ||
<div | ||
role="button" | ||
tabIndex={ 0 } | ||
onKeyDown={ onEnter( item ) } | ||
className={ classNames( | ||
'dataviews-list-view__item', | ||
{ | ||
'dataviews-list-view__item-selected': | ||
selection.includes( item.id ), | ||
} | ||
) } | ||
onClick={ () => onSelectionChange( [ item ] ) } | ||
> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you tried/considered using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. Other than styling, my main concern was that this component is going to have buttons within (this, but also actions). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm... OK. In which case, we shouldn't be doing this at all! Will need to have a think about how best to handle this scenario. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hopefully it's okay to 'visually nest' those buttons, with either absolute positioning, something like the List view implementation, or some other technique? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah, it's a pretty well established paradigm to visually overlay interactive elements. We just can't nest them in the DOM. For example, this card pattern is routinely used on news sites and in other related content... There are two interactive elements in this card – links for the article title, and the the article category. However, through some clever use of generated content, and a bit of We don't have to implement it exactly like that, but the general idea is there. One thing to be aware of when doing this is target size – it's bad enough when it's hard to tap on something, but when it's surrounded by a different action context it's even worse. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice, thanks for sharing the details. It'll be useful when we implement this feature. |
||
<HStack spacing={ 3 }> | ||
<div className="dataviews-list-view__media-wrapper"> | ||
{ mediaField?.render( { item } ) || ( | ||
<div className="dataviews-list-view__media-placeholder"></div> | ||
) } | ||
</div> | ||
<HStack> | ||
<VStack spacing={ 1 }> | ||
{ primaryField?.render( { item } ) } | ||
<div className="dataviews-list-view__fields"> | ||
{ visibleFields.map( ( field ) => { | ||
return ( | ||
<span | ||
key={ field.id } | ||
className="dataviews-list-view__field" | ||
> | ||
{ field.render( { | ||
item, | ||
} ) } | ||
</span> | ||
); | ||
} ) } | ||
</div> | ||
</VStack> | ||
</HStack> | ||
</HStack> | ||
</div> | ||
</li> | ||
); | ||
} ) } | ||
</ul> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -344,8 +344,7 @@ function ViewTable( { | |
const columns = useMemo( () => { | ||
const _columns = fields.map( ( field ) => { | ||
const { render, getValue, ...column } = field; | ||
column.cell = ( props ) => | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can access the view in the fields API directly, no need to pass it to the field through render. Same for |
||
render( { item: props.row.original, view } ); | ||
column.cell = ( props ) => render( { item: props.row.original } ); | ||
if ( getValue ) { | ||
column.accessorFn = ( item ) => getValue( { item } ); | ||
} | ||
|
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.
This means there is no longer a need to export anything from dataviews (
VIEW_LAYOUTS
or any specificgetViewSupport
function).