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 Sidebar: Display item count on DataViews sidebar #62028

Closed
wants to merge 4 commits into from
Closed

DataViews Sidebar: Display item count on DataViews sidebar #62028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b5531a
feat: Display item count on dataviews sidebar
Souptik2001 May 27, 2024
ce135d9
Merge branch 'WordPress:trunk' into update/dataviews-sidebar-display-…
Souptik2001 May 27, 2024
eb0647f
Merge branch 'trunk' into update/dataviews-sidebar-display-item-count
Souptik2001 May 28, 2024
fb217d1
feat: Remove trash page count on "all pages" view
Souptik2001 May 28, 2024
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 packages/edit-site/src/components/sidebar-dataviews/dataview-item.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const { useLocation } = unlock( routerPrivateApis );
export default function DataViewItem( {
title,
slug,
count,
customViewId,
type,
icon,
Expand Down Expand Up @@ -51,6 +52,7 @@ export default function DataViewItem( {
<SidebarNavigationItem
icon={ iconToUse }
{ ...linkInfo }
suffix={ <span>{ count }</span> }
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the span needed? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

The span is not needed, but if count is 0 it might mess up with the boolean logic:

gutenberg/packages/edit-site/src/components/sidebar-navigation-item/index.js

Line 56 in e1aaee2

{ 'with-suffix': ! withChevron && suffix },

Passing an object (which the React element <span /> essentially is) will always evaluate to true, bypassing that problem.

But again, the span is not needed. The same can be achieved with a React.Fragment:

Suggested change
suffix={ <span>{ count }</span> }
suffix={ <>{ count }</> }

Alternatively, we'd alter suffix to deal better with 0 values, but this sounds like overkill.

aria-current={ isActive ? 'true' : undefined }
>
{ title }
Expand Down
12 changes: 11 additions & 1 deletion packages/edit-site/src/components/sidebar-dataviews/default-views.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
LAYOUT_TABLE,
LAYOUT_GRID,
OPERATOR_IS_ANY,
OPERATOR_IS_NONE,
} from '../../utils/constants';

export const DEFAULT_CONFIG_PER_VIEW_TYPE = {
Expand Down Expand Up @@ -52,7 +53,16 @@ export const DEFAULT_VIEWS = {
title: __( 'All pages' ),
slug: 'all',
icon: pages,
view: DEFAULT_PAGE_BASE,
view: {
...DEFAULT_PAGE_BASE,
filters: [
{
field: 'status',
operator: OPERATOR_IS_NONE,
value: 'trash',
},
],
Comment on lines +58 to +64
Copy link
Author

Choose a reason for hiding this comment

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

Although this filter is not needed for the query which is run to fetch all the pages, because trash is excluded by default, but this needed in the JS filter we are doing here - https://github.com/WordPress/gutenberg/pull/62028/files#diff-67163f722d76f852faaff5a7c0c6046d411846fc118479e62b70273363272d5cR27

Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we doing JS filtering? IMO, we shouldn't because the REST API can be altered and extended in multiple ways so the only way to get a reliable result is to use the same REST API used in the dataviews.

Copy link
Member

Choose a reason for hiding this comment

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

I need to spend more time with data views, so I can't respond to the above point, but I don't think we need this filter here at all. The resulting filter badge, when clicked, does nothing.

},
},
{
title: __( 'Published' ),
Expand Down
6 changes: 4 additions & 2 deletions packages/edit-site/src/components/sidebar-dataviews/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,17 @@ import { privateApis as routerPrivateApis } from '@wordpress/router';
* Internal dependencies
*/

import { DEFAULT_VIEWS } from './default-views';
import { unlock } from '../../lock-unlock';
const { useLocation } = unlock( routerPrivateApis );
import DataViewItem from './dataview-item';
import CustomDataViewsList from './custom-dataviews-list';
import useDataViews from './use-data-views';

export default function DataViewsSidebarContent() {
const {
params: { postType, activeView = 'all', isCustom = 'false' },
} = useLocation();
const views = useDataViews( postType );
if ( ! postType ) {
return null;
}
Expand All @@ -26,11 +27,12 @@ export default function DataViewsSidebarContent() {
return (
<>
<ItemGroup>
{ DEFAULT_VIEWS[ postType ].map( ( dataview ) => {
{ views.map( ( dataview ) => {
return (
<DataViewItem
key={ dataview.slug }
slug={ dataview.slug }
count={ dataview.records?.length || 0 }
title={ dataview.title }
icon={ dataview.icon }
type={ dataview.view.type }
Expand Down
54 changes: 54 additions & 0 deletions packages/edit-site/src/components/sidebar-dataviews/use-data-views.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/**
* WordPress dependencies
*/
import { useEntityRecords } from '@wordpress/core-data';
import { useMemo } from '@wordpress/element';

/**
* Internal dependencies
*/
import { OPERATOR_IS_ANY, OPERATOR_IS_NONE } from '../../utils/constants';
import { DEFAULT_VIEWS } from './default-views';

export default function useDataViews( postType ) {
// Fetch all the posts of the post type.
const data = useEntityRecords( 'postType', postType, {
per_page: -1,
Copy link
Author

Choose a reason for hiding this comment

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

I am a little skeptical about fetching all the results using -1 like this.
But I am not able to think about any better way. Please suggest if you get any better approach. 🙏

Copy link
Member

@ramonjd ramonjd Jun 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If you just want the total count, did you try looking at totalItems returned by useEntityRecords?

Example:

const { totalItems } = useEntityRecords( 'postType', 'post', { status: [ 'any', 'trash' ] } ) );

useEntityRecords makes a call to the getEntityRecordsTotalItems selector, which returns the count sent via response headers in X-WP-Total when calling getEntityRecords.

Or you call getEntityRecordsTotalItems directly maybe.

Copy link
Member

Choose a reason for hiding this comment

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

Trying something over here with a new route and wp_count_posts

status: [ 'any', 'trash' ],
} )?.records;

// Insert the records for each view.
const views = useMemo( () => {
return DEFAULT_VIEWS[ postType ].map( ( view ) => {
const viewFilters = view?.view?.filters;
// Filter the records matching the view filters.
view.records = data?.filter( ( record ) => {
// Check if the record matches all the filters.
return viewFilters.every( ( filter ) => {
let filterValues = filter.value;
const filterField = filter.field;
const filterOperator = filter.operator;

// Convert string to array value if required.
if ( ! Array.isArray( filterValues ) ) {
filterValues = [ filterValues ];
}

if ( filterOperator === OPERATOR_IS_ANY ) {
return filterValues.includes( record[ filterField ] );
}

if ( filterOperator === OPERATOR_IS_NONE ) {
return ! filterValues.includes( record[ filterField ] );
}

return true;
} );
} );

return view;
} );
}, [ data, postType ] );

return views;
}
Loading