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.

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

Merged
merged 30 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
d7b6a1a
ViewList: bootstrap view with previewField and onClickPreviewField
oandregal Dec 1, 2023
047a1c7
Remove dependency from View supports
oandregal Dec 1, 2023
17b00d5
Document API for preview
oandregal Dec 1, 2023
448cd12
Let the previewField handle its render
oandregal Dec 1, 2023
4ab95f6
Use primaryField/preview from view.layout
oandregal Dec 4, 2023
122b743
Take into account layout.preview to render interactivity hook & event…
oandregal Dec 4, 2023
494a1c9
Remove preview field: the list view should always have a preview
oandregal Dec 4, 2023
658e444
onClickPreview to onSelectionChange
oandregal Dec 4, 2023
a0254f1
Fix up rebase
oandregal Dec 4, 2023
623151f
Improve code legibility
oandregal Dec 4, 2023
98eee14
Do not pass the view to the render
oandregal Dec 4, 2023
e70540b
Make sure media field sizes are set for all views
oandregal Dec 4, 2023
58b2a57
Remove icon: it is meant to open the page details
oandregal Dec 5, 2023
e2fbf38
Display other fields
oandregal Dec 5, 2023
6910c9f
Display mediaField
oandregal Dec 5, 2023
b822691
Document use of mediaField
oandregal Dec 5, 2023
4d6994b
Styling
jameskoster Dec 5, 2023
d0aaf51
Use an element other than li as interactive
oandregal Dec 5, 2023
b081b15
Track selection
oandregal Dec 5, 2023
771f5f4
Do not render a link for the primary field in the list view
oandregal Dec 5, 2023
32dcc5d
Media wrapper and list item margin
jameskoster Dec 5, 2023
c0e1a36
Item interaction styles
jameskoster Dec 5, 2023
525c93c
Fix CSS lint issue
oandregal Dec 5, 2023
a8447bb
Selected item focus: use same styles as hover
oandregal Dec 5, 2023
1f72b36
Add keycodes to package-lock
oandregal Dec 5, 2023
9013662
Focus styles
jameskoster Dec 5, 2023
ed554c5
Heading weight
jameskoster Dec 5, 2023
4d3a7f2
Use formatListBullets for list view
jameskoster Dec 5, 2023
5621413
Button role: support SPACE
oandregal Dec 5, 2023
4d0011d
Button role: add aria-pressed
oandregal Dec 5, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions packages/dataviews/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ npm install @wordpress/dataviews --save
fields={ fields }
actions={ [ trashPostAction ] }
paginationInfo={ { totalItems, totalPages } }
onSelectionChange={ ( items ) => { /* ... */ } }
/>
```

Expand Down Expand Up @@ -75,8 +76,8 @@ Example:
- `value`: the actual value selected by the user.
- `hiddenFields`: the `id` of the fields that are hidden in the UI.
- `layout`: config that is specific to a particular layout type.
- `mediaField`: used by the `grid` layout. The `id` of the field to be used for rendering each card's media.
- `primaryField`: used by the `grid` layout. The `id` of the field to be used for rendering each card's title.
- `mediaField`: used by the `grid` and `list` layouts. The `id` of the field to be used for rendering each card's media.
- `primaryField`: used by the `grid` and `list` layouts. The `id` of the field to be highlighted in each card/list item.

### View <=> data

Expand Down
1 change: 1 addition & 0 deletions packages/dataviews/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"@wordpress/element": "file:../element",
"@wordpress/i18n": "file:../i18n",
"@wordpress/icons": "file:../icons",
"@wordpress/keycodes": "file:../keycodes",
"@wordpress/private-apis": "file:../private-apis",
"classnames": "^2.3.1",
"remove-accents": "^0.5.0"
Expand Down
9 changes: 0 additions & 9 deletions packages/dataviews/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,26 +29,17 @@ export const VIEW_LAYOUTS = [
label: __( 'Table' ),
component: ViewTable,
icon: blockTable,
supports: {
Copy link
Member Author

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 specific getViewSupport function).

preview: false,
},
},
{
type: LAYOUT_GRID,
label: __( 'Grid' ),
component: ViewGrid,
icon: category,
supports: {
preview: false,
},
},
{
type: LAYOUT_LIST,
label: __( 'List' ),
component: ViewList,
icon: drawerLeft,
supports: {
preview: true,
},
},
];
12 changes: 11 additions & 1 deletion packages/dataviews/src/dataviews.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
__experimentalVStack as VStack,
__experimentalHStack as HStack,
} from '@wordpress/components';
import { useMemo } from '@wordpress/element';
import { useMemo, useState } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -28,7 +28,15 @@ export default function DataViews( {
isLoading = false,
paginationInfo,
supportedLayouts,
onSelectionChange,
} ) {
const [ selection, setSelection ] = useState( [] );
youknowriad marked this conversation as resolved.
Show resolved Hide resolved

const onSetSelection = ( items ) => {
setSelection( items.map( ( item ) => item.id ) );
onSelectionChange( items );
};

const ViewComponent = VIEW_LAYOUTS.find(
( v ) => v.type === view.type
).component;
Expand Down Expand Up @@ -72,6 +80,8 @@ export default function DataViews( {
data={ data }
getItemId={ getItemId }
isLoading={ isLoading }
onSelectionChange={ onSetSelection }
selection={ selection }
/>
<Pagination
view={ view }
Expand Down
80 changes: 80 additions & 0 deletions packages/dataviews/src/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,86 @@
}
}

.dataviews-list-view {
li {
border-bottom: $border-width solid $gray-100;
margin: 0;
&:last-child {
border-bottom: 0;
}
}

.dataviews-list-view__item {
padding: $grid-unit-15 $grid-unit-40;
cursor: default;
&:focus,
&:hover {
background-color: lighten($gray-100, 3%);
}
&:focus {
box-shadow: inset 0 0 0 var(--wp-admin-border-width-focus) var(--wp-admin-theme-color);
}
h3 {
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
}
}

.dataviews-list-view__item-selected,
.dataviews-list-view__item-selected:hover {
background-color: $gray-100;

&:focus {
box-shadow: inset 0 0 0 var(--wp-admin-border-width-focus) var(--wp-admin-theme-color);
}
}

.dataviews-list-view__media-wrapper {
min-width: $grid-unit-40;
height: $grid-unit-40;
border-radius: $grid-unit-05;
overflow: hidden;
position: relative;

&::after {
content: "";
position: absolute;
top: 0;
left: 0;
width: 100%;
height: 100%;
box-shadow: inset 0 0 0 $border-width rgba(0, 0, 0, 0.1);
border-radius: $grid-unit-05;
}
}

.edit-site-page-pages__featured-image,
.dataviews-list-view__media-placeholder {
min-width: $grid-unit-40;
height: $grid-unit-40;
}

.dataviews-list-view__media-placeholder {
background-color: $gray-200;
}

.dataviews-list-view__fields {
color: $gray-700;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;

.dataviews-list-view__field {
margin-right: $grid-unit-15;

&:last-child {
margin-right: 0;
}
}
}
}

.dataviews-action-modal {
z-index: z-index(".dataviews-action-modal");
}
7 changes: 3 additions & 4 deletions packages/dataviews/src/view-grid.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 } ) }
Copy link
Member Author

Choose a reason for hiding this comment

The 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 view-table.js. This change is not necessary for this PR to work.

{ 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 }
Expand All @@ -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;
Expand All @@ -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>
);
Expand Down
97 changes: 92 additions & 5 deletions packages/dataviews/src/view-list.js
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 ] ) }
>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried/considered using Button here? Generally better to use existing components rather than reimplementing behaviour.

Copy link
Member Author

Choose a reason for hiding this comment

The 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).

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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?

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...

An article 'card' showing an image, a large title, some text, and a category. The title is underlined with a dark focus ring. An article 'card' showing an image, a large title, some text, and a category. The category is underlined with a dark focus ring.

There are two interactive elements in this card – links for the article title, and the the article category.

An article 'card' showing an image, a large title, some text, and a category. The title is underlined, and a mouse pointer is visible in the centre of the card. An article 'card' showing an image, a large title, some text, and a category. The category is underlined, with a mouse pointer visible on top of it.

However, through some clever use of generated content, and a bit of z-index magic, the entire card becomes clickable, and behaves as if it's all one big link, unless you intentionally hover over the category link, which then becomes the target action.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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>
);
}
3 changes: 1 addition & 2 deletions packages/dataviews/src/view-table.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,7 @@ function ViewTable( {
const columns = useMemo( () => {
const _columns = fields.map( ( field ) => {
const { render, getValue, ...column } = field;
column.cell = ( props ) =>
Copy link
Member Author

Choose a reason for hiding this comment

The 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 view-grid.js. This change is not necessary for this PR to work.

render( { item: props.row.original, view } );
column.cell = ( props ) => render( { item: props.row.original } );
if ( getValue ) {
column.accessorFn = ( item ) => getValue( { item } );
}
Expand Down
Loading
Loading