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: Add some client side data handling utils #57488

Merged
merged 6 commits into from
Jan 3, 2024
Merged

Dataviews: Add some client side data handling utils #57488

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
706b378
Dataviews: Add some client side data handling utils
ntsekouras Jan 2, 2024
5d60ac0
rename params `items->data`
ntsekouras Jan 3, 2024
9bd1be1
small fix
ntsekouras Jan 3, 2024
ab5feff
reset the page number when the category changes
ntsekouras Jan 3, 2024
a4c8fac
fix passed text fields
ntsekouras Jan 3, 2024
ea4b2ff
change the whole view when we change category id
ntsekouras Jan 3, 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
1 change: 1 addition & 0 deletions packages/dataviews/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export { default as DataViews } from './dataviews';
export { sortByTextFields, getPaginationResults } from './utils';
export { VIEW_LAYOUTS } from './constants';
51 changes: 51 additions & 0 deletions packages/dataviews/src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/**
* Helper util to sort data by text fields, when sorting is done client side.
*
* @param {Object} params Function params.
* @param {Object[]} params.data Data to sort.
* @param {Object} params.view Current view object.
* @param {Object[]} params.fields Array of available fields.
* @param {string[]} params.textFields Array of the field ids to sort.
*
* @return {Object[]} Sorted data.
*/
export const sortByTextFields = ( { data, view, fields, textFields } ) => {
const sortedData = [ ...data ];
const fieldId = view.sort.field;
if ( textFields.includes( fieldId ) ) {
const fieldToSort = fields.find( ( field ) => {
return field.id === fieldId;
} );
sortedData.sort( ( a, b ) => {
const valueA = fieldToSort.getValue( { item: a } ) ?? '';
const valueB = fieldToSort.getValue( { item: b } ) ?? '';
return view.sort.direction === 'asc'
? valueA.localeCompare( valueB )
: valueB.localeCompare( valueA );
} );
}
return sortedData;
};

/**
* Helper util to get the paginated data and the paginateInfo needed,
* when pagination is done client side.
*
* @param {Object} params Function params.
* @param {Object[]} params.data Available data.
* @param {Object} params.view Current view object.
*
* @return {Object} Paginated data and paginationInfo.
*/
export function getPaginationResults( { data, view } ) {
const start = ( view.page - 1 ) * view.perPage;
const totalItems = data?.length || 0;
data = data?.slice( start, start + view.perPage );
return {
data,
paginationInfo: {
totalItems,
totalPages: Math.ceil( totalItems / view.perPage ),
},
};
}
55 changes: 29 additions & 26 deletions packages/edit-site/src/components/page-patterns/dataviews-patterns.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,22 @@ import {
} from '@wordpress/components';
import { getQueryArgs } from '@wordpress/url';
import { __ } from '@wordpress/i18n';
import { useState, useMemo, useCallback, useId } from '@wordpress/element';
import {
useState,
useMemo,
useCallback,
useId,
useEffect,
} from '@wordpress/element';
import {
BlockPreview,
privateApis as blockEditorPrivateApis,
} from '@wordpress/block-editor';
import { DataViews } from '@wordpress/dataviews';
import {
DataViews,
sortByTextFields,
getPaginationResults,
} from '@wordpress/dataviews';
import {
Icon,
header,
Expand All @@ -24,6 +34,7 @@ import {
symbol,
lockSmall,
} from '@wordpress/icons';
import { usePrevious } from '@wordpress/compose';

/**
* Internal dependencies
Expand Down Expand Up @@ -192,6 +203,7 @@ export default function DataviewsPatterns() {
const [ view, setView ] = useState( DEFAULT_VIEW );
const isUncategorizedThemePatterns =
type === PATTERN_TYPES.theme && categoryId === 'uncategorized';
const previousCategoryId = usePrevious( categoryId );
const { patterns, isResolving } = usePatterns(
type,
isUncategorizedThemePatterns ? '' : categoryId,
Expand Down Expand Up @@ -229,7 +241,12 @@ export default function DataviewsPatterns() {
],
[ view.type, categoryId ]
);

// Reset the page number when the category changes.
useEffect( () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an unrelated fix for a bug that I observed here.

if ( previousCategoryId !== categoryId && view.page !== 1 ) {
setView( { ...view, page: 1 } );
Copy link
Contributor

Choose a reason for hiding this comment

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

Right now, these categories are built as separate pages, so for me we should reset the entire "view" object when changing categories.

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 kind of disagree here because while what you say it's true, it feels like we have external filters in the sidebar and for example if I have sorted by title, I'd expect the other categories to be sorted too. No strong opinions for now though, I can update.

Copy link
Contributor

Choose a reason for hiding this comment

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

No strong opinion too. I don't like this effect though and I'm not sure how to avoid it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my mind this is just a filter that we will need to incorporate in the view object. Current patterns page is WIP and has many more things to think about - refactor/simplify etc.. It's just that is there is a lot of complexity and want to tackle things iteratively.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is also close to the "default views" of the pages data views, it's not really clear whether it's filters or just separate default views.

Copy link
Member

Choose a reason for hiding this comment

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

This is also close to the "default views" of the pages data views, it's not really clear whether it's filters or just separate default views.

The "default views" are just presets, preconfigured settings for the view, so we still need a "category" filter in place.

}
}, [ categoryId, previousCategoryId, view ] );
const { data, paginationInfo } = useMemo( () => {
if ( ! patterns ) {
return {
Expand All @@ -240,32 +257,18 @@ export default function DataviewsPatterns() {
let filteredData = [ ...patterns ];
// Handle sorting.
if ( view.sort ) {
const stringSortingFields = [ 'title' ];
const fieldId = view.sort.field;
if ( stringSortingFields.includes( fieldId ) ) {
const fieldToSort = fields.find( ( field ) => {
return field.id === fieldId;
} );
filteredData.sort( ( a, b ) => {
const valueA = fieldToSort.getValue( { item: a } ) ?? '';
const valueB = fieldToSort.getValue( { item: b } ) ?? '';
return view.sort.direction === 'asc'
? valueA.localeCompare( valueB )
: valueB.localeCompare( valueA );
} );
}
filteredData = sortByTextFields( {
data: filteredData,
view,
fields,
textFields: [ 'title', 'author' ],
} );
}
// Handle pagination.
const start = ( view.page - 1 ) * view.perPage;
const totalItems = filteredData?.length || 0;
filteredData = filteredData?.slice( start, start + view.perPage );
return {
return getPaginationResults( {
data: filteredData,
paginationInfo: {
totalItems,
totalPages: Math.ceil( totalItems / view.perPage ),
},
};
view,
} );
}, [ patterns, view, fields ] );

const actions = useMemo(
Expand Down
50 changes: 18 additions & 32 deletions packages/edit-site/src/components/page-templates/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ import {
BlockPreview,
privateApis as blockEditorPrivateApis,
} from '@wordpress/block-editor';
import { DataViews } from '@wordpress/dataviews';
import {
DataViews,
sortByTextFields,
getPaginationResults,
} from '@wordpress/dataviews';
import { privateApis as routerPrivateApis } from '@wordpress/router';

/**
Expand Down Expand Up @@ -252,10 +256,10 @@ export default function DataviewsTemplates() {
[ authors, view.type ]
);

const { shownTemplates, paginationInfo } = useMemo( () => {
const { data, paginationInfo } = useMemo( () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this hook become a reusable hook that filters and sorts and everything based on "view", "allData" and "fields"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can explore this separately better because I think we need to advance the filters API more and see how we can abstract it best. Right now there is a lot of specific code regarding the filtering(see author in templates).

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't seem specific at all for me. author_text is just getValue and the rest is standard. I'm fine if it's explored separately but I think having something like this is going to help us make the right decisions. And we won't need to expose multiple helpers from the dataviews package only a single hook.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course, eventually we shouldn't have much helpers..

if ( ! allTemplates ) {
return {
shownTemplates: EMPTY_ARRAY,
data: EMPTY_ARRAY,
paginationInfo: { totalItems: 0, totalPages: 0 },
};
}
Expand Down Expand Up @@ -301,36 +305,18 @@ export default function DataviewsTemplates() {

// Handle sorting.
if ( view.sort ) {
const stringSortingFields = [ 'title', 'author' ];
const fieldId = view.sort.field;
if ( stringSortingFields.includes( fieldId ) ) {
const fieldToSort = fields.find( ( field ) => {
return field.id === fieldId;
} );
filteredTemplates.sort( ( a, b ) => {
const valueA = fieldToSort.getValue( { item: a } ) ?? '';
const valueB = fieldToSort.getValue( { item: b } ) ?? '';
return view.sort.direction === 'asc'
? valueA.localeCompare( valueB )
: valueB.localeCompare( valueA );
} );
}
filteredTemplates = sortByTextFields( {
data: filteredTemplates,
view,
fields,
textFields: [ 'title', 'author' ],
} );
}

// Handle pagination.
const start = ( view.page - 1 ) * view.perPage;
const totalItems = filteredTemplates?.length || 0;
filteredTemplates = filteredTemplates?.slice(
start,
start + view.perPage
);
return {
shownTemplates: filteredTemplates,
paginationInfo: {
totalItems,
totalPages: Math.ceil( totalItems / view.perPage ),
},
};
return getPaginationResults( {
data: filteredTemplates,
view,
} );
}, [ allTemplates, view, fields ] );

const resetTemplateAction = useResetTemplateAction();
Expand Down Expand Up @@ -381,7 +367,7 @@ export default function DataviewsTemplates() {
paginationInfo={ paginationInfo }
fields={ fields }
actions={ actions }
data={ shownTemplates }
data={ data }
isLoading={ isLoadingData }
view={ view }
onChangeView={ onChangeView }
Expand Down
Loading