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

[DataViews]: Update the view config to include fields visibility #55247

Merged
merged 4 commits into from
Oct 11, 2023
Merged

[DataViews]: Update the view config to include fields visibility #55247

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d1ceb97
[DataViews]: Update the view config to include fields visibility
ntsekouras Oct 11, 2023
9ac931d
fix typo
ntsekouras Oct 11, 2023
4708622
use array instead of Set for `hidden`
ntsekouras Oct 11, 2023
a64c48a
use `hiddenFields` top level prop
ntsekouras Oct 11, 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
51 changes: 48 additions & 3 deletions packages/edit-site/src/components/dataviews/dataviews.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import ViewActions from './view-actions';
import TextFilter from './text-filter';
import { moreVertical } from '@wordpress/icons';

const EMPTY_OBJECT = {};

export default function DataViews( {
actions,
data,
Expand All @@ -43,8 +45,8 @@ export default function DataViews( {
options: { pageCount },
} ) {
const columns = useMemo( () => {
const _columns = [ ...fields ];
if ( actions && actions.length ) {
let _columns = [ ...fields ];
if ( actions?.length ) {
_columns.push( {
header: <VisuallyHidden>{ __( 'Actions' ) }</VisuallyHidden>,
id: 'actions',
Expand Down Expand Up @@ -79,9 +81,30 @@ export default function DataViews( {
enableHiding: false,
} );
}
if ( view.fields?.hidable?.length ) {
_columns = _columns.map( ( column ) => {
return {
...column,
enableHiding: view.fields.hidable.includes( column.id ),
};
} );
}

return _columns;
}, [ fields, actions ] );
}, [ fields, actions, view.fields?.hidable ] );

const columnVisibility = useMemo( () => {
if ( ! view.fields?.hidden?.size ) {
return;
}
return Array.from( view.fields.hidden ).reduce(
( accumulator, fieldId ) => ( {
...accumulator,
[ fieldId ]: false,
} ),
{}
);
}, [ view.fields?.hidden ] );

const dataView = useReactTable( {
data,
Expand All @@ -104,6 +127,7 @@ export default function DataViews( {
pageIndex: view.page,
pageSize: view.perPage,
},
columnVisibility: columnVisibility ?? EMPTY_OBJECT,
},
onSortingChange: ( sortingUpdater ) => {
onChangeView( ( currentView ) => {
Expand Down Expand Up @@ -135,6 +159,27 @@ export default function DataViews( {
};
} );
},
onColumnVisibilityChange: ( columnVisibilityUpdater ) => {
onChangeView( ( currentView ) => {
const hiddenFields = Object.entries(
columnVisibilityUpdater()
).reduce( ( accumulator, [ fieldId, value ] ) => {
if ( value ) {
accumulator.delete( fieldId );
} else {
accumulator.add( fieldId );
}
return accumulator;
}, new Set( currentView.fields.hidden ) );
return {
...currentView,
fields: {
...currentView.fields,
hidden: hiddenFields,
},
};
} );
},
onGlobalFilterChange: ( value ) => {
onChangeView( { ...view, search: value, page: 0 } );
},
Expand Down
6 changes: 3 additions & 3 deletions packages/edit-site/src/components/dataviews/view-actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,10 @@ function PageSizeMenu( { dataView } ) {
}

function FieldsVisibilityMenu( { dataView } ) {
const hideableFields = dataView
const hidableFields = dataView
.getAllColumns()
.filter( ( columnn ) => columnn.getCanHide() );
if ( ! hideableFields?.length ) {
if ( ! hidableFields?.length ) {
return null;
}
return (
Expand All @@ -118,7 +118,7 @@ function FieldsVisibilityMenu( { dataView } ) {
</DropdownSubMenuTriggerV2>
}
>
{ hideableFields?.map( ( field ) => {
{ hidableFields?.map( ( field ) => {
return (
<DropdownMenuItemV2
key={ field.id }
Expand Down
20 changes: 19 additions & 1 deletion packages/edit-site/src/components/page-pages/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { __ } from '@wordpress/i18n';
import { useEntityRecords } from '@wordpress/core-data';
import { decodeEntities } from '@wordpress/html-entities';
import { useState, useMemo } from '@wordpress/element';
import { dateI18n, getDate, getSettings } from '@wordpress/date';

/**
* Internal dependencies
Expand All @@ -31,6 +32,12 @@ export default function PagePages() {
field: 'date',
direction: 'desc',
},
fields: {
// All fields are visible by default, so it's
// better to keep track of the hidden ones.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this assumption is true for all the views. I'd think that it's probably the opposite: most fields should be hidden by default but I'm ok starting this way.

hidden: new Set( [ 'date' ] ),
ntsekouras marked this conversation as resolved.
Show resolved Hide resolved
hidable: [ 'author', 'status', 'date' ],
},
} );
// Request post statuses to get the proper labels.
const { records: statuses } = useEntityRecords( 'root', 'status' );
Expand Down Expand Up @@ -95,7 +102,6 @@ export default function PagePages() {
},
maxWidth: 400,
sortingFn: 'alphanumeric',
enableHiding: false,
},
{
header: __( 'Author' ),
Expand All @@ -117,6 +123,18 @@ export default function PagePages() {
postStatuses[ page.status ] ?? page.status,
enableSorting: false,
},
{
header: 'Date',
id: 'date',
cell: ( props ) => {
Copy link
Member

Choose a reason for hiding this comment

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

Following up this rationale, what do you think of doing this instead (pseudo-code):

accessorFn: ( page ) => {
    return dateI18n(
        getSettings().formats.datetimeAbbreviated,
        getDate( props.row.original.date )
    );
}
cell: ( props ) => { return <time>{props.row.getValue()}</time> }

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to redo accessorFn and cell to be less tied to the API of tanstack at some point, and yeah we need to agree on a given way of retrieving the fields data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd echo Riad on this. We need to start forming the API that will map internally to tanstack.

const formattedDate = dateI18n(
getSettings().formats.datetimeAbbreviated,
getDate( props.row.original.date )
Copy link
Member

@ellatrix ellatrix Oct 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The default edit.php page seems to fallback to the modified data for drafts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The date field in this PR was just a placeholder without the proper handling of this field to showcase the API. We can address this in follow ups.

);
return <time>{ formattedDate }</time>;
},
enableSorting: false,
Copy link
Member

Choose a reason for hiding this comment

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

This field is used to sort the table by default. Why a user shouldn't be able to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe a user can do it in the future, but it might be more convoluted when we have composite fields or something that would need to provide the sorting args. I think more of the APIs will evolve/created per iteration and use case.

Copy link
Member

Choose a reason for hiding this comment

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

Follow-up in #55388

},
],
[ postStatuses ]
);
Expand Down
Loading