Skip to content

feat(TreeView): allow selection without expansion #7714

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.

Sign up for GitHub

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

Merged
tlabaj merged 12 commits into from
Jul 29, 2022
Merged

feat(TreeView): allow selection without expansion #7714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0801291
feat(TreeView): allow selection without expansion
kmcfaul Jul 19, 2022
e5edf32
update structure, remove extra button from example
kmcfaul Jul 20, 2022
11d4d58
further html updates
kmcfaul Jul 21, 2022
9ae280a
update structure
kmcfaul Jul 25, 2022
b6c4d2c
add id to toggle button
kmcfaul Jul 25, 2022
dbbb826
apply id only to selectable nodes and update test
kmcfaul Jul 25, 2022
42bbd4f
fix duplicate ids
kmcfaul Jul 26, 2022
bae969f
match checkbox aria
kmcfaul Jul 26, 2022
6af83a2
update snap
kmcfaul Jul 26, 2022
e5773f9
remove htmlfor from selectable nodes
kmcfaul Jul 28, 2022
f05e46d
pr feedback
kmcfaul Jul 29, 2022
32f881f
rename node prop
kmcfaul Jul 29, 2022
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
5 changes: 5 additions & 0 deletions packages/react-core/src/components/TreeView/TreeView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ export interface TreeViewProps {
hasBadges?: boolean;
/** Flag indicating if tree view has guide lines. */
hasGuides?: boolean;
/** Flag indicating that tree nodes should be independently selectable, even when having children */
hasSelectableNodes?: boolean;
Comment on lines +48 to +49
Copy link
Copy Markdown
Contributor

@thatblindgeye thatblindgeye Jul 29, 2022

Choose a reason for hiding this comment

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

Nitpick that I wouldn't block over: isSelectable could be an alternative prop name, as it would also align with our Card and Table selectable variants that have a similar prop.

Copy link
Copy Markdown
Contributor Author

@kmcfaul kmcfaul Jul 29, 2022

Choose a reason for hiding this comment

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

Yeah the prop name is something I wasn't too sure on. I thought about isSelectable but wasn't sure if it would be clear that it would make the nodes selectable opposed to the whole tree. I'm open to either prop name if anyone feels strongly one way or the other.

Copy link
Copy Markdown
Contributor

@thatblindgeye thatblindgeye Jul 29, 2022

Choose a reason for hiding this comment

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

Ah that's a fair point. I might think that having selectable items would make the tree view selectable in nature, but I do see how it isn't exactly clear. How do you feel about the idea of TreeView with prop hasSelectableNodes, and the TreeViewListItem with prop isSelectable?

Copy link
Copy Markdown
Contributor Author

@kmcfaul kmcfaul Jul 29, 2022

Choose a reason for hiding this comment

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

I like that, will update

/** Variant presentation styles for the tree view. */
variant?: 'default' | 'compact' | 'compactNoBackground';
/** Icon for all leaf or unexpanded node items */
Expand Down Expand Up @@ -79,6 +81,7 @@ export const TreeView: React.FunctionComponent<TreeViewProps> = ({
hasChecks = false,
hasBadges = false,
hasGuides = false,
hasSelectableNodes = false,
variant = 'default',
defaultAllExpanded = false,
allExpanded,
Expand All @@ -103,6 +106,7 @@ export const TreeView: React.FunctionComponent<TreeViewProps> = ({
title={item.title}
id={item.id}
isExpanded={allExpanded}
isSelectable={hasSelectableNodes}
defaultExpanded={item.defaultExpanded !== undefined ? item.defaultExpanded : defaultAllExpanded}
onSelect={onSelect}
onCheck={onCheck}
Expand All @@ -129,6 +133,7 @@ export const TreeView: React.FunctionComponent<TreeViewProps> = ({
hasChecks={hasChecks}
hasBadges={hasBadges}
hasGuides={hasGuides}
hasSelectableNodes={hasSelectableNodes}
variant={variant}
allExpanded={allExpanded}
defaultAllExpanded={defaultAllExpanded}
Expand Down
48 changes: 31 additions & 17 deletions packages/react-core/src/components/TreeView/TreeViewListItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ export interface TreeViewListItemProps {
checkProps?: TreeViewCheckProps;
/** Flag indicating if a tree view item has a badge */
hasBadge?: boolean;
/** Flag indicating that tree nodes should be independently selectable, even when having children */
isSelectable?: boolean;
/** Optional prop for custom badge */
customBadgeContent?: React.ReactNode;
/** Additional properties of the tree view item badge */
Expand Down Expand Up @@ -73,6 +75,7 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
hasBadge = false,
customBadgeContent,
badgeProps = { isRead: true },
isSelectable = false,
isCompact,
activeItems = [],
itemData,
Expand All @@ -93,17 +96,27 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
}
}, [isExpanded, defaultExpanded]);

const Component = hasCheck ? 'div' : 'button';
const ToggleComponent = hasCheck ? 'button' : 'div';
let Component: 'label' | 'div' | 'button' = 'button';
if (hasCheck) {
Component = 'label';
} else if (isSelectable) {
Component = 'div';
}

const ToggleComponent = hasCheck || isSelectable ? 'button' : 'span';

const renderToggle = (randomId: string) => (
<ToggleComponent
className={css(styles.treeViewNodeToggle)}
onClick={() => {
if (hasCheck) {
onClick={(evt: React.MouseEvent) => {
if (isSelectable || hasCheck) {
setIsExpanded(!internalIsExpanded);
}
if (isSelectable) {
evt.stopPropagation();
}
}}
{...(hasCheck && { 'aria-labelledby': `label-${randomId}` })}
{...((hasCheck || isSelectable) && { 'aria-labelledby': `label-${randomId}` })}
tabIndex={-1}
>
<span className={css(styles.treeViewNodeToggleIcon)}>
Expand Down Expand Up @@ -131,20 +144,18 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
{internalIsExpanded && (expandedIcon || icon)}
</span>
);
const renderNodeContent = (randomId: string) => {
const renderNodeContent = () => {
const content = (
<>
{isCompact && title && <span className={css(styles.treeViewNodeTitle)}>{title}</span>}
{hasCheck ? (
<label className={css(styles.treeViewNodeText)} htmlFor={randomId} id={`label-${randomId}`}>
{name}
</label>
<span className={css(styles.treeViewNodeText)}>{name}</span>
) : (
<span className={css(styles.treeViewNodeText)}>{name}</span>
)}
</>
);
return isCompact ? <div className={css(styles.treeViewNodeContent)}>{content}</div> : content;
return isCompact ? <span className={css(styles.treeViewNodeContent)}>{content}</span> : content;
};
const badgeRendered = (
<>
Expand All @@ -171,12 +182,13 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
tabIndex={-1}
>
<div className={css(styles.treeViewContent)}>
<GenerateId prefix="checkbox-id">
<GenerateId prefix={isSelectable ? 'selectable-id' : 'checkbox-id'}>
{randomId => (
<Component
className={css(
styles.treeViewNode,
!children &&
children && (isSelectable || hasCheck) && styles.modifiers.selectable,
(!children || isSelectable) &&
activeItems &&
activeItems.length > 0 &&
activeItems.some(item => compareItems && item && compareItems(item, itemData))
Expand All @@ -186,20 +198,22 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
onClick={(evt: React.MouseEvent) => {
if (!hasCheck) {
onSelect && onSelect(evt, itemData, parentItem);
if (children && evt.isDefaultPrevented() !== true) {
if (!isSelectable && children && evt.isDefaultPrevented() !== true) {
setIsExpanded(!internalIsExpanded);
}
}
}}
tabIndex={-1}
tabIndex={isSelectable ? 0 : -1}
{...(hasCheck && { htmlFor: randomId })}
{...((hasCheck || (isSelectable && children)) && { id: `label-${randomId}` })}
>
<div className={css(styles.treeViewNodeContainer)}>
<span className={css(styles.treeViewNodeContainer)}>
{children && renderToggle(randomId)}
{hasCheck && renderCheck(randomId)}
{icon && iconRendered}
{renderNodeContent(randomId)}
{renderNodeContent()}
{badgeRendered}
</div>
</span>
</Component>
)}
</GenerateId>
Expand Down
Loading