From 87c2d35fc43ed40433e762bc16446eb392ecb4e2 Mon Sep 17 00:00:00 2001 From: adamviktora Date: Tue, 1 Oct 2024 18:46:43 +0200 Subject: [PATCH 1/4] fix(DualListSelectorTree example): improved behaviour with applied filter - "move all" button moves only those options that are visible due to the filter - change "number of options selected" text based on number of options shown due to the filter - when having selected options, then filtering them out, the "move selected" button will be disabled and won't move them --- .../examples/DualListSelectorTree.tsx | 61 +++++++++---------- 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/packages/react-core/src/components/DualListSelector/examples/DualListSelectorTree.tsx b/packages/react-core/src/components/DualListSelector/examples/DualListSelectorTree.tsx index 5f87520a26b..0b4970755b3 100644 --- a/packages/react-core/src/components/DualListSelector/examples/DualListSelectorTree.tsx +++ b/packages/react-core/src/components/DualListSelector/examples/DualListSelectorTree.tsx @@ -36,8 +36,6 @@ export const DualListSelectorComposableTree: React.FunctionComponent(['beans', 'beef', 'chicken', 'tofu']); const [chosenFilter, setChosenFilter] = React.useState(''); const [availableFilter, setAvailableFilter] = React.useState(''); - let hiddenChosen: string[] = []; - let hiddenAvailable: string[] = []; // helper function to build memoized lists const buildTextById = (node: FoodNode): { [key: string]: string } => { @@ -82,7 +80,7 @@ export const DualListSelectorComposableTree: React.FunctionComponent { + const { memoizedLeavesById, memoizedAllLeaves, memoizedNodeTexts } = React.useMemo(() => { let leavesById = {}; let allLeaves: string[] = []; let nodeTexts = {}; @@ -94,32 +92,47 @@ export const DualListSelectorComposableTree: React.FunctionComponent { + const filterMatchingNodeIds = Object.keys(memoizedLeavesById).filter((nodeId) => + memoizedNodeTexts[nodeId].includes(filter) + ); + const filterMatchingLeafIds = filterMatchingNodeIds.map((nodeId) => memoizedLeavesById[nodeId]).flat(); + return leafIds.filter((leafId) => filterMatchingLeafIds.includes(leafId)); + }; + + const availableLeafIds = memoizedAllLeaves.filter((leafId) => !chosenLeafIds.includes(leafId)); + const visibleChosenLeafIds = getVisibleLeafIds(chosenLeafIds, chosenFilter); + const visibleAvailableLeafIds = getVisibleLeafIds(availableLeafIds, availableFilter); + const moveChecked = (toChosen: boolean) => { + const visibleCheckedChosenLeafIds = checkedLeafIds.filter((leafId) => visibleChosenLeafIds.includes(leafId)); + const visibleCheckedAvailableLeafIds = checkedLeafIds.filter((leafId) => visibleAvailableLeafIds.includes(leafId)); + setChosenLeafIds( (prevChosenIds) => toChosen - ? [...prevChosenIds, ...checkedLeafIds] // add checked ids to chosen list - : [...prevChosenIds.filter((x) => !checkedLeafIds.includes(x))] // remove checked ids from chosen list + ? [...prevChosenIds, ...visibleCheckedAvailableLeafIds] // add visible checked ids to chosen list + : prevChosenIds.filter((x) => !visibleCheckedChosenLeafIds.includes(x)) // remove visible checked ids from chosen list ); // uncheck checked ids that just moved setCheckedLeafIds((prevChecked) => toChosen - ? [...prevChecked.filter((x) => chosenLeafIds.includes(x))] - : [...prevChecked.filter((x) => !chosenLeafIds.includes(x))] + ? prevChecked.filter((x) => !visibleCheckedAvailableLeafIds.includes(x)) + : prevChecked.filter((x) => !visibleCheckedChosenLeafIds.includes(x)) ); }; const moveAll = (toChosen: boolean) => { if (toChosen) { - setChosenLeafIds(memoizedAllLeaves); + setChosenLeafIds((prevChosenIds) => [...prevChosenIds, ...visibleAvailableLeafIds]); } else { - setChosenLeafIds([]); + setChosenLeafIds((prevChosenIds) => prevChosenIds.filter((id) => !visibleChosenLeafIds.includes(id))); } }; @@ -149,15 +162,9 @@ export const DualListSelectorComposableTree: React.FunctionComponent { const nodeIdsToCheck = memoizedLeavesById[node.id].filter((id) => - isChosen - ? chosenLeafIds.includes(id) && !hiddenChosen.includes(id) - : !chosenLeafIds.includes(id) && !hiddenAvailable.includes(id) + isChosen ? visibleChosenLeafIds.includes(id) : visibleAvailableLeafIds.includes(id) ); - if (isChosen) { - hiddenChosen = []; - } else { - hiddenAvailable = []; - } + setCheckedLeafIds((prevChecked) => { const otherCheckedNodeNames = prevChecked.filter((id) => !nodeIdsToCheck.includes(id)); return !isChecked ? otherCheckedNodeNames : [...otherCheckedNodeNames, ...nodeIdsToCheck]; @@ -196,7 +203,7 @@ export const DualListSelectorComposableTree: React.FunctionComponent !chosenLeafIds.includes(id)); const hasMatchingChildren = - filterValue && descendentsOnThisPane.some((id) => memoizedNodeText[id].includes(filterValue)); + filterValue && descendentsOnThisPane.some((id) => memoizedNodeTexts[id].includes(filterValue)); const isFilterMatch = filterValue && node.text.includes(filterValue) && descendentsOnThisPane.length > 0; // A node is displayed if either of the following is true: @@ -208,14 +215,6 @@ export const DualListSelectorComposableTree: React.FunctionComponent 0) || isFilterMatch; - if (!isDisplayed) { - if (isChosen) { - hiddenChosen.push(node.id); - } else { - hiddenAvailable.push(node.id); - } - } - return [ ...(isDisplayed ? [ @@ -242,9 +241,9 @@ export const DualListSelectorComposableTree: React.FunctionComponent { const options: DualListSelectorTreeItemData[] = buildOptions(isChosen, data, false); - const numOptions = isChosen ? chosenLeafIds.length : memoizedAllLeaves.length - chosenLeafIds.length; + const numOptions = isChosen ? visibleChosenLeafIds.length : visibleAvailableLeafIds.length; const numSelected = checkedLeafIds.filter((id) => - isChosen ? chosenLeafIds.includes(id) : !chosenLeafIds.includes(id) + isChosen ? visibleChosenLeafIds.includes(id) : visibleAvailableLeafIds.includes(id) ).length; const status = `${numSelected} of ${numOptions} options selected`; const filterApplied = isChosen ? chosenFilter !== '' : availableFilter !== ''; @@ -285,7 +284,7 @@ export const DualListSelectorComposableTree: React.FunctionComponent !chosenLeafIds.includes(x)).length} + isDisabled={!checkedLeafIds.filter((x) => visibleAvailableLeafIds.includes(x)).length} onClick={() => moveChecked(true)} aria-label="Add selected" icon={} @@ -304,7 +303,7 @@ export const DualListSelectorComposableTree: React.FunctionComponent moveChecked(false)} - isDisabled={!checkedLeafIds.filter((x) => !!chosenLeafIds.includes(x)).length} + isDisabled={!checkedLeafIds.filter((x) => visibleChosenLeafIds.includes(x)).length} aria-label="Remove selected" icon={} /> From a5968157e777d5f181c50a75cdb5fe226c71ca63 Mon Sep 17 00:00:00 2001 From: adamviktora Date: Wed, 2 Oct 2024 14:45:24 +0200 Subject: [PATCH 2/4] feat(DualListSelectorTree example): case insensitive filter --- .../DualListSelector/examples/DualListSelectorTree.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/react-core/src/components/DualListSelector/examples/DualListSelectorTree.tsx b/packages/react-core/src/components/DualListSelector/examples/DualListSelectorTree.tsx index 0b4970755b3..9da2cda7645 100644 --- a/packages/react-core/src/components/DualListSelector/examples/DualListSelectorTree.tsx +++ b/packages/react-core/src/components/DualListSelector/examples/DualListSelectorTree.tsx @@ -97,9 +97,11 @@ export const DualListSelectorComposableTree: React.FunctionComponent value.toLowerCase().includes(filter.trim().toLowerCase()); + const getVisibleLeafIds = (leafIds: string[], filter: string) => { const filterMatchingNodeIds = Object.keys(memoizedLeavesById).filter((nodeId) => - memoizedNodeTexts[nodeId].includes(filter) + matchesFilter(memoizedNodeTexts[nodeId], filter) ); const filterMatchingLeafIds = filterMatchingNodeIds.map((nodeId) => memoizedLeavesById[nodeId]).flat(); return leafIds.filter((leafId) => filterMatchingLeafIds.includes(leafId)); @@ -203,8 +205,8 @@ export const DualListSelectorComposableTree: React.FunctionComponent !chosenLeafIds.includes(id)); const hasMatchingChildren = - filterValue && descendentsOnThisPane.some((id) => memoizedNodeTexts[id].includes(filterValue)); - const isFilterMatch = filterValue && node.text.includes(filterValue) && descendentsOnThisPane.length > 0; + filterValue && descendentsOnThisPane.some((id) => matchesFilter(memoizedNodeTexts[id], filterValue)); + const isFilterMatch = filterValue && matchesFilter(node.text, filterValue) && descendentsOnThisPane.length > 0; // A node is displayed if either of the following is true: // - There is no filter value and this node or its descendents belong on this pane From 7c7a81ddebfc33399e2eac3e5cef8fbaf8cb9e9f Mon Sep 17 00:00:00 2001 From: adamviktora Date: Wed, 9 Oct 2024 14:11:48 +0200 Subject: [PATCH 3/4] test(DualListSelectorTree): reenable integration tests + update demo app --- .../integration/duallistselectortree.spec.ts | 20 +++--- .../DualListSelectorTreeDemo.tsx | 68 +++++++++---------- 2 files changed, 46 insertions(+), 42 deletions(-) diff --git a/packages/react-integration/cypress/integration/duallistselectortree.spec.ts b/packages/react-integration/cypress/integration/duallistselectortree.spec.ts index a7abf220b2d..e546b3dd729 100644 --- a/packages/react-integration/cypress/integration/duallistselectortree.spec.ts +++ b/packages/react-integration/cypress/integration/duallistselectortree.spec.ts @@ -31,14 +31,18 @@ describe('Dual List Selector Tree Demo Test', () => { cy.get('.pf-v6-c-dual-list-selector__list').eq(1).find('li').should('have.length', 2); }); - xit('Verify add all filtered options works', () => { + it('Verify add all filtered options works', () => { cy.get('.pf-v6-c-dual-list-selector__list').eq(0).find('li').should('have.length', 2); cy.get('.pf-v6-c-dual-list-selector__tools-filter input').eq(0).type('Fru'); - cy.get('.pf-v6-c-dual-list-selector__list').eq(0).find('li').should('have.length', 1); + cy.get('.pf-v6-c-dual-list-selector__list').eq(0).find('li').should('have.length', 6); cy.get('.pf-v6-c-dual-list-selector__controls-item').eq(1).click(); - cy.get('.pf-v6-c-dual-list-selector__list').eq(1).find('li').should('have.length', 3); + cy.get('.pf-v6-c-dual-list-selector__status-text').eq(0).should('have.text', '0 of 0 options selected'); + cy.get('.pf-v6-c-empty-state').eq(0).should('exist'); + cy.get('.pf-v6-c-dual-list-selector__list').eq(0).find('li').should('have.length', 3); // "Chosen" list is at index 0, because "Available" displays empty state instead + cy.get('.pf-v6-c-dual-list-selector__status-text').eq(1).should('have.text', '0 of 9 options selected'); cy.get('.pf-v6-c-dual-list-selector__tools-filter input').eq(0).type('{backspace}{backspace}{backspace}'); cy.get('.pf-v6-c-dual-list-selector__list').eq(0).find('li').should('have.length', 1); + cy.get('.pf-v6-c-dual-list-selector__status-text').eq(0).should('have.text', '0 of 2 options selected'); }); it('Verify chosen search works', () => { @@ -49,16 +53,16 @@ describe('Dual List Selector Tree Demo Test', () => { cy.get('.pf-v6-c-dual-list-selector__menu').eq(1).find('li').should('have.length', 1); }); - xit('Verify remove all filtered options works', () => { - cy.get('.pf-v6-c-dual-list-selector__list').eq(0).find('li').should('have.length', 0); - cy.get('.pf-v6-c-dual-list-selector__list').eq(1).find('li').should('have.length', 1); + it('Verify remove all filtered options works', () => { + cy.get('.pf-v6-c-dual-list-selector__menu').eq(0).should('be.empty'); + cy.get('.pf-v6-c-dual-list-selector__list').eq(0).find('li').should('have.length', 1); // "Chosen" list is at index 0, because "Available" is empty cy.get('.pf-v6-c-dual-list-selector__controls-item').eq(2).click(); cy.get('.pf-v6-c-dual-list-selector__list').eq(0).find('li').should('have.length', 1); - cy.get('.pf-v6-c-dual-list-selector__list').eq(1).find('li').should('have.length', 0); + cy.get('.pf-v6-c-empty-state').eq(0).should('exist'); cy.get('.pf-v6-c-dual-list-selector__tools-filter input').eq(1).type('{backspace}{backspace}{backspace}'); cy.get('.pf-v6-c-dual-list-selector__list').eq(1).find('li').should('have.length', 3); cy.get('.pf-v6-c-dual-list-selector__controls-item').eq(2).click(); cy.get('.pf-v6-c-dual-list-selector__list').eq(0).find('li').should('have.length', 4); - cy.get('.pf-v6-c-dual-list-selector__list').eq(1).find('li').should('have.length', 0); + cy.get('.pf-v6-c-dual-list-selector__menu').eq(1).should('be.empty'); }); }); diff --git a/packages/react-integration/demo-app-ts/src/components/demos/DualListSelectorDemo/DualListSelectorTreeDemo.tsx b/packages/react-integration/demo-app-ts/src/components/demos/DualListSelectorDemo/DualListSelectorTreeDemo.tsx index f9ad771e584..61324a39eae 100644 --- a/packages/react-integration/demo-app-ts/src/components/demos/DualListSelectorDemo/DualListSelectorTreeDemo.tsx +++ b/packages/react-integration/demo-app-ts/src/components/demos/DualListSelectorDemo/DualListSelectorTreeDemo.tsx @@ -36,8 +36,6 @@ const DualListSelectorComposableTree: React.FunctionComponent = ({ const [chosenLeafIds, setChosenLeafIds] = React.useState(['beans', 'beef', 'chicken', 'tofu']); const [chosenFilter, setChosenFilter] = React.useState(''); const [availableFilter, setAvailableFilter] = React.useState(''); - let hiddenChosen: string[] = []; - let hiddenAvailable: string[] = []; // helper function to build memoized lists const buildTextById = (node: FoodNode): { [key: string]: string } => { @@ -82,7 +80,7 @@ const DualListSelectorComposableTree: React.FunctionComponent = ({ }; // Builds a map of child leaf nodes by node id - memoized so that it only rebuilds the list if the data changes. - const { memoizedLeavesById, memoizedAllLeaves, memoizedNodeText } = React.useMemo(() => { + const { memoizedLeavesById, memoizedAllLeaves, memoizedNodeTexts } = React.useMemo(() => { let leavesById: { [key: string]: string[] } = {}; let allLeaves: string[] = []; let nodeTexts: { [key: string]: string } = {}; @@ -94,32 +92,49 @@ const DualListSelectorComposableTree: React.FunctionComponent = ({ return { memoizedLeavesById: leavesById, memoizedAllLeaves: allLeaves, - memoizedNodeText: nodeTexts + memoizedNodeTexts: nodeTexts }; // eslint-disable-next-line react-hooks/exhaustive-deps }, [data]); + const matchesFilter = (value: string, filter: string) => value.toLowerCase().includes(filter.trim().toLowerCase()); + + const getVisibleLeafIds = (leafIds: string[], filter: string) => { + const filterMatchingNodeIds = Object.keys(memoizedLeavesById).filter((nodeId) => + matchesFilter(memoizedNodeTexts[nodeId], filter) + ); + const filterMatchingLeafIds = filterMatchingNodeIds.map((nodeId) => memoizedLeavesById[nodeId]).flat(); + return leafIds.filter((leafId) => filterMatchingLeafIds.includes(leafId)); + }; + + const availableLeafIds = memoizedAllLeaves.filter((leafId) => !chosenLeafIds.includes(leafId)); + const visibleChosenLeafIds = getVisibleLeafIds(chosenLeafIds, chosenFilter); + const visibleAvailableLeafIds = getVisibleLeafIds(availableLeafIds, availableFilter); + const moveChecked = (toChosen: boolean) => { + const visibleCheckedChosenLeafIds = checkedLeafIds.filter((leafId) => visibleChosenLeafIds.includes(leafId)); + const visibleCheckedAvailableLeafIds = checkedLeafIds.filter((leafId) => visibleAvailableLeafIds.includes(leafId)); + setChosenLeafIds( (prevChosenIds) => toChosen - ? [...prevChosenIds, ...checkedLeafIds] // add checked ids to chosen list - : [...prevChosenIds.filter((x) => !checkedLeafIds.includes(x))] // remove checked ids from chosen list + ? [...prevChosenIds, ...visibleCheckedAvailableLeafIds] // add visible checked ids to chosen list + : prevChosenIds.filter((x) => !visibleCheckedChosenLeafIds.includes(x)) // remove visible checked ids from chosen list ); // uncheck checked ids that just moved setCheckedLeafIds((prevChecked) => toChosen - ? [...prevChecked.filter((x) => chosenLeafIds.includes(x))] - : [...prevChecked.filter((x) => !chosenLeafIds.includes(x))] + ? prevChecked.filter((x) => !visibleCheckedAvailableLeafIds.includes(x)) + : prevChecked.filter((x) => !visibleCheckedChosenLeafIds.includes(x)) ); }; const moveAll = (toChosen: boolean) => { if (toChosen) { - setChosenLeafIds(memoizedAllLeaves); + setChosenLeafIds((prevChosenIds) => [...prevChosenIds, ...visibleAvailableLeafIds]); } else { - setChosenLeafIds([]); + setChosenLeafIds((prevChosenIds) => prevChosenIds.filter((id) => !visibleChosenLeafIds.includes(id))); } }; @@ -149,15 +164,9 @@ const DualListSelectorComposableTree: React.FunctionComponent = ({ isChosen: boolean ) => { const nodeIdsToCheck = memoizedLeavesById[node.id].filter((id) => - isChosen - ? chosenLeafIds.includes(id) && !hiddenChosen.includes(id) - : !chosenLeafIds.includes(id) && !hiddenAvailable.includes(id) + isChosen ? visibleChosenLeafIds.includes(id) : visibleAvailableLeafIds.includes(id) ); - if (isChosen) { - hiddenChosen = []; - } else { - hiddenAvailable = []; - } + setCheckedLeafIds((prevChecked) => { const otherCheckedNodeNames = prevChecked.filter((id) => !nodeIdsToCheck.includes(id)); return !isChecked ? otherCheckedNodeNames : [...otherCheckedNodeNames, ...nodeIdsToCheck]; @@ -189,16 +198,15 @@ const DualListSelectorComposableTree: React.FunctionComponent = ({ const isChecked = isNodeChecked(node, isChosen); - const filterValue = (isChosen ? chosenFilter : availableFilter).toLowerCase().trim(); + const filterValue = isChosen ? chosenFilter : availableFilter; const descendentLeafIds = memoizedLeavesById[node.id]; const descendentsOnThisPane = isChosen ? descendentLeafIds.filter((id) => chosenLeafIds.includes(id)) : descendentLeafIds.filter((id) => !chosenLeafIds.includes(id)); const hasMatchingChildren = - filterValue && descendentsOnThisPane.some((id) => memoizedNodeText[id].toLowerCase().includes(filterValue)); - const isFilterMatch = - filterValue && node.text.toLowerCase().includes(filterValue) && descendentsOnThisPane.length > 0; + filterValue && descendentsOnThisPane.some((id) => matchesFilter(memoizedNodeTexts[id], filterValue)); + const isFilterMatch = filterValue && matchesFilter(node.text, filterValue) && descendentsOnThisPane.length > 0; // A node is displayed if either of the following is true: // - There is no filter value and this node or its descendents belong on this pane @@ -209,14 +217,6 @@ const DualListSelectorComposableTree: React.FunctionComponent = ({ (hasParentMatch && descendentsOnThisPane.length > 0) || isFilterMatch; - if (!isDisplayed) { - if (isChosen) { - hiddenChosen.push(node.id); - } else { - hiddenAvailable.push(node.id); - } - } - return [ ...(isDisplayed ? [ @@ -243,9 +243,9 @@ const DualListSelectorComposableTree: React.FunctionComponent = ({ const buildPane = (isChosen: boolean): React.ReactNode => { const options: DualListSelectorTreeItemData[] = buildOptions(isChosen, data, false); - const numOptions = isChosen ? chosenLeafIds.length : memoizedAllLeaves.length - chosenLeafIds.length; + const numOptions = isChosen ? visibleChosenLeafIds.length : visibleAvailableLeafIds.length; const numSelected = checkedLeafIds.filter((id) => - isChosen ? chosenLeafIds.includes(id) : !chosenLeafIds.includes(id) + isChosen ? visibleChosenLeafIds.includes(id) : visibleAvailableLeafIds.includes(id) ).length; const status = `${numSelected} of ${numOptions} options selected`; const filterApplied = isChosen ? chosenFilter !== '' : availableFilter !== ''; @@ -286,7 +286,7 @@ const DualListSelectorComposableTree: React.FunctionComponent = ({ {buildPane(false)} !chosenLeafIds.includes(x)).length} + isDisabled={!checkedLeafIds.filter((x) => visibleAvailableLeafIds.includes(x)).length} onClick={() => moveChecked(true)} aria-label="Add selected" icon={} @@ -305,7 +305,7 @@ const DualListSelectorComposableTree: React.FunctionComponent = ({ /> moveChecked(false)} - isDisabled={!checkedLeafIds.filter((x) => !!chosenLeafIds.includes(x)).length} + isDisabled={!checkedLeafIds.filter((x) => visibleChosenLeafIds.includes(x)).length} aria-label="Remove selected" icon={} /> From 7fa5766bdfd00bac36268265e3fca8ee4541bb80 Mon Sep 17 00:00:00 2001 From: adamviktora Date: Wed, 9 Oct 2024 18:03:19 +0200 Subject: [PATCH 4/4] test: fix notificationdrawergroups.spec.ts integration test --- .../cypress/integration/notificationdrawergroups.spec.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/react-integration/cypress/integration/notificationdrawergroups.spec.ts b/packages/react-integration/cypress/integration/notificationdrawergroups.spec.ts index 9fb4f509dd4..91237207f5a 100644 --- a/packages/react-integration/cypress/integration/notificationdrawergroups.spec.ts +++ b/packages/react-integration/cypress/integration/notificationdrawergroups.spec.ts @@ -79,6 +79,10 @@ describe('Notification Drawer Groups Demo Test', () => { cy.wrap(toggleButton).type('{esc}', { waitForAnimations: true }); cy.tick(200); cy.get('.notification-9.pf-v6-c-menu').should('not.exist'); + // restore the clock + cy.clock().then((clock) => { + clock.restore(); + }); }); });