From ecb7cea502aeb8287dff9f2c2f768042bc2464f9 Mon Sep 17 00:00:00 2001 From: Katie McFaul Date: Wed, 9 Aug 2023 10:35:34 -0400 Subject: [PATCH 1/3] feat(TreeView): onCollapse callback --- .../src/components/TreeView/TreeView.tsx | 7 ++++++- .../components/TreeView/TreeViewListItem.tsx | 20 ++++++++++++------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/react-core/src/components/TreeView/TreeView.tsx b/packages/react-core/src/components/TreeView/TreeView.tsx index bb796a7130f..8acad7cfb8b 100644 --- a/packages/react-core/src/components/TreeView/TreeView.tsx +++ b/packages/react-core/src/components/TreeView/TreeView.tsx @@ -76,6 +76,8 @@ export interface TreeViewProps { onCheck?: (event: React.ChangeEvent, item: TreeViewDataItem, parentItem: TreeViewDataItem) => void; /** Callback for item selection. */ onSelect?: (event: React.MouseEvent, item: TreeViewDataItem, parentItem: TreeViewDataItem) => void; + /** Callback for toggling a node with children. */ + onCollapse?: (event: React.MouseEvent, item: TreeViewDataItem, parentItem: TreeViewDataItem) => void; /** Internal. Parent item of a tree view list item. */ parentItem?: TreeViewDataItem; /** Toolbar to display above the tree view. */ @@ -104,6 +106,7 @@ export const TreeView: React.FunctionComponent = ({ parentItem, onSelect, onCheck, + onCollapse, toolbar, activeItems, compareItems = (item, itemToCheck) => item.id === itemToCheck.id, @@ -113,7 +116,7 @@ export const TreeView: React.FunctionComponent = ({ }: TreeViewProps) => { const treeViewList = ( - {data.map(item => ( + {data.map((item) => ( = ({ defaultExpanded={item.defaultExpanded !== undefined ? item.defaultExpanded : defaultAllExpanded} onSelect={onSelect} onCheck={onCheck} + onCollapse={onCollapse} hasCheckbox={item.hasCheckbox !== undefined ? item.hasCheckbox : hasCheckboxes} checkProps={item.checkProps} hasBadge={item.hasBadge !== undefined ? item.hasBadge : hasBadges} @@ -153,6 +157,7 @@ export const TreeView: React.FunctionComponent = ({ defaultAllExpanded={defaultAllExpanded} onSelect={onSelect} onCheck={onCheck} + onCollapse={onCollapse} activeItems={activeItems} icon={icon} expandedIcon={expandedIcon} diff --git a/packages/react-core/src/components/TreeView/TreeViewListItem.tsx b/packages/react-core/src/components/TreeView/TreeViewListItem.tsx index 0bc4b3946b2..882a7d3880e 100644 --- a/packages/react-core/src/components/TreeView/TreeViewListItem.tsx +++ b/packages/react-core/src/components/TreeView/TreeViewListItem.tsx @@ -55,6 +55,8 @@ export interface TreeViewListItemProps { * from toggling. */ onSelect?: (event: React.MouseEvent, item: TreeViewDataItem, parent: TreeViewDataItem) => void; + /** Callback for toggling a node with children. */ + onCollapse?: (event: React.MouseEvent, item: TreeViewDataItem, parentItem: TreeViewDataItem) => void; /** Parent item of tree view item. */ parentItem?: TreeViewDataItem; /** Title of a tree view item. */ @@ -74,6 +76,7 @@ const TreeViewListItemBase: React.FunctionComponent = ({ defaultExpanded = false, children = null, onSelect, + onCollapse, onCheck, hasCheckbox = false, checkProps = { @@ -117,6 +120,7 @@ const TreeViewListItemBase: React.FunctionComponent = ({ className={css(styles.treeViewNodeToggle)} onClick={(evt: React.MouseEvent) => { if (isSelectable || hasCheckbox) { + onCollapse && onCollapse(evt, itemData, parentItem); setIsExpanded(!internalIsExpanded); } if (isSelectable) { @@ -135,9 +139,9 @@ const TreeViewListItemBase: React.FunctionComponent = ({ onCheck && onCheck(evt, itemData, parentItem)} - onClick={evt => evt.stopPropagation()} - ref={elem => elem && (elem.indeterminate = checkProps.checked === null)} + onChange={(evt) => onCheck && onCheck(evt, itemData, parentItem)} + onClick={(evt) => evt.stopPropagation()} + ref={(elem) => elem && (elem.indeterminate = checkProps.checked === null)} {...checkProps} checked={checkProps.checked === null ? false : checkProps.checked} id={randomId} @@ -192,7 +196,7 @@ const TreeViewListItemBase: React.FunctionComponent = ({ >
- {randomId => ( + {(randomId) => ( = ({ (!children || isSelectable) && activeItems && activeItems.length > 0 && - activeItems.some(item => compareItems && item && compareItems(item, itemData)) + activeItems.some((item) => compareItems && item && compareItems(item, itemData)) ? styles.modifiers.current : '' )} @@ -208,6 +212,7 @@ const TreeViewListItemBase: React.FunctionComponent = ({ if (!hasCheckbox) { onSelect && onSelect(evt, itemData, parentItem); if (!isSelectable && children && evt.isDefaultPrevented() !== true) { + onCollapse && onCollapse(evt, itemData, parentItem); setIsExpanded(!internalIsExpanded); } } @@ -241,13 +246,13 @@ export const TreeViewListItem = React.memo(TreeViewListItemBase, (prevProps, nex prevProps.activeItems && prevProps.activeItems.length > 0 && prevProps.activeItems.some( - item => prevProps.compareItems && item && prevProps.compareItems(item, prevProps.itemData) + (item) => prevProps.compareItems && item && prevProps.compareItems(item, prevProps.itemData) ); const nextIncludes = nextProps.activeItems && nextProps.activeItems.length > 0 && nextProps.activeItems.some( - item => nextProps.compareItems && item && nextProps.compareItems(item, nextProps.itemData) + (item) => nextProps.compareItems && item && nextProps.compareItems(item, nextProps.itemData) ); if (prevIncludes || nextIncludes) { @@ -262,6 +267,7 @@ export const TreeViewListItem = React.memo(TreeViewListItemBase, (prevProps, nex prevProps.defaultExpanded !== nextProps.defaultExpanded || prevProps.onSelect !== nextProps.onSelect || prevProps.onCheck !== nextProps.onCheck || + prevProps.onCollapse !== nextProps.onCollapse || prevProps.hasCheckbox !== nextProps.hasCheckbox || prevProps.checkProps !== nextProps.checkProps || prevProps.hasBadge !== nextProps.hasBadge || From ead99ef9d0c6765cfdd24add7910f94b649060dd Mon Sep 17 00:00:00 2001 From: Katie McFaul Date: Tue, 15 Aug 2023 09:41:33 -0400 Subject: [PATCH 2/3] separate callbacks into collapse and expand --- .../src/components/TreeView/TreeView.tsx | 7 ++++++- .../components/TreeView/TreeViewListItem.tsx | 18 +++++++++++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/packages/react-core/src/components/TreeView/TreeView.tsx b/packages/react-core/src/components/TreeView/TreeView.tsx index 8acad7cfb8b..315c5eb8ac6 100644 --- a/packages/react-core/src/components/TreeView/TreeView.tsx +++ b/packages/react-core/src/components/TreeView/TreeView.tsx @@ -76,7 +76,9 @@ export interface TreeViewProps { onCheck?: (event: React.ChangeEvent, item: TreeViewDataItem, parentItem: TreeViewDataItem) => void; /** Callback for item selection. */ onSelect?: (event: React.MouseEvent, item: TreeViewDataItem, parentItem: TreeViewDataItem) => void; - /** Callback for toggling a node with children. */ + /** Callback for expanding a node with children. */ + onExpand?: (event: React.MouseEvent, item: TreeViewDataItem, parentItem: TreeViewDataItem) => void; + /** Callback for collapsing a node with children. */ onCollapse?: (event: React.MouseEvent, item: TreeViewDataItem, parentItem: TreeViewDataItem) => void; /** Internal. Parent item of a tree view list item. */ parentItem?: TreeViewDataItem; @@ -106,6 +108,7 @@ export const TreeView: React.FunctionComponent = ({ parentItem, onSelect, onCheck, + onExpand, onCollapse, toolbar, activeItems, @@ -127,6 +130,7 @@ export const TreeView: React.FunctionComponent = ({ defaultExpanded={item.defaultExpanded !== undefined ? item.defaultExpanded : defaultAllExpanded} onSelect={onSelect} onCheck={onCheck} + onExpand={onExpand} onCollapse={onCollapse} hasCheckbox={item.hasCheckbox !== undefined ? item.hasCheckbox : hasCheckboxes} checkProps={item.checkProps} @@ -157,6 +161,7 @@ export const TreeView: React.FunctionComponent = ({ defaultAllExpanded={defaultAllExpanded} onSelect={onSelect} onCheck={onCheck} + onExpand={onExpand} onCollapse={onCollapse} activeItems={activeItems} icon={icon} diff --git a/packages/react-core/src/components/TreeView/TreeViewListItem.tsx b/packages/react-core/src/components/TreeView/TreeViewListItem.tsx index 882a7d3880e..bc087516aa0 100644 --- a/packages/react-core/src/components/TreeView/TreeViewListItem.tsx +++ b/packages/react-core/src/components/TreeView/TreeViewListItem.tsx @@ -55,7 +55,9 @@ export interface TreeViewListItemProps { * from toggling. */ onSelect?: (event: React.MouseEvent, item: TreeViewDataItem, parent: TreeViewDataItem) => void; - /** Callback for toggling a node with children. */ + /** Callback for expanding a node with children. */ + onExpand?: (event: React.MouseEvent, item: TreeViewDataItem, parentItem: TreeViewDataItem) => void; + /** Callback for collapsing a node with children. */ onCollapse?: (event: React.MouseEvent, item: TreeViewDataItem, parentItem: TreeViewDataItem) => void; /** Parent item of tree view item. */ parentItem?: TreeViewDataItem; @@ -76,6 +78,7 @@ const TreeViewListItemBase: React.FunctionComponent = ({ defaultExpanded = false, children = null, onSelect, + onExpand, onCollapse, onCheck, hasCheckbox = false, @@ -120,7 +123,11 @@ const TreeViewListItemBase: React.FunctionComponent = ({ className={css(styles.treeViewNodeToggle)} onClick={(evt: React.MouseEvent) => { if (isSelectable || hasCheckbox) { - onCollapse && onCollapse(evt, itemData, parentItem); + if (internalIsExpanded) { + onCollapse && onCollapse(evt, itemData, parentItem); + } else { + onExpand && onExpand(evt, itemData, parentItem); + } setIsExpanded(!internalIsExpanded); } if (isSelectable) { @@ -212,7 +219,11 @@ const TreeViewListItemBase: React.FunctionComponent = ({ if (!hasCheckbox) { onSelect && onSelect(evt, itemData, parentItem); if (!isSelectable && children && evt.isDefaultPrevented() !== true) { - onCollapse && onCollapse(evt, itemData, parentItem); + if (internalIsExpanded) { + onCollapse && onCollapse(evt, itemData, parentItem); + } else { + onExpand && onExpand(evt, itemData, parentItem); + } setIsExpanded(!internalIsExpanded); } } @@ -267,6 +278,7 @@ export const TreeViewListItem = React.memo(TreeViewListItemBase, (prevProps, nex prevProps.defaultExpanded !== nextProps.defaultExpanded || prevProps.onSelect !== nextProps.onSelect || prevProps.onCheck !== nextProps.onCheck || + prevProps.onExpand !== nextProps.onExpand || prevProps.onCollapse !== nextProps.onCollapse || prevProps.hasCheckbox !== nextProps.hasCheckbox || prevProps.checkProps !== nextProps.checkProps || From 4917c32810a92de7cfb1f52bbcf3b7faa3fbaace Mon Sep 17 00:00:00 2001 From: Katie McFaul Date: Wed, 16 Aug 2023 10:12:08 -0400 Subject: [PATCH 3/3] add test --- .../TreeView/__tests__/TreeView.test.tsx | 33 ++++++++++- .../__snapshots__/TreeView.test.tsx.snap | 56 +++++++++---------- 2 files changed, 60 insertions(+), 29 deletions(-) diff --git a/packages/react-core/src/components/TreeView/__tests__/TreeView.test.tsx b/packages/react-core/src/components/TreeView/__tests__/TreeView.test.tsx index 4ee8e389732..9b893398742 100644 --- a/packages/react-core/src/components/TreeView/__tests__/TreeView.test.tsx +++ b/packages/react-core/src/components/TreeView/__tests__/TreeView.test.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { render } from '@testing-library/react'; +import { render, screen, fireEvent } from '@testing-library/react'; import { TreeView } from '../TreeView'; import { Button } from '@patternfly/react-core'; import { FolderIcon, FolderOpenIcon } from '@patternfly/react-icons'; @@ -148,6 +148,37 @@ describe('tree view', () => { expect(asFragment()).toMatchSnapshot(); }); + test('calls onExpand and onCollapse appropriately', () => { + const onExpand = jest.fn(); + const onCollapse = jest.fn(); + const { asFragment } = render( + + ); + expect(onExpand).not.toHaveBeenCalled(); + expect(onCollapse).not.toHaveBeenCalled(); + expect(screen.queryByText('Application 3')).toBeNull(); + expect(screen.getByText('Cost Management')).toBeInTheDocument(); + fireEvent( + screen.getByText('Cost Management'), + new MouseEvent('click', { + bubbles: true, + cancelable: true + }) + ); + expect(onExpand).toHaveBeenCalled(); + expect(onCollapse).not.toHaveBeenCalled(); + expect(screen.getByText('Application 3')).toBeInTheDocument(); + fireEvent( + screen.getByText('Cost Management'), + new MouseEvent('click', { + bubbles: true, + cancelable: true + }) + ); + expect(onCollapse).toHaveBeenCalled(); + expect(screen.queryByText('Application 3')).toBeNull(); + }); + test('renders active successfully', () => { const { asFragment } = render(); expect(asFragment()).toMatchSnapshot(); diff --git a/packages/react-core/src/components/TreeView/__tests__/__snapshots__/TreeView.test.tsx.snap b/packages/react-core/src/components/TreeView/__tests__/__snapshots__/TreeView.test.tsx.snap index 9af75101b45..3da0823e398 100644 --- a/packages/react-core/src/components/TreeView/__tests__/__snapshots__/TreeView.test.tsx.snap +++ b/packages/react-core/src/components/TreeView/__tests__/__snapshots__/TreeView.test.tsx.snap @@ -1905,14 +1905,14 @@ exports[`tree view renders checkboxes successfully 1`] = ` >