Skip to content

Commit

Permalink
Merge pull request #128 from VEuPathDB/improve-CheckboxTree-performance
Browse files Browse the repository at this point in the history
Replace CheckboxTreeNode's inline styles with CSS selectors
  • Loading branch information
jernestmyers committed Dec 2, 2022
2 parents 5a6000e + 8e800c5 commit b090d7e
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 34 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@veupathdb/coreui",
"version": "0.18.14",
"version": "0.18.15",
"author": "Michael Dunn <mdunn4@nd.edu>",
"description": "Core components and style definitions for VEuPath applications.",
"private": false,
Expand Down
1 change: 1 addition & 0 deletions src/components/inputs/SelectTree/SelectTree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ function SelectTree<T>(props: SelectTreeProps<T>) {
isAdditionalFilterApplied={props.isAdditionalFilterApplied}
wrapTreeSection={props.wrapTreeSection}
styleOverrides={props.styleOverrides}
customTreeNodeCssSelectors={props.customTreeNodeCssSelectors}
/>
)

Expand Down
30 changes: 26 additions & 4 deletions src/components/inputs/checkboxes/CheckboxTree/CheckboxTree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { useCallback, MouseEventHandler, useMemo } from 'react';
import { css } from '@emotion/react';
import { merge } from 'lodash';

import CheckboxTreeNode, { CustomCheckboxes, CheckboxTreeNodeStyleSpec } from './CheckboxTreeNode';
import CheckboxTreeNode, { CustomCheckboxes, CheckboxTreeNodeStyleSpec, defaultTreeNodeStyleSpec } from './CheckboxTreeNode';
import SearchBox, { SearchBoxStyleSpec } from '../../SearchBox/SearchBox';
import { Warning } from '../../../icons';

Expand Down Expand Up @@ -95,7 +95,7 @@ const defaultCheckboxTreeStyleSpec: CheckboxTreeStyleSpec = {
padding: '0 1em',
}
},
treeNode: {},
treeNode: defaultTreeNodeStyleSpec,
}

type StatefulNode<T> = T & {
Expand Down Expand Up @@ -222,6 +222,8 @@ export type CheckboxTreeProps<T> = {
wrapTreeSection?: (treeSection: React.ReactNode) => React.ReactNode;

styleOverrides?: CheckboxTreeStyleSpec;

customTreeNodeCssSelectors?: object;
};

type TreeLinkHandler = MouseEventHandler<HTMLButtonElement>;
Expand Down Expand Up @@ -632,6 +634,7 @@ function CheckboxTree<T> (props: CheckboxTreeProps<T>) {
expandedList,
renderNoResults,
styleOverrides = {},
customTreeNodeCssSelectors = {},
} = props;

const styleSpec: CheckboxTreeStyleSpec = useMemo(() => {
Expand Down Expand Up @@ -825,9 +828,29 @@ function CheckboxTree<T> (props: CheckboxTreeProps<T>) {
/>
);

const treeNodeCssSelectors = useMemo(() => {
return ({
'.list': styleSpec.treeNode?.list,
'.visible-element': { display: '' },
'.hidden-element': { display: 'none' },
'.node-wrapper': {...styleSpec.treeNode?.nodeWrapper},
'.top-level-node-wrapper': {...styleSpec.treeNode?.nodeWrapper, ...styleSpec.treeNode?.topLevelNodeWrapper},
'.arrow-icon': { fill: '#aaa', fontSize: '0.75em', cursor: 'pointer' },
'.label-text-wrapper': { ...styleSpec.treeNode?.labelTextWrapper },
'.leaf-node-label': { ...styleSpec.treeNode?.leafNodeLabel },
'.node-label': { ...styleSpec.treeNode?.nodeLabel },
'.children': styleSpec.treeNode?.children,
'.active-search-buffer': { width: '0.75em' },
...customTreeNodeCssSelectors
})
}, [styleSpec.treeNode, customTreeNodeCssSelectors])

let treeSection = (
<div style={styleSpec.treeSection?.container}>
<ul style={styleSpec.treeSection?.ul}>
<ul
style={styleSpec.treeSection?.ul}
css={treeNodeCssSelectors}
>
{topLevelNodes.map((node, index) => {
const nodeId = getNodeId(node);

Expand All @@ -848,7 +871,6 @@ function CheckboxTree<T> (props: CheckboxTreeProps<T>) {
getNodeChildren={getStatefulChildren}
renderNode={renderNode}
customCheckboxes={customCheckboxes as unknown as CustomCheckboxes<StatefulNode<T>>}
styleOverrides={styleSpec.treeNode}
isTopLevelNode={true}
/>
)
Expand Down
48 changes: 19 additions & 29 deletions src/components/inputs/checkboxes/CheckboxTree/CheckboxTreeNode.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React, { useMemo } from 'react';
import { merge } from 'lodash';
import React from 'react';
import { isLeaf } from '../../SelectTree/Utils';
import IndeterminateCheckbox, { IndeterminateCheckboxProps } from '../IndeterminateCheckbox';
import { ArrowRight, ArrowDown } from '../../../icons';
Expand All @@ -19,7 +18,7 @@ export type CheckboxTreeNodeStyleSpec = {
labelTextWrapper?: React.CSSProperties;
};

const defaultStyleSpec: CheckboxTreeNodeStyleSpec = {
export const defaultTreeNodeStyleSpec: CheckboxTreeNodeStyleSpec = {
list: {
listStyle: 'none',
},
Expand Down Expand Up @@ -50,9 +49,6 @@ const defaultStyleSpec: CheckboxTreeNodeStyleSpec = {
}
}

const visibleElement = { display: '' };
const hiddenElement = { display: 'none' };

type TreeRadioProps<T> = {
name: string;
checked: boolean;
Expand Down Expand Up @@ -110,7 +106,6 @@ type Props<T> = {
renderNode: (node: T, path?: number[]) => React.ReactNode;
customCheckboxes?: CustomCheckboxes<T>;
shouldExpandOnClick: boolean;
styleOverrides?: CheckboxTreeNodeStyleSpec;
isTopLevelNode?: boolean;
}

Expand All @@ -129,18 +124,12 @@ export default function CheckboxTreeNode<T>({
renderNode,
customCheckboxes,
shouldExpandOnClick,
styleOverrides = {},
isTopLevelNode = false,
}: Props<T>
) {
const styleSpec: CheckboxTreeNodeStyleSpec = useMemo(() => {
return merge({}, defaultStyleSpec, styleOverrides)
}, [styleOverrides])

let { isSelected, isIndeterminate, isVisible, isExpanded } = getNodeState(node);
let isLeafNode = isLeaf(node, getNodeChildren);
let nodeVisibilityCss = isVisible ? visibleElement : hiddenElement;
let childrenVisibilityCss = isExpanded ? visibleElement : hiddenElement;
let inputName = isLeafNode ? name : '';
let nodeId = getNodeId(node);
const nodeElement = renderNode(node, path);
Expand All @@ -154,23 +143,22 @@ export default function CheckboxTreeNode<T>({
const CustomCheckbox = (customCheckboxes && (nodeId in customCheckboxes)) ? customCheckboxes[nodeId] : undefined;

return (
<li css={{
...nodeVisibilityCss,
...styleSpec.list,
}}>
<li
className={`list ${isVisible ? 'visible-element' : 'hidden-element'}`}
>
<div
css={
isTopLevelNode ? { ...styleSpec.nodeWrapper, ...styleSpec.topLevelNodeWrapper} : {...styleSpec.nodeWrapper}
className={
isTopLevelNode ? 'top-level-node-wrapper' : 'node-wrapper'
}
>
{isLeafNode ? null
: isActiveSearch ? (
// this retains the space of the expansion toggle icons for easier formatting
<div style={{width: '0.75em'}}></div>
<div className='active-search-buffer'></div>
) : (
isExpanded ?
<ArrowDown
style={{fill: '#aaa', fontSize: '0.75em', cursor: 'pointer'}}
className='arrow-icon'
tabIndex={0}
onClick={(e) => {
e.stopPropagation();
Expand All @@ -179,7 +167,7 @@ export default function CheckboxTreeNode<T>({
onKeyDown={(e) => e.key === 'Enter' ? toggleExpansion(node) : null}
/> :
<ArrowRight
style={{fill: '#aaa', fontSize: '0.75em', cursor: 'pointer'}}
className='arrow-icon'
tabIndex={0}
onClick={(e) => {
e.stopPropagation();
Expand All @@ -190,15 +178,15 @@ export default function CheckboxTreeNode<T>({
)}
{!isSelectable || (!isMultiPick && !isLeafNode) ? (
<div
css={{...styleSpec.labelTextWrapper}}
className='label-text-wrapper'
onClick={shouldExpandOnClick ? () => toggleExpansion(node) : undefined}
>
{nodeElement}
</div>
) : (
<label css={
isLeafNode ? {...styleSpec.leafNodeLabel} : {...styleSpec.nodeLabel}
}>
<label
className={isLeafNode ? 'leaf-node-label' : 'node-label'}
>
{CustomCheckbox ? <CustomCheckbox {...checkboxProps} /> : isMultiPick
? <IndeterminateCheckbox {...checkboxProps} />
: <TreeRadio
Expand All @@ -207,15 +195,17 @@ export default function CheckboxTreeNode<T>({
/>
}
<div
css={{...styleSpec.labelTextWrapper}}
className='label-text-wrapper'
>
{nodeElement}
</div>
</label>
)}
</div>
{ !isLeafNode && isVisible && isExpanded &&
<ul css={{...childrenVisibilityCss, ...styleSpec.children}}>
<ul
className={`children ${isExpanded ? 'visible-element' : 'hidden-element'}`}
>
{getNodeChildren(node).map((child, index) =>
<CheckboxTreeNode
key={"node_" + getNodeId(child)}
Expand All @@ -233,7 +223,7 @@ export default function CheckboxTreeNode<T>({
getNodeChildren={getNodeChildren}
renderNode={renderNode}
customCheckboxes={customCheckboxes}
styleOverrides={styleOverrides} />
/>
)}
</ul>
}
Expand Down

0 comments on commit b090d7e

Please sign in to comment.