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: Type all the filters components #61795

Merged
merged 2 commits into from
May 21, 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
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* External dependencies
*/
import type { Ref } from 'react';

/**
* WordPress dependencies
*/
Expand All @@ -12,14 +17,25 @@ import { forwardRef } from '@wordpress/element';
* Internal dependencies
*/
import { unlock } from './lock-unlock';
import type { NormalizedFilter, View } from './types';

const {
DropdownMenuV2: DropdownMenu,
DropdownMenuItemV2: DropdownMenuItem,
DropdownMenuItemLabelV2: DropdownMenuItemLabel,
} = unlock( componentsPrivateApis );

function AddFilter( { filters, view, onChangeView, setOpenedFilter }, ref ) {
interface AddFilterProps {
filters: NormalizedFilter[];
view: View;
onChangeView: ( view: View ) => void;
setOpenedFilter: ( filter: string | null ) => void;
}

function AddFilter(
{ filters, view, onChangeView, setOpenedFilter }: AddFilterProps,
ref: Ref< HTMLButtonElement >
) {
if ( ! filters.length || filters.every( ( { isPrimary } ) => isPrimary ) ) {
return null;
}
Expand Down
7 changes: 6 additions & 1 deletion packages/dataviews/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
*/
import { __ } from '@wordpress/i18n';

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

// Filter operators.
Copy link
Member

Choose a reason for hiding this comment

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

I wrote a big comment about how we can rely more on the type system and make a lot of these constants irrelevant without sacrificing clarity or readability (in my opinion).

Instead of writing the comment, I pushed a change that implements that idea. Please feel free to keep it or revert it, but it's a powerful concept and a compelling way to rely on the information the type system provides.

export const OPERATOR_IS = 'is';
export const OPERATOR_IS_NOT = 'isNot';
Expand All @@ -19,7 +24,7 @@ export const ALL_OPERATORS = [
OPERATOR_IS_ALL,
OPERATOR_IS_NOT_ALL,
];
export const OPERATORS = {
export const OPERATORS: Record< Operator, { key: string; label: string } > = {
[ OPERATOR_IS ]: {
key: 'is-filter',
label: __( 'Is' ),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import clsx from 'clsx';
import type { RefObject } from 'react';

/**
* WordPress dependencies
Expand Down Expand Up @@ -35,8 +36,30 @@ import {
OPERATOR_IS_ALL,
OPERATOR_IS_NOT_ALL,
} from './constants';
import type { Filter, NormalizedFilter, Operator, Option, View } from './types';

const FilterText = ( { activeElements, filterInView, filter } ) => {
interface FilterTextProps {
activeElements: Option[];
filterInView?: Filter;
filter: NormalizedFilter;
}

interface OperatorSelectorProps {
filter: NormalizedFilter;
view: View;
onChangeView: ( view: View ) => void;
}

interface FilterSummaryProps extends OperatorSelectorProps {
addFilterRef: RefObject< HTMLButtonElement >;
openedFilter: string | null;
}

const FilterText = ( {
activeElements,
filterInView,
filter,
}: FilterTextProps ) => {
if ( activeElements === undefined || activeElements.length === 0 ) {
return filter.name;
}
Expand Down Expand Up @@ -125,7 +148,11 @@ const FilterText = ( { activeElements, filterInView, filter } ) => {
);
};

function OperatorSelector( { filter, view, onChangeView } ) {
function OperatorSelector( {
filter,
view,
onChangeView,
}: OperatorSelectorProps ) {
const operatorOptions = filter.operators?.map( ( operator ) => ( {
value: operator,
label: OPERATORS[ operator ]?.label,
Expand All @@ -150,13 +177,14 @@ function OperatorSelector( { filter, view, onChangeView } ) {
value={ value }
options={ operatorOptions }
onChange={ ( newValue ) => {
const operator = newValue as Operator;
const newFilters = currentFilter
? [
...view.filters.map( ( _filter ) => {
if ( _filter.field === filter.field ) {
return {
..._filter,
operator: newValue,
operator,
};
}
return _filter;
Expand All @@ -166,7 +194,8 @@ function OperatorSelector( { filter, view, onChangeView } ) {
...view.filters,
{
field: filter.field,
operator: newValue,
operator,
value: undefined,
},
];
onChangeView( {
Expand All @@ -188,8 +217,8 @@ export default function FilterSummary( {
addFilterRef,
openedFilter,
...commonProps
} ) {
const toggleRef = useRef();
}: FilterSummaryProps ) {
const toggleRef = useRef< HTMLDivElement >( null );
const { filter, view, onChangeView } = commonProps;
const filterInView = view.filters.find( ( f ) => f.field === filter.field );
const activeElements = filter.elements.filter( ( element ) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* WordPress dependencies
*/
import { memo, useRef } from '@wordpress/element';
import { __experimentalHStack as HStack } from '@wordpress/components';

/**
* Internal dependencies
Expand All @@ -11,17 +12,25 @@ import AddFilter from './add-filter';
import ResetFilters from './reset-filters';
import { sanitizeOperators } from './utils';
import { ALL_OPERATORS, OPERATOR_IS, OPERATOR_IS_NOT } from './constants';
import { __experimentalHStack as HStack } from '@wordpress/components';
import type { AnyItem, NormalizedField, NormalizedFilter, View } from './types';

interface FiltersProps< Item extends AnyItem > {
fields: NormalizedField< Item >[];
view: View;
onChangeView: ( view: View ) => void;
openedFilter: string | null;
setOpenedFilter: ( openedFilter: string | null ) => void;
}

const Filters = memo( function Filters( {
const Filters = memo( function Filters< Item extends AnyItem >( {
fields,
view,
onChangeView,
openedFilter,
setOpenedFilter,
} ) {
const addFilterRef = useRef();
const filters = [];
}: FiltersProps< Item > ) {
const addFilterRef = useRef< HTMLButtonElement >( null );
const filters: NormalizedFilter[] = [];
fields.forEach( ( field ) => {
if ( ! field.elements?.length ) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,23 @@
import { Button } from '@wordpress/components';
import { __ } from '@wordpress/i18n';

export default function ResetFilter( { filters, view, onChangeView } ) {
const isPrimary = ( field ) =>
/**
* Internal dependencies
*/
import type { NormalizedFilter, View } from './types';

interface ResetFilterProps {
filters: NormalizedFilter[];
view: View;
onChangeView: ( view: View ) => void;
}

export default function ResetFilter( {
filters,
view,
onChangeView,
}: ResetFilterProps ) {
const isPrimary = ( field: string ) =>
filters.some(
( _filter ) => _filter.field === field && _filter.isPrimary
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,20 @@ import { SVG, Circle } from '@wordpress/primitives';
* Internal dependencies
*/
import { unlock } from './lock-unlock';
import type { Filter, NormalizedFilter, View } from './types';

const {
CompositeV2: Composite,
CompositeItemV2: CompositeItem,
useCompositeStoreV2: useCompositeStore,
} = unlock( componentsPrivateApis );

interface SearchWidgetProps {
view: View;
filter: NormalizedFilter;
onChangeView: ( view: View ) => void;
}

const radioCheck = (
<SVG xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24">
<Circle cx={ 12 } cy={ 12 } r={ 3 }></Circle>
Expand All @@ -39,8 +46,11 @@ function normalizeSearchInput( input = '' ) {
return removeAccents( input.trim().toLowerCase() );
}

const EMPTY_ARRAY = [];
const getCurrentValue = ( filterDefinition, currentFilter ) => {
const EMPTY_ARRAY: [] = [];
const getCurrentValue = (
filterDefinition: NormalizedFilter,
currentFilter?: Filter
) => {
if ( filterDefinition.singleSelection ) {
return currentFilter?.value;
}
Expand All @@ -56,7 +66,11 @@ const getCurrentValue = ( filterDefinition, currentFilter ) => {
return EMPTY_ARRAY;
};

const getNewValue = ( filterDefinition, currentFilter, value ) => {
const getNewValue = (
filterDefinition: NormalizedFilter,
currentFilter: Filter | undefined,
value: any
) => {
if ( filterDefinition.singleSelection ) {
return value;
}
Expand All @@ -70,7 +84,7 @@ const getNewValue = ( filterDefinition, currentFilter, value ) => {
return [ value ];
};

function ListBox( { view, filter, onChangeView } ) {
function ListBox( { view, filter, onChangeView }: SearchWidgetProps ) {
const compositeStore = useCompositeStore( {
virtualFocus: true,
focusLoop: true,
Expand Down Expand Up @@ -184,7 +198,7 @@ function ListBox( { view, filter, onChangeView } ) {
);
}

function ComboboxList( { view, filter, onChangeView } ) {
function ComboboxList( { view, filter, onChangeView }: SearchWidgetProps ) {
const [ searchValue, setSearchValue ] = useState( '' );
const deferredSearchValue = useDeferredValue( searchValue );
const currentFilter = view.filters.find(
Expand Down Expand Up @@ -234,7 +248,13 @@ function ComboboxList( { view, filter, onChangeView } ) {
setValue={ setSearchValue }
>
<div className="dataviews-search-widget-filter-combobox__wrapper">
<Ariakit.ComboboxLabel render={ <VisuallyHidden /> }>
<Ariakit.ComboboxLabel
render={
<VisuallyHidden>
{ __( 'Search items' ) }
</VisuallyHidden>
}
>
{ __( 'Search items' ) }
</Ariakit.ComboboxLabel>
<Ariakit.Combobox
Expand Down Expand Up @@ -290,7 +310,7 @@ function ComboboxList( { view, filter, onChangeView } ) {
);
}

export default function SearchWidget( props ) {
export default function SearchWidget( props: SearchWidgetProps ) {
const Widget = props.filter.elements.length > 10 ? ComboboxList : ListBox;
return <Widget { ...props } />;
}
46 changes: 41 additions & 5 deletions packages/dataviews/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ export type SortDirection = 'asc' | 'desc';
/**
* Generic option type.
*/
interface Option< Value extends any = any > {
export interface Option< Value extends any = any > {
value: Value;
label: string;
description?: string;
}

interface FilterByConfig {
Expand All @@ -28,15 +29,13 @@ interface FilterByConfig {
isPrimary?: boolean;
}

type DeprecatedOperator = 'in' | 'notIn';
type Operator =
export type Operator =
| 'is'
| 'isNot'
| 'isAny'
| 'isNone'
| 'isAll'
| 'isNotAll'
| DeprecatedOperator;
| 'isNotAll';

export type AnyItem = Record< string, any >;

Expand Down Expand Up @@ -136,6 +135,43 @@ export interface Filter {
value: any;
}

export interface NormalizedFilter {
/**
* The field to filter by.
*/
field: string;

/**
* The field name.
*/
name: string;

/**
* The list of options to pick from when using the field as a filter.
*/
elements: Option[];

/**
* Is a single selection filter.
*/
singleSelection: boolean;

/**
* The list of operators supported by the field.
*/
operators: Operator[];

/**
* Whether the filter is visible.
*/
isVisible: boolean;

/**
* Whether it is a primary filter.
*/
isPrimary: boolean;
}

interface ViewBase {
/**
* The layout of the view.
Expand Down
11 changes: 0 additions & 11 deletions packages/dataviews/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,6 @@ export function sanitizeOperators< Item extends AnyItem >(
operators = [ OPERATOR_IS_ANY, OPERATOR_IS_NONE ];
}

// Transform legacy in, notIn operators to is, isNot.
// To be removed in the future.
if ( operators.includes( 'in' ) ) {
operators = operators.filter( ( operator ) => operator !== 'is' );
operators.push( 'is' );
}
if ( operators.includes( 'notIn' ) ) {
operators = operators.filter( ( operator ) => operator !== 'notIn' );
operators.push( 'isNot' );
}
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 had confirmed previously with @oandregal that these are not used anymore.

Copy link
Member

Choose a reason for hiding this comment

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

I've prepared a PR that adds a changelog note about this, to let consumers of the package know about the breaking changes: #62013


// Make sure only valid operators are used.
operators = operators.filter( ( operator ) =>
ALL_OPERATORS.includes( operator )
Expand Down
Loading