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

Jump to bottom

Dataviews: Improve story #59773

Merged
merged 3 commits into from
Mar 12, 2024
Merged

Dataviews: Improve story #59773

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2bf4386
Dataviews: Improve story
youknowriad Mar 12, 2024
0f24636
filter results
ntsekouras Mar 12, 2024
9e304f9
Apply suggestions from code review
ntsekouras Mar 12, 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
13 changes: 12 additions & 1 deletion packages/dataviews/src/stories/fixtures.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,66 +20,77 @@ export const data = [
title: 'Apollo',
description: 'Apollo description',
image: 'https://live.staticflickr.com/5725/21726228300_51333bd62c_b.jpg',
type: 'Not a planet',
},
{
id: 2,
title: 'Space',
description: 'Space description',
image: 'https://live.staticflickr.com/5678/21911065441_92e2d44708_b.jpg',
type: 'Not a planet',
},
{
id: 3,
title: 'NASA',
description: 'NASA photo',
image: 'https://live.staticflickr.com/742/21712365770_8f70a2c91e_b.jpg',
type: 'Not a planet',
},
{
id: 4,
title: 'Neptune',
description: 'Neptune description',
image: 'https://live.staticflickr.com/5725/21726228300_51333bd62c_b.jpg',
type: 'Ice giant',
},
{
id: 5,
title: 'Mercury',
description: 'Mercury description',
image: 'https://live.staticflickr.com/5725/21726228300_51333bd62c_b.jpg',
type: 'Terrestrial',
},
{
id: 6,
title: 'Venus',
description: 'Venus description',
image: 'https://live.staticflickr.com/5725/21726228300_51333bd62c_b.jpg',
type: 'Terrestrial',
},
{
id: 7,
title: 'Earth',
description: 'Earth description',
image: 'https://live.staticflickr.com/5725/21726228300_51333bd62c_b.jpg',
type: 'Terrestrial',
},
{
id: 8,
title: 'Mars',
description: 'Mars description',
image: 'https://live.staticflickr.com/5725/21726228300_51333bd62c_b.jpg',
type: 'Terrestrial',
},
{
id: 9,
title: 'Jupiter',
description: 'Jupiter description',
image: 'https://live.staticflickr.com/5725/21726228300_51333bd62c_b.jpg',
type: 'Gaz giant',
ntsekouras marked this conversation as resolved.
Show resolved Hide resolved
},
{
id: 10,
title: 'Saturn',
description: 'Saturn description',
image: 'https://live.staticflickr.com/5725/21726228300_51333bd62c_b.jpg',
type: 'Gaz giant',
ntsekouras marked this conversation as resolved.
Show resolved Hide resolved
},
{
id: 11,
title: 'Uranus',
description: 'Uranus description',
image: 'https://live.staticflickr.com/5725/21726228300_51333bd62c_b.jpg',
type: 'Ice giant',
},
];

Expand All @@ -88,7 +99,7 @@ export const DEFAULT_VIEW = {
search: '',
page: 1,
perPage: 10,
hiddenFields: [ 'image' ],
hiddenFields: [ 'image', 'type' ],
layout: {},
filters: [],
};
Expand Down
46 changes: 44 additions & 2 deletions packages/dataviews/src/stories/index.story.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,15 @@ import { useState, useMemo, useCallback } from '@wordpress/element';
*/
import { DataViews } from '../index';
import { DEFAULT_VIEW, actions, data } from './fixtures';
import { LAYOUT_GRID, LAYOUT_TABLE } from '../constants';
import {
LAYOUT_GRID,
LAYOUT_TABLE,
OPERATOR_IS_NONE,
OPERATOR_IS_ANY,
} from '../constants';

const meta = {
title: 'DataViews (Experimental)/DataViews',
title: 'DataViews/DataViews',
component: DataViews,
};
export default meta;
Expand Down Expand Up @@ -49,6 +54,20 @@ const fields = [
maxWidth: 400,
enableHiding: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oandregal I wanted to add a simple "text" field, Like an input where I can type the value or "start with" something ... I was surprised to find out that we don't support that yet and we only support "enumeration".

Copy link
Member

Choose a reason for hiding this comment

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

Yes, all the current implementations use the global search instead. Added it to the backlog and asked for design input.

},
{
header: 'Type',
id: 'type',
getValue: ( { item } ) => item.type,
maxWidth: 400,
enableHiding: false,
type: 'enumeration',
elements: [
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 the filtering happens manually, so this just adds the filter in UI, but not filters results. For client side handling this should probably be absorbed by the package in the future.

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 DataViews component can't know if the filtering is happening client side or server side, which means it can't know if the available values are exhaustive. I'm fine with this being defined on the field personally.

Copy link
Contributor

Choose a reason for hiding this comment

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

I pushed the filtering for now.

{ value: 'Not a planet', label: 'Not a planet' },
{ value: 'Ice giant', label: 'Ice giant' },
{ value: 'Terrestrial', label: 'Terrestrial' },
{ value: 'Gas giant', label: 'Gas giant' },
],
},
{
header: 'Description',
id: 'description',
Expand All @@ -72,6 +91,29 @@ export const Default = ( props ) => {
].some( ( field ) => field.includes( normalizedSearch ) );
} );
}

if ( view.filters.length > 0 ) {
view.filters.forEach( ( filter ) => {
if (
filter.field === 'type' &&
filter.operator === OPERATOR_IS_ANY &&
filter?.value?.length > 0
) {
filteredData = filteredData.filter( ( item ) => {
return filter.value.includes( item.type );
} );
} else if (
filter.field === 'type' &&
filter.operator === OPERATOR_IS_NONE &&
filter?.value?.length > 0
) {
filteredData = filteredData.filter( ( item ) => {
return ! filter.value.includes( item.type );
} );
}
} );
}
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 should build a reusable hook for this, the code will become more complex over time when we add more filter types and operators.


// Handle sorting.
if ( view.sort ) {
const stringSortingFields = [ 'title' ];
Expand Down
2 changes: 1 addition & 1 deletion packages/dataviews/src/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
.dataviews-wrapper {
width: 100%;
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 style makes the assumption that the parent div doesn't have a padding which is not always true.

height: 100%;
overflow: auto;
box-sizing: border-box;
scroll-padding-bottom: $grid-unit-80;
}

.dataviews-filters__view-actions {
box-sizing: border-box;
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 style is present in WordPress "reset.css" but is needed for places like storybook or usage outside of WP. This makes the style more portable.

padding: $grid-unit-15 $grid-unit-40 0;
margin-bottom: $grid-unit-15;
flex-shrink: 0;
Expand Down
Loading