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: sort author by name + allow custom sort function #64064

Merged
merged 9 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 17 additions & 0 deletions packages/dataviews/src/components/dataviews/stories/fixtures.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export const data = [
type: 'Not a planet',
categories: [ 'Space', 'NASA' ],
satellites: 0,
satellites_no_type: 0,
},
{
id: 2,
Expand All @@ -32,6 +33,7 @@ export const data = [
type: 'Not a planet',
categories: [ 'Space' ],
satellites: 0,
satellites_no_type: 0,
},
{
id: 3,
Expand All @@ -41,6 +43,7 @@ export const data = [
type: 'Not a planet',
categories: [ 'NASA' ],
satellites: 0,
satellites_no_type: 0,
},
{
id: 4,
Expand All @@ -50,6 +53,7 @@ export const data = [
type: 'Ice giant',
categories: [ 'Space', 'Planet', 'Solar system' ],
satellites: 14,
satellites_no_type: 14,
},
{
id: 5,
Expand All @@ -59,6 +63,7 @@ export const data = [
type: 'Terrestrial',
categories: [ 'Space', 'Planet', 'Solar system' ],
satellites: 0,
satellites_no_type: 0,
},
{
id: 6,
Expand All @@ -68,6 +73,7 @@ export const data = [
type: 'Terrestrial',
categories: [ 'Space', 'Planet', 'Solar system' ],
satellites: 0,
satellites_no_type: 0,
},
{
id: 7,
Expand All @@ -77,6 +83,7 @@ export const data = [
type: 'Terrestrial',
categories: [ 'Space', 'Planet', 'Solar system' ],
satellites: 1,
satellites_no_type: 1,
},
{
id: 8,
Expand All @@ -86,6 +93,7 @@ export const data = [
type: 'Terrestrial',
categories: [ 'Space', 'Planet', 'Solar system' ],
satellites: 2,
satellites_no_type: 2,
},
{
id: 9,
Expand All @@ -95,6 +103,7 @@ export const data = [
type: 'Gas giant',
categories: [ 'Space', 'Planet', 'Solar system' ],
satellites: 95,
satellites_no_type: 95,
},
{
id: 10,
Expand All @@ -104,6 +113,7 @@ export const data = [
type: 'Gas giant',
categories: [ 'Space', 'Planet', 'Solar system' ],
satellites: 146,
satellites_no_type: 146,
},
{
id: 11,
Expand All @@ -113,6 +123,7 @@ export const data = [
type: 'Ice giant',
categories: [ 'Space', 'Ice giant', 'Solar system' ],
satellites: 28,
satellites_no_type: 28,
},
];

Expand Down Expand Up @@ -189,6 +200,12 @@ export const fields = [
{
label: 'Satellites',
id: 'satellites',
type: 'integer',
enableSorting: true,
},
{
label: 'Satellites (no type)',
id: 'satellites_no_type',
enableSorting: true,
},
{
Expand Down
22 changes: 22 additions & 0 deletions packages/dataviews/src/field-types/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* Internal dependencies
*/
import { default as integer } from './integer';
import type { FieldType } from '../types';

/**
*
* @param {FieldType} type The field type definition to get.
*
* @return A field type definition.
*/
export default function getFieldTypeDefinition( type?: FieldType ) {
if ( 'integer' === type ) {
return integer;
}

// If no type found, the sort function doesn't do anything.
Copy link
Member Author

Choose a reason for hiding this comment

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

What do we think of this? What do we do if the type is not found?

return {
sort: () => 0,
};
}
12 changes: 12 additions & 0 deletions packages/dataviews/src/field-types/integer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice file :)

Should we move the "control" to the field types definition?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes! And validation, etc. I'll do that in a follow-up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Validation moved to the type definition at #64164

* Internal dependencies
*/
import type { SortDirection } from '../types';

function sort( a: any, b: any, direction: SortDirection ) {
return direction === 'asc' ? a - b : b - a;
}

export default {
sort,
};
9 changes: 9 additions & 0 deletions packages/dataviews/src/filter-and-sort-data-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,15 @@ export function filterSortAndPaginate< Item >(
const valueA = fieldToSort.getValue( { item: a } ) ?? '';
const valueB = fieldToSort.getValue( { item: b } ) ?? '';

if ( fieldToSort.type === 'integer' ) {
return fieldToSort.sort(
oandregal marked this conversation as resolved.
Show resolved Hide resolved
a,
b,
view.sort?.direction ?? 'desc'
);
}

// When/if types become required, we can remove the following logic.
if (
typeof valueA === 'number' &&
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that I've left the existing logic here because type is optional. When/If make it required this could be removed.

typeof valueB === 'number'
Expand Down
14 changes: 14 additions & 0 deletions packages/dataviews/src/normalize-fields.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* Internal dependencies
*/
import getFieldTypeDefinition from './field-types';
import type { Field, NormalizedField, ItemRecord } from './types';

/**
Expand All @@ -13,15 +14,28 @@ export function normalizeFields< Item >(
fields: Field< Item >[]
): NormalizedField< Item >[] {
return fields.map( ( field ) => {
const fieldTypeDefinition = getFieldTypeDefinition( field.type );

const getValue =
field.getValue ||
( ( { item }: { item: ItemRecord } ) => item[ field.id ] );

const sort =
field.sort ??
function sort( a, b, direction ) {
return fieldTypeDefinition.sort(
getValue( { item: a } ),
getValue( { item: b } ),
direction
);
};

return {
...field,
label: field.label || field.id,
getValue,
render: field.render || getValue,
sort,
};
} );
}
15 changes: 15 additions & 0 deletions packages/dataviews/src/test/filter-and-sort-data-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,21 @@ describe( 'sorting', () => {
} );

it( 'should sort by number', () => {
const { data: result } = filterSortAndPaginate(
data,
{
sort: { field: 'satellites_no_type', direction: 'desc' },
},
fields
);

expect( result ).toHaveLength( 11 );
expect( result[ 0 ].title ).toBe( 'Saturn' );
expect( result[ 1 ].title ).toBe( 'Jupiter' );
expect( result[ 2 ].title ).toBe( 'Uranus' );
} );

it( 'should sort by type integer', () => {
const { data: result } = filterSortAndPaginate(
data,
{
Expand Down
6 changes: 6 additions & 0 deletions packages/dataviews/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ export type Field< Item > = {
*/
render?: ComponentType< { item: Item } >;

/**
* Callback used to sort the field.
*/
sort?: ( a: Item, b: Item, direction: SortDirection ) => number;

/**
* Whether the field is sortable.
*/
Expand Down Expand Up @@ -124,6 +129,7 @@ export type NormalizedField< Item > = Field< Item > & {
label: string;
getValue: ( args: { item: Item } ) => any;
render: ComponentType< { item: Item } >;
sort: ( a: Item, b: Item, direction: SortDirection ) => number;
};

/**
Expand Down
8 changes: 8 additions & 0 deletions packages/edit-site/src/components/post-fields/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,14 @@ function usePostFields( viewType ) {
label: name,
} ) ) || [],
render: PostAuthorField,
sort: ( a, b, direction ) => {
oandregal marked this conversation as resolved.
Show resolved Hide resolved
const nameA = a._embedded?.author?.[ 0 ]?.name || '';
const nameB = b._embedded?.author?.[ 0 ]?.name || '';

return direction === 'asc'
? nameA.localeCompare( nameB )
: nameB.localeCompare( nameA );
},
},
{
label: __( 'Status' ),
Expand Down
26 changes: 20 additions & 6 deletions packages/edit-site/src/components/post-list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
import { useState, useMemo, useCallback, useEffect } from '@wordpress/element';
import { privateApis as routerPrivateApis } from '@wordpress/router';
import { useSelect, useDispatch } from '@wordpress/data';
import { DataViews } from '@wordpress/dataviews';
import { DataViews, filterSortAndPaginate } from '@wordpress/dataviews';
import { privateApis as editorPrivateApis } from '@wordpress/editor';
import { __ } from '@wordpress/i18n';
import { drawerRight } from '@wordpress/icons';
Expand Down Expand Up @@ -161,6 +161,8 @@ export default function PostList( { postType } ) {
[ history ]
);

const { isLoading: isLoadingFields, fields } = usePostFields( view.type );

const queryArgs = useMemo( () => {
const filters = {};
view.filters.forEach( ( filter ) => {
Expand Down Expand Up @@ -200,12 +202,25 @@ export default function PostList( { postType } ) {
}, [ view ] );
const {
records,
isResolving: isLoadingMainEntities,
isResolving: isLoadingData,
totalItems,
totalPages,
} = useEntityRecordsWithPermissions( 'postType', postType, queryArgs );

const ids = records?.map( ( record ) => getItemId( record ) ) ?? [];
// The REST API sort the authors by ID, but we want to sort them by name.
const data = useMemo( () => {
if ( ! isLoadingFields && view?.sort?.field === 'author' ) {
return filterSortAndPaginate(
records,
{ sort: { ...view.sort } },
fields
).data;
}

return records;
}, [ records, fields, isLoadingFields, view?.sort ] );

const ids = data?.map( ( record ) => getItemId( record ) ) ?? [];
const prevIds = usePrevious( ids ) ?? [];
const deletedIds = prevIds.filter( ( id ) => ! ids.includes( id ) );
const postIdWasDeleted = deletedIds.includes( postId );
Expand Down Expand Up @@ -263,7 +278,6 @@ export default function PostList( { postType } ) {
} );
closeModal();
};
const { isLoading: isLoadingFields, fields } = usePostFields( view.type );

return (
<Page
Expand Down Expand Up @@ -294,8 +308,8 @@ export default function PostList( { postType } ) {
paginationInfo={ paginationInfo }
fields={ fields }
actions={ actions }
data={ records || EMPTY_ARRAY }
isLoading={ isLoadingMainEntities || isLoadingFields }
data={ data || EMPTY_ARRAY }
isLoading={ isLoadingData || isLoadingFields }
view={ view }
onChangeView={ setView }
selection={ selection }
Expand Down
Loading