From 3f25916114a67b20c7d4550d4ffd15563f613bb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Tue, 19 Dec 2023 12:43:53 +0100 Subject: [PATCH] DataViews: align filter implementations (#57059) --- packages/dataviews/src/add-filter.js | 134 ++++++++++------------- packages/dataviews/src/filter-summary.js | 77 ++++++------- packages/dataviews/src/view-table.js | 112 +++++++++---------- 3 files changed, 147 insertions(+), 176 deletions(-) diff --git a/packages/dataviews/src/add-filter.js b/packages/dataviews/src/add-filter.js index 8df218c060370..c403baa9c13d3 100644 --- a/packages/dataviews/src/add-filter.js +++ b/packages/dataviews/src/add-filter.js @@ -73,6 +73,9 @@ export default function AddFilter( { filters, view, onChangeView } ) { const filterInView = view.filters.find( ( f ) => f.field === filter.field ); + const otherFilters = view.filters.filter( + ( f ) => f.field !== filter.field + ); const activeElement = filter.elements.find( ( element ) => element.value === filterInView?.value ); @@ -107,50 +110,45 @@ export default function AddFilter( { filters, view, onChangeView } ) { > - { filter.elements.map( ( element ) => ( - - ) - } - onSelect={ ( event ) => { - event.preventDefault(); - onChangeView( - ( currentView ) => ( { - ...currentView, + { filter.elements.map( ( element ) => { + const isActive = + activeElement?.value === + element.value; + return ( + + ) + } + onSelect={ ( event ) => { + event.preventDefault(); + onChangeView( { + ...view, page: 1, filters: [ - ...currentView.filters.filter( - ( f ) => - f.field !== - filter.field - ), + ...otherFilters, { field: filter.field, operator: activeOperator, - value: - activeElement?.value === - element.value - ? undefined - : element.value, + value: isActive + ? undefined + : element.value, }, ], - } ) - ); - } } - > - { element.label } - - ) ) } + } ); + } } + > + { element.label } + + ); + } ) } { filter.operators.length > 1 && ( { event.preventDefault(); - onChangeView( - ( currentView ) => ( { - ...currentView, - page: 1, - filters: [ - ...view.filters.filter( - ( f ) => - f.field !== - filter.field - ), - { - field: filter.field, - operator: - OPERATOR_IN, - value: filterInView?.value, - }, - ], - } ) - ); + onChangeView( { + ...view, + page: 1, + filters: [ + ...otherFilters, + { + field: filter.field, + operator: + OPERATOR_IN, + value: filterInView?.value, + }, + ], + } ); } } > { __( 'Is' ) } @@ -229,25 +221,19 @@ export default function AddFilter( { filters, view, onChangeView } ) { } onSelect={ ( event ) => { event.preventDefault(); - onChangeView( - ( currentView ) => ( { - ...currentView, - page: 1, - filters: [ - ...view.filters.filter( - ( f ) => - f.field !== - filter.field - ), - { - field: filter.field, - operator: - OPERATOR_NOT_IN, - value: filterInView?.value, - }, - ], - } ) - ); + onChangeView( { + ...view, + page: 1, + filters: [ + ...otherFilters, + { + field: filter.field, + operator: + OPERATOR_NOT_IN, + value: filterInView?.value, + }, + ], + } ); } } > { __( 'Is not' ) } diff --git a/packages/dataviews/src/filter-summary.js b/packages/dataviews/src/filter-summary.js index 3c30c6837103a..098e73db5eeb3 100644 --- a/packages/dataviews/src/filter-summary.js +++ b/packages/dataviews/src/filter-summary.js @@ -74,9 +74,13 @@ function WithSeparators( { children } ) { export default function FilterSummary( { filter, view, onChangeView } ) { const filterInView = view.filters.find( ( f ) => f.field === filter.field ); + const otherFilters = view.filters.filter( + ( f ) => f.field !== filter.field + ); const activeElement = filter.elements.find( ( element ) => element.value === filterInView?.value ); + const activeOperator = filterInView?.operator || filter.operators[ 0 ]; return ( { filter.elements.map( ( element ) => { + const isActive = activeElement?.value === element.value; return ( - ) - } + aria-checked={ isActive } + prefix={ isActive && } onSelect={ () => - onChangeView( ( currentView ) => ( { - ...currentView, + onChangeView( { + ...view, page: 1, filters: [ - ...view.filters.filter( - ( f ) => - f.field !== filter.field - ), + ...otherFilters, { field: filter.field, - operator: - filterInView?.operator || - filter.operators[ 0 ], - value: - activeElement?.value === - element.value - ? undefined - : element.value, + operator: activeOperator, + value: isActive + ? undefined + : element.value, }, ], - } ) ) + } ) } > { element.label } @@ -142,9 +134,10 @@ export default function FilterSummary( { filter, view, onChangeView } ) { - { filterInView.operator === OPERATOR_IN - ? __( 'Is' ) - : __( 'Is not' ) } + { activeOperator === OPERATOR_IN && + __( 'Is' ) } + { activeOperator === OPERATOR_NOT_IN && + __( 'Is not' ) } { ' ' } } @@ -156,29 +149,25 @@ export default function FilterSummary( { filter, view, onChangeView } ) { ) } onSelect={ () => - onChangeView( ( currentView ) => ( { - ...currentView, + onChangeView( { + ...view, page: 1, filters: [ - ...view.filters.filter( - ( f ) => f.field !== filter.field - ), + ...otherFilters, { field: filter.field, operator: OPERATOR_IN, value: filterInView?.value, }, ], - } ) ) + } ) } > { __( 'Is' ) } @@ -186,29 +175,25 @@ export default function FilterSummary( { filter, view, onChangeView } ) { ) } onSelect={ () => - onChangeView( ( currentView ) => ( { - ...currentView, + onChangeView( { + ...view, page: 1, filters: [ - ...view.filters.filter( - ( f ) => f.field !== filter.field - ), + ...otherFilters, { field: filter.field, operator: OPERATOR_NOT_IN, value: filterInView?.value, }, ], - } ) ) + } ) } > { __( 'Is not' ) } diff --git a/packages/dataviews/src/view-table.js b/packages/dataviews/src/view-table.js index c5323bebea0ef..e08449b76491d 100644 --- a/packages/dataviews/src/view-table.js +++ b/packages/dataviews/src/view-table.js @@ -40,32 +40,36 @@ const sortingItemsInfo = { }; const sortArrows = { asc: '↑', desc: '↓' }; +const sanitizeOperators = ( field ) => { + 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 ) + ); +}; + function HeaderMenu( { field, view, onChangeView } ) { - const isSortable = field.enableSorting !== false; const isHidable = field.enableHiding !== false; + + const isSortable = field.enableSorting !== false; const isSorted = view.sort?.field === field.id; - let filter, filterInView; - const otherFilters = []; - if ( field.type === ENUMERATION_TYPE ) { - let columnOperators = field.filterBy?.operators; - if ( ! columnOperators || ! Array.isArray( columnOperators ) ) { - columnOperators = [ OPERATOR_IN, OPERATOR_NOT_IN ]; - } - const operators = columnOperators.filter( ( operator ) => - [ OPERATOR_IN, OPERATOR_NOT_IN ].includes( operator ) + + let filter, filterInView, activeElement, activeOperator, otherFilters; + const operators = sanitizeOperators( field ); + if ( field.type === ENUMERATION_TYPE && operators.length > 0 ) { + filter = { + field: field.id, + operators, + elements: field.elements || [], + }; + filterInView = view.filters.find( ( f ) => f.field === filter.field ); + otherFilters = view.filters.filter( ( f ) => f.field !== filter.field ); + activeElement = filter.elements.find( + ( element ) => element.value === filterInView?.value ); - if ( operators.length > 0 ) { - filter = { - field: field.id, - operators, - elements: field.elements || [], - }; - filterInView = { - field: filter.field, - operator: filter.operators[ 0 ], - value: undefined, - }; - } + activeOperator = filterInView?.operator || filter.operators[ 0 ]; } const isFilterable = !! filter; @@ -73,18 +77,6 @@ function HeaderMenu( { field, view, onChangeView } ) { return field.header; } - if ( isFilterable ) { - const columnFilters = view.filters; - columnFilters.forEach( ( columnFilter ) => { - if ( columnFilter.field === filter.field ) { - filterInView = { - ...columnFilter, - }; - } else { - otherFilters.push( columnFilter ); - } - } ); - } return ( } suffix={ - + <> + { activeElement && + activeOperator === + OPERATOR_IN && + __( 'Is' ) } + { activeElement && + activeOperator === + OPERATOR_NOT_IN && + __( 'Is not' ) } + { activeElement && ' ' } + { activeElement?.label } + + } > { __( 'Filter by' ) } @@ -174,17 +178,9 @@ function HeaderMenu( { field, view, onChangeView } ) { { filter.elements.map( ( element ) => { - let isActive = false; - if ( filterInView ) { - // Intentionally use loose comparison, so it does type conversion. - // This covers the case where a top-level filter for the same field converts a number into a string. - /* eslint-disable eqeqeq */ - isActive = - element.value == - filterInView.value; - /* eslint-enable eqeqeq */ - } - + const isActive = + activeElement?.value === + element.value; return ( { onChangeView( { ...view, + page: 1, filters: [ ...otherFilters, { field: filter.field, operator: - filterInView?.operator, + activeOperator, value: isActive ? undefined : element.value, @@ -223,10 +220,12 @@ function HeaderMenu( { field, view, onChangeView } ) { - { filterInView.operator === - OPERATOR_IN - ? __( 'Is' ) - : __( 'Is not' ) } + { activeOperator === + OPERATOR_IN && + __( 'Is' ) } + { activeOperator === + OPERATOR_NOT_IN && + __( 'Is not' ) } ) @@ -255,6 +253,7 @@ function HeaderMenu( { field, view, onChangeView } ) { onSelect={ () => onChangeView( { ...view, + page: 1, filters: [ ...otherFilters, { @@ -273,11 +272,11 @@ function HeaderMenu( { field, view, onChangeView } ) { key="not-in-filter" role="menuitemradio" aria-checked={ - filterInView?.operator === + activeOperator === OPERATOR_NOT_IN } prefix={ - filterInView?.operator === + activeOperator === OPERATOR_NOT_IN && ( ) @@ -285,6 +284,7 @@ function HeaderMenu( { field, view, onChangeView } ) { onSelect={ () => onChangeView( { ...view, + page: 1, filters: [ ...otherFilters, {