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: add support for NOT IN operator in filter #56479

Merged
merged 16 commits into from
Dec 4, 2023
Merged
2 changes: 2 additions & 0 deletions packages/dataviews/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ Example:
- `type`: the type of the field. Used to generate the proper filters. Only `enumeration` available at the moment.
- `enableSorting`: whether the data can be sorted by the given field. True by default.
- `enableHiding`: whether the field can be hidden. True by default.
- `filterBy`: configuration for the filters.
- `operators`: the list of operators supported by the field.

## Actions

Expand Down
3 changes: 1 addition & 2 deletions packages/dataviews/src/add-filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ export default function AddFilter( { fields, view, onChangeView } ) {
name: field.header,
elements: field.elements || [],
isVisible: view.filters.some(
( f ) =>
f.field === field.id && f.operator === OPERATOR_IN
( f ) => f.field === field.id
),
} );
}
Expand Down
1 change: 1 addition & 0 deletions packages/dataviews/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const ENUMERATION_TYPE = 'enumeration';

// Filter operators.
export const OPERATOR_IN = 'in';
export const OPERATOR_NOT_IN = 'notIn';

// View layouts.
export const LAYOUT_TABLE = 'table';
Expand Down
222 changes: 182 additions & 40 deletions packages/dataviews/src/filter-summary.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,72 @@ import {
privateApis as componentsPrivateApis,
Icon,
} from '@wordpress/components';
import { chevronDown } from '@wordpress/icons';
import { chevronDown, chevronRightSmall, check } from '@wordpress/icons';
import { __, sprintf } from '@wordpress/i18n';
import { Children, Fragment } from '@wordpress/element';

/**
* Internal dependencies
*/
import { OPERATOR_IN } from './constants';
import { OPERATOR_IN, OPERATOR_NOT_IN } from './constants';
import { unlock } from './lock-unlock';

const {
DropdownMenuV2: DropdownMenu,
DropdownMenuCheckboxItemV2: DropdownMenuCheckboxItem,
DropdownMenuGroupV2: DropdownMenuGroup,
DropdownMenuItemV2: DropdownMenuItem,
DropdownMenuSeparatorV2: DropdownMenuSeparator,
DropdownSubMenuV2: DropdownSubMenu,
DropdownSubMenuTriggerV2: DropdownSubMenuTrigger,
} = unlock( componentsPrivateApis );

const FilterText = ( { activeElement, filterInView, filter } ) => {
if ( activeElement === undefined ) {
return filter.name;
}

if (
activeElement !== undefined &&
filterInView?.operator === OPERATOR_IN
) {
return sprintf(
/* translators: 1: Filter name. 2: Filter value. e.g.: "Author is Admin". */
__( '%1$s is %2$s' ),
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder how more synthetic languages are handled in this sort of UI (e.g. Notion). I'm not discouraging this bit of string interpolation, just pointing out that we should see how others have done this, so that later we are sure that this UI is localisable.

Copy link
Member Author

Choose a reason for hiding this comment

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

At some point, the proposal was Field: value. Though it didn't scale well when you have more operators (in, not in, starts with, etc.).

This is how notion does it (suffers from the same issue):

Gravacao.do.ecra.2023-12-14.as.10.14.24.mov

This is linear (same issue):

Gravacao.do.ecra.2023-12-14.as.10.16.52.mov

The same in other platforms (github, etc.).

I find synthetic languages difficult for this use case because the individual parts (field name, value) are translated separately, and you have no context what they are when translating this string as a whole. Happy to look at examples to improve it.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is German:

Gravacao.do.ecra.2023-12-14.as.10.25.31.mov

The gender does affect other parts of the sentence, though the general structure is the same – I'd think what we have here is fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for having a look! Yeah, I think it's doable, and at worst there might be a handful of languages in which the UI reads slightly awkwardly but remains understandable.

filter.name,
activeElement.label
);
}

if (
activeElement !== undefined &&
filterInView?.operator === OPERATOR_NOT_IN
) {
return sprintf(
/* translators: 1: Filter name. 2: Filter value. e.g.: "Author is not Admin". */
__( '%1$s is not %2$s' ),
filter.name,
activeElement.label
);
}

return sprintf(
/* translators: 1: Filter name e.g.: "Unknown status for Author". */
__( 'Unknown status for %1$s' ),
filter.name
);
};

function WithSeparators( { children } ) {
return Children.toArray( children )
.filter( Boolean )
.map( ( child, i ) => (
<Fragment key={ i }>
{ i > 0 && <DropdownMenuSeparator /> }
{ child }
</Fragment>
) );
}

export default function FilterSummary( { filter, view, onChangeView } ) {
const filterInView = view.filters.find( ( f ) => f.field === filter.field );
const activeElement = filter.elements.find(
Expand All @@ -31,49 +83,139 @@ export default function FilterSummary( { filter, view, onChangeView } ) {
key={ filter.field }
trigger={
<Button variant="tertiary" size="compact" label={ filter.name }>
{ activeElement !== undefined
? sprintf(
/* translators: 1: Filter name. 2: filter value. e.g.: "Author is Admin". */
__( '%1$s is %2$s' ),
filter.name,
activeElement.label
)
: filter.name }
<FilterText
activeElement={ activeElement }
filterInView={ filterInView }
filter={ filter }
/>
<Icon icon={ chevronDown } style={ { flexShrink: 0 } } />
</Button>
}
>
{ filter.elements.map( ( element ) => {
return (
<DropdownMenuCheckboxItem
key={ element.value }
value={ element.value }
checked={ activeElement?.value === element.value }
onSelect={ () =>
onChangeView( ( currentView ) => ( {
...currentView,
page: 1,
filters: [
...view.filters.filter(
( f ) => f.field !== filter.field
),
{
field: filter.field,
operator: OPERATOR_IN,
value:
activeElement?.value ===
element.value
? undefined
: element.value,
},
],
} ) )
<WithSeparators>
<DropdownMenuGroup>
{ filter.elements.map( ( element ) => {
return (
<DropdownMenuItem
key={ element.value }
role="menuitemradio"
aria-checked={
activeElement?.value === element.value
}
prefix={
activeElement?.value === element.value && (
<Icon icon={ check } />
)
}
onSelect={ () =>
onChangeView( ( currentView ) => ( {
...currentView,
page: 1,
filters: [
...view.filters.filter(
( f ) =>
f.field !== filter.field
),
{
field: filter.field,
operator:
filterInView?.operator ||
filter.operators[ 0 ],
value:
activeElement?.value ===
element.value
? undefined
: element.value,
},
],
} ) )
}
>
{ element.label }
</DropdownMenuItem>
);
} ) }
</DropdownMenuGroup>
{ filter.operators.length > 1 && (
<DropdownSubMenu
trigger={
<DropdownSubMenuTrigger
suffix={
<>
{ filterInView.operator === OPERATOR_IN
? __( 'Is' )
: __( 'Is not' ) }
<Icon icon={ chevronRightSmall } />{ ' ' }
</>
}
>
{ __( 'Conditions' ) }
</DropdownSubMenuTrigger>
}
>
{ element.label }
</DropdownMenuCheckboxItem>
);
} ) }
<DropdownMenuItem
key="in-filter"
role="menuitemradio"
aria-checked={
filterInView?.operator === OPERATOR_IN
}
prefix={
filterInView?.operator === OPERATOR_IN && (
<Icon icon={ check } />
)
}
onSelect={ () =>
onChangeView( ( currentView ) => ( {
...currentView,
page: 1,
filters: [
...view.filters.filter(
( f ) => f.field !== filter.field
),
{
field: filter.field,
operator: OPERATOR_IN,
value: filterInView?.value,
},
],
} ) )
}
>
{ __( 'Is' ) }
</DropdownMenuItem>
<DropdownMenuItem
key="not-in-filter"
role="menuitemradio"
aria-checked={
filterInView?.operator === OPERATOR_NOT_IN
}
prefix={
filterInView?.operator === OPERATOR_NOT_IN && (
<Icon icon={ check } />
)
}
onSelect={ () =>
onChangeView( ( currentView ) => ( {
...currentView,
page: 1,
filters: [
...view.filters.filter(
( f ) => f.field !== filter.field
),
{
field: filter.field,
operator: OPERATOR_NOT_IN,
value: filterInView?.value,
},
],
} ) )
}
>
{ __( 'Is not' ) }
</DropdownMenuItem>
</DropdownSubMenu>
) }
</WithSeparators>
</DropdownMenu>
);
}
23 changes: 21 additions & 2 deletions packages/dataviews/src/filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,17 @@
import FilterSummary from './filter-summary';
import AddFilter from './add-filter';
import ResetFilters from './reset-filters';
import { ENUMERATION_TYPE, OPERATOR_IN } from './constants';
import { ENUMERATION_TYPE, OPERATOR_IN, OPERATOR_NOT_IN } from './constants';

const operatorsFromField = ( field ) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if a better name might be sanitizeOperators.

Copy link
Member Author

Choose a reason for hiding this comment

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

let operators = field.filterBy?.operators;
if ( ! operators || ! Array.isArray( operators ) ) {
operators = [ OPERATOR_IN, OPERATOR_NOT_IN ];
}
return operators.filter( ( operator ) =>
[ OPERATOR_IN, OPERATOR_NOT_IN ].includes( operator )
);
};

export default function Filters( { fields, view, onChangeView } ) {
const filters = [];
Expand All @@ -13,15 +23,24 @@ export default function Filters( { fields, view, onChangeView } ) {
return;
}

const operators = operatorsFromField( field );
if ( operators.length === 0 ) {
return;
}

switch ( field.type ) {
case ENUMERATION_TYPE:
filters.push( {
field: field.id,
name: field.header,
elements: field.elements || [],
operators,
isVisible: view.filters.some(
( f ) =>
f.field === field.id && f.operator === OPERATOR_IN
f.field === field.id &&
[ OPERATOR_IN, OPERATOR_NOT_IN ].includes(
f.operator
)
),
} );
}
Expand Down
1 change: 1 addition & 0 deletions packages/dataviews/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ export {
LAYOUT_LIST,
ENUMERATION_TYPE,
OPERATOR_IN,
OPERATOR_NOT_IN,
} from './constants';
Loading
Loading