From a3ac1cc19038873bfb72918dc1ad7618fa56f379 Mon Sep 17 00:00:00 2001 From: Jorge Costa Date: Tue, 16 Jul 2024 14:03:42 +0300 Subject: [PATCH 1/2] Remove isShowingFilter props, and use a key approach (includes some debug code). (+15 squashed commits) Squashed commits: [2198f16852] style change [0f23576ae6] hide filters when changing views [fbee4cad46] lint fix [eaaccae54b] end to end test updates [7b58faec1c] make filters toggle at the side of search [2f5a957a47] end to end test updates [9e40065f17] end to end test updare [9fd5ca03b5] lint filters and make the filter button the first [877f13bd54] Count styling [4fc07717be] Reduce spacing, remove negative margin [871ebf4553] Remove negative margin [0d8632a666] lint fix [31d893beb3] post rebase fixes [1d2b4bf1d3] fix don't show all filters [224d0ef7b1] fix number positioning (+3 squashed commits) Squashed commits: [f0acb40a7d] enhanements [309567963c] post rebase style fixes [9ce5f3686a] Remove hstack form search (+2 squashed commits) Squashed commits: [62c7124201] lint fixes [afa224734a] apply feedback --- .../dataviews-filters/add-filter.tsx | 58 +++-- .../components/dataviews-filters/index.tsx | 210 +++++++++++++----- .../components/dataviews-filters/style.scss | 30 +++ .../src/components/dataviews-search/index.tsx | 1 + .../src/components/dataviews/index.tsx | 26 ++- .../src/components/dataviews/style.scss | 7 +- .../src/components/page-patterns/index.js | 1 + .../src/components/page-templates/index.js | 1 + .../src/components/post-list/index.js | 8 +- .../dataviews-list-layout-keyboard.spec.js | 9 - .../site-editor/new-templates-list.spec.js | 2 +- test/e2e/specs/site-editor/patterns.spec.js | 8 + 12 files changed, 259 insertions(+), 102 deletions(-) diff --git a/packages/dataviews/src/components/dataviews-filters/add-filter.tsx b/packages/dataviews/src/components/dataviews-filters/add-filter.tsx index 38dda38c96e70..ca1043d6e99fd 100644 --- a/packages/dataviews/src/components/dataviews-filters/add-filter.tsx +++ b/packages/dataviews/src/components/dataviews-filters/add-filter.tsx @@ -32,29 +32,18 @@ interface AddFilterProps { setOpenedFilter: ( filter: string | null ) => void; } -function AddFilter( - { filters, view, onChangeView, setOpenedFilter }: AddFilterProps, - ref: Ref< HTMLButtonElement > -) { - if ( ! filters.length || filters.every( ( { isPrimary } ) => isPrimary ) ) { - return null; - } +export function AddFilterDropdownMenu( { + filters, + view, + onChangeView, + setOpenedFilter, + trigger, +}: AddFilterProps & { + trigger: React.ReactNode; +} ) { const inactiveFilters = filters.filter( ( filter ) => ! filter.isVisible ); return ( - - { __( 'Add filter' ) } - - } - > + { inactiveFilters.map( ( filter ) => { return ( +) { + if ( ! filters.length || filters.every( ( { isPrimary } ) => isPrimary ) ) { + return null; + } + const inactiveFilters = filters.filter( ( filter ) => ! filter.isVisible ); + return ( + + { __( 'Add filter' ) } + + } + { ...{ filters, view, onChangeView, setOpenedFilter } } + /> + ); +} + export default forwardRef( AddFilter ); diff --git a/packages/dataviews/src/components/dataviews-filters/index.tsx b/packages/dataviews/src/components/dataviews-filters/index.tsx index 449e0ff0323a8..de3477914c008 100644 --- a/packages/dataviews/src/components/dataviews-filters/index.tsx +++ b/packages/dataviews/src/components/dataviews-filters/index.tsx @@ -1,65 +1,150 @@ /** * WordPress dependencies */ -import { memo, useContext, useRef } from '@wordpress/element'; -import { __experimentalHStack as HStack } from '@wordpress/components'; +import { + memo, + useContext, + useRef, + useMemo, + useCallback, +} from '@wordpress/element'; +import { __experimentalHStack as HStack, Button } from '@wordpress/components'; +import { funnel } from '@wordpress/icons'; +import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ import FilterSummary from './filter-summary'; -import AddFilter from './add-filter'; +import { default as AddFilter, AddFilterDropdownMenu } from './add-filter'; import ResetFilters from './reset-filters'; import DataViewsContext from '../dataviews-context'; import { sanitizeOperators } from '../../utils'; import { ALL_OPERATORS, OPERATOR_IS, OPERATOR_IS_NOT } from '../../constants'; -import type { NormalizedFilter } from '../../types'; +import type { NormalizedFilter, NormalizedField, View } from '../../types'; -function Filters() { - const { fields, view, onChangeView, openedFilter, setOpenedFilter } = - useContext( DataViewsContext ); - const addFilterRef = useRef< HTMLButtonElement >( null ); - const filters: NormalizedFilter[] = []; - fields.forEach( ( field ) => { - if ( ! field.elements?.length ) { - return; - } +export function useFilters( fields: NormalizedField< any >[], view: View ) { + return useMemo( () => { + const filters: NormalizedFilter[] = []; + fields.forEach( ( field ) => { + if ( ! field.elements?.length ) { + return; + } - const operators = sanitizeOperators( field ); - if ( operators.length === 0 ) { - return; - } + const operators = sanitizeOperators( field ); + if ( operators.length === 0 ) { + return; + } - const isPrimary = !! field.filterBy?.isPrimary; - filters.push( { - field: field.id, - name: field.label, - elements: field.elements, - singleSelection: operators.some( ( op ) => - [ OPERATOR_IS, OPERATOR_IS_NOT ].includes( op ) - ), - operators, - isVisible: - isPrimary || - !! view.filters?.some( - ( f ) => - f.field === field.id && - ALL_OPERATORS.includes( f.operator ) + const isPrimary = !! field.filterBy?.isPrimary; + filters.push( { + field: field.id, + name: field.label, + elements: field.elements, + singleSelection: operators.some( ( op ) => + [ OPERATOR_IS, OPERATOR_IS_NOT ].includes( op ) ), - isPrimary, + operators, + isVisible: + isPrimary || + !! view.filters?.some( + ( f ) => + f.field === field.id && + ALL_OPERATORS.includes( f.operator ) + ), + isPrimary, + } ); } ); - } ); - // Sort filters by primary property. We need the primary filters to be first. - // Then we sort by name. - filters.sort( ( a, b ) => { - if ( a.isPrimary && ! b.isPrimary ) { - return -1; - } - if ( ! a.isPrimary && b.isPrimary ) { - return 1; - } - return a.name.localeCompare( b.name ); - } ); + // Sort filters by primary property. We need the primary filters to be first. + // Then we sort by name. + filters.sort( ( a, b ) => { + if ( a.isPrimary && ! b.isPrimary ) { + return -1; + } + if ( ! a.isPrimary && b.isPrimary ) { + return 1; + } + return a.name.localeCompare( b.name ); + } ); + return filters; + }, [ fields, view ] ); +} + +export function FilterVisibilityToggle( { + filters, + view, + onChangeView, + setOpenedFilter, + isShowingFilter, + setIsShowingFilter, +}: { + filters: NormalizedFilter[]; + view: View; + onChangeView: ( view: View ) => void; + setOpenedFilter: ( filter: string | null ) => void; + isShowingFilter: boolean; + setIsShowingFilter: React.Dispatch< React.SetStateAction< boolean > >; +} ) { + const onChangeViewWithFilterVisibility = useCallback( + ( _view: View ) => { + onChangeView( _view ); + setIsShowingFilter( true ); + }, + [ onChangeView, setIsShowingFilter ] + ); + const visibleFilters = filters.filter( ( filter ) => filter.isVisible ); + + const hasVisibleFilters = !! visibleFilters.length; + if ( ! hasVisibleFilters ) { + return ( + + } + /> + ); + } + return ( +
+
+ ); +} + +function Filters() { + const { fields, view, onChangeView, openedFilter, setOpenedFilter } = + useContext( DataViewsContext ); + const addFilterRef = useRef< HTMLButtonElement >( null ); + const filters = useFilters( fields, view ); const addFilter = ( ); + const visibleFilters = filters.filter( ( filter ) => filter.isVisible ); + if ( visibleFilters.length === 0 ) { + return null; + } const filterComponents = [ - ...filters.map( ( filter ) => { - if ( ! filter.isVisible ) { - return null; - } - + ...visibleFilters.map( ( filter ) => { return ( 1 ) { - filterComponents.push( - - ); - } + filterComponents.push( + + ); return ( - + { filterComponents } ); diff --git a/packages/dataviews/src/components/dataviews-filters/style.scss b/packages/dataviews/src/components/dataviews-filters/style.scss index 26e5e613fcbe4..6912b5cc48316 100644 --- a/packages/dataviews/src/components/dataviews-filters/style.scss +++ b/packages/dataviews/src/components/dataviews-filters/style.scss @@ -2,6 +2,10 @@ position: relative; } +.dataviews-filters__container { + padding-top: 0; +} + .dataviews-filters__reset-button.dataviews-filters__reset-button[aria-disabled="true"] { &, &:hover { @@ -250,3 +254,29 @@ width: $icon-size; } } + +.dataviews-filters__container-visibility-toggle { + position: relative; + flex-shrink: 0; +} + +.dataviews-filters-toggle__count { + position: absolute; + top: 0; + right: 0; + transform: translate(50%, -50%); + background: var(--wp-admin-theme-color, #3858e9); + height: $grid-unit-20; + min-width: $grid-unit-20; + line-height: $grid-unit-20; + padding: 0 $grid-unit-05; + text-align: center; + border-radius: $grid-unit-10; + font-size: 11px; + outline: var(--wp-admin-border-width-focus) solid $white; + color: $white; +} + +.dataviews-search { + width: fit-content; +} diff --git a/packages/dataviews/src/components/dataviews-search/index.tsx b/packages/dataviews/src/components/dataviews-search/index.tsx index 5e5ce705e1119..8461acfe9d27a 100644 --- a/packages/dataviews/src/components/dataviews-search/index.tsx +++ b/packages/dataviews/src/components/dataviews-search/index.tsx @@ -39,6 +39,7 @@ const DataViewsSearch = memo( function Search( { label }: SearchProps ) { const searchLabel = label || __( 'Search' ); return ( ( { }: DataViewsProps< Item > ) { const [ selectionState, setSelectionState ] = useState< string[] >( [] ); const [ density, setDensity ] = useState< number >( 0 ); + const [ isShowingFilter, setIsShowingFilter ] = + useState< boolean >( false ); const isUncontrolled = selectionProperty === undefined || onChangeSelection === undefined; const selection = isUncontrolled ? selectionState : selectionProperty; @@ -90,6 +96,7 @@ export default function DataViews< Item >( { ); }, [ selection, data, getItemId ] ); + const filters = useFilters( _fields, view ); return ( ( { alignment="top" justify="start" className="dataviews__view-actions" + spacing={ 1 } > - + { search && } - + { view.type === LAYOUT_GRID && ( ( { { header } + { isShowingFilter && } diff --git a/packages/dataviews/src/components/dataviews/style.scss b/packages/dataviews/src/components/dataviews/style.scss index 6b8af6a90007d..742c8c42134df 100644 --- a/packages/dataviews/src/components/dataviews/style.scss +++ b/packages/dataviews/src/components/dataviews/style.scss @@ -9,7 +9,8 @@ flex-direction: column; } -.dataviews__view-actions { +.dataviews__view-actions, +.dataviews-filters__container { box-sizing: border-box; padding: $grid-unit-20 $grid-unit-60; flex-shrink: 0; @@ -78,7 +79,8 @@ /* stylelint-disable-next-line scss/at-rule-no-unknown -- '@container' not globally permitted */ @container (max-width: 430px) { - .dataviews__view-actions { + .dataviews__view-actions, + .dataviews-filters__container { padding: $grid-unit-15 $grid-unit-30; .components-search-control { @@ -95,3 +97,4 @@ padding-right: $grid-unit-30; } } + diff --git a/packages/edit-site/src/components/page-patterns/index.js b/packages/edit-site/src/components/page-patterns/index.js index a8db73272c0ce..66d2d25a4b8f0 100644 --- a/packages/edit-site/src/components/page-patterns/index.js +++ b/packages/edit-site/src/components/page-patterns/index.js @@ -171,6 +171,7 @@ export default function DataviewsPatterns() { descriptionId={ `${ id }-description` } /> } > { @@ -334,6 +339,7 @@ export default function PostList( { postType } ) { } > { page.getByRole( 'button', { name: 'Add filter' } ) ).toBeFocused(); - await page.keyboard.press( 'Tab' ); - await expect( - page.getByRole( 'button', { name: 'Reset' } ) - ).toBeFocused(); - await page.keyboard.press( 'Tab' ); await expect( page.getByRole( 'button', { name: 'Layout' } ) @@ -74,7 +69,6 @@ test.describe( 'Dataviews List Layout', () => { await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); - await page.keyboard.press( 'Tab' ); // Make sure the items have loaded before reaching for the 1st item in the list. await expect( page.getByRole( 'grid' ) ).toBeVisible(); @@ -104,7 +98,6 @@ test.describe( 'Dataviews List Layout', () => { await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); - await page.keyboard.press( 'Tab' ); // Make sure the items have loaded before reaching for the 1st item in the list. await expect( page.getByRole( 'grid' ) ).toBeVisible(); @@ -128,7 +121,6 @@ test.describe( 'Dataviews List Layout', () => { await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); - await page.keyboard.press( 'Tab' ); // Make sure the items have loaded before reaching for the 1st item in the list. await expect( page.getByRole( 'grid' ) ).toBeVisible(); @@ -170,7 +162,6 @@ test.describe( 'Dataviews List Layout', () => { await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); - await page.keyboard.press( 'Tab' ); // Make sure the items have loaded before reaching for the 1st item in the list. await expect( page.getByRole( 'grid' ) ).toBeVisible(); diff --git a/test/e2e/specs/site-editor/new-templates-list.spec.js b/test/e2e/specs/site-editor/new-templates-list.spec.js index 92edc74101183..878f0ed2eb48c 100644 --- a/test/e2e/specs/site-editor/new-templates-list.spec.js +++ b/test/e2e/specs/site-editor/new-templates-list.spec.js @@ -56,7 +56,7 @@ test.describe( 'Templates', () => { await expect( titles ).toHaveCount( 1 ); await expect( titles.first() ).toHaveText( 'Tag Archives' ); await page - .getByRole( 'button', { name: 'Reset', exact: true } ) + .getByRole( 'button', { name: 'Reset search', exact: true } ) .click(); await expect( titles ).toHaveCount( 6 ); diff --git a/test/e2e/specs/site-editor/patterns.spec.js b/test/e2e/specs/site-editor/patterns.spec.js index 0b0fada39c262..538f9ba936a89 100644 --- a/test/e2e/specs/site-editor/patterns.spec.js +++ b/test/e2e/specs/site-editor/patterns.spec.js @@ -148,6 +148,14 @@ test.describe( 'Patterns', () => { await admin.visitSiteEditor( { postType: 'wp_block' } ); await expect( patterns.item ).toHaveCount( 3 ); + + await patterns.content + .getByRole( 'button', { + name: 'Toggle filter display', + exact: true, + } ) + .click(); + const searchBox = patterns.content.getByRole( 'searchbox', { name: 'Search', } ); From 8be73490bbd1b53f2f25ad1d9712c2fd802f318a Mon Sep 17 00:00:00 2001 From: Jorge Costa Date: Fri, 2 Aug 2024 11:54:44 +0100 Subject: [PATCH 2/2] fix search invalidates the views --- .../src/components/dataviews-search/index.tsx | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/dataviews/src/components/dataviews-search/index.tsx b/packages/dataviews/src/components/dataviews-search/index.tsx index 8461acfe9d27a..1aef99872960b 100644 --- a/packages/dataviews/src/components/dataviews-search/index.tsx +++ b/packages/dataviews/src/components/dataviews-search/index.tsx @@ -30,11 +30,13 @@ const DataViewsSearch = memo( function Search( { label }: SearchProps ) { viewRef.current = view; }, [ onChangeView, view ] ); useEffect( () => { - onChangeViewRef.current( { - ...viewRef.current, - page: 1, - search: debouncedSearch, - } ); + if ( debouncedSearch !== viewRef.current?.search ) { + onChangeViewRef.current( { + ...viewRef.current, + page: 1, + search: debouncedSearch, + } ); + } }, [ debouncedSearch ] ); const searchLabel = label || __( 'Search' ); return (