Redesign resource tree (#1865)

* start redesign work

* add left padding to all tree nodes

* fiddling with padding

* align tab bar line with first item in resource tree

* final touch ups

* fix a strange password manager autofill prompt

* add keyboard shortcuts

* revert testing change

* nudge messagebar to layout row height

* tidy up

* switch to Allotment to stop ResizeObserver issues with monaco

* refmt and fix lints

* fabric touch-ups

* update snapshots

* remove explicit react-icons dependency

* reinstall packages

* remove background from FluentProvider

* fix alignment of message bar

* undo temporary workaround

* restore refresh button

* fix e2e tests and reformat

* fix compiler error

* remove uiw/react-split

* uncomment selection change on expand

src/Explorer/Controls/TreeComponent/Styles.ts

@@ -0,0 +1,65 @@
+import { makeStyles, shorthands, treeItemLevelToken } from "@fluentui/react-components";
+import { cosmosShorthands, tokens } from "Explorer/Theme/ThemeUtil";
+
+export type TreeStyleName = keyof ReturnType<typeof useTreeStyles>;
+
+const treeIconWidth = "--cosmos-Tree--iconWidth" as const;
+const leafNodeSpacing = "--cosmos-Tree--leafNodeSpacing" as const;
+const actionButtonBackground = "--cosmos-Tree--actionButtonBackground" as const;
+
+export const useTreeStyles = makeStyles({
+  treeContainer: {
+    height: "100%",
+    ...shorthands.overflow("auto"),
+  },
+  tree: {
+    width: "fit-content",
+    minWidth: "100%",
+    rowGap: "0px",
+    paddingTop: "0px",
+    [treeIconWidth]: "20px",
+    [leafNodeSpacing]: "24px",
+  },
+  nodeIcon: {
+    width: `var(${treeIconWidth})`,
+    height: `var(${treeIconWidth})`,
+  },
+  treeItem: {},
+  nodeLabel: {},
+  treeItemLayout: {
+    fontSize: tokens.fontSizeBase300,
+    height: tokens.layoutRowHeight,
+    ...cosmosShorthands.borderBottom(),
+    paddingLeft: `calc(var(${treeItemLevelToken}, 1) * ${tokens.spacingHorizontalXXL})`,
+
+    // Some sneaky CSS variables stuff to change the background color of the action button on hover.
+    [actionButtonBackground]: tokens.colorNeutralBackground1,
+    "&:hover": {
+      [actionButtonBackground]: tokens.colorNeutralBackground1Hover,
+    },
+  },
+  actionsButtonContainer: {
+    position: "sticky",
+    right: 0,
+  },
+  actionsButton: {
+    backgroundColor: `var(${actionButtonBackground})`,
+  },
+  treeItemLayoutNoIcon: {
+    // Pad the text out by the level, the width of the icon, AND the usual spacing between the icon and the level.
+    // It would be nice to see if we can use Grid layout or something here, but that would require overriding a lot of the existing Tree component behavior.
+    paddingLeft: `calc((var(${treeItemLevelToken}, 1) * ${tokens.spacingHorizontalXXL}) + var(${leafNodeSpacing}))`,
+  },
+  selectedItem: {
+    backgroundColor: tokens.colorNeutralBackground1Selected,
+  },
+  databaseNode: {
+    fontWeight: tokens.fontWeightSemibold,
+  },
+  collectionNode: {
+    fontWeight: tokens.fontWeightSemibold,
+  },
+  loadMoreNode: {
+    color: tokens.colorBrandForegroundLink,
+  },
+});

src/Explorer/Controls/TreeComponent/TreeNodeComponent.test.tsx

@@ -1,4 +1,4 @@
-import { TreeItem, TreeItemLayout, tokens } from "@fluentui/react-components";
+import { TreeItem, TreeItemLayout } from "@fluentui/react-components";
 import PromiseSource from "Utils/PromiseSource";
 import { mount, shallow } from "enzyme";
 import React from "react";
@@ -9,7 +9,7 @@ function generateTestNode(id: string, additionalProps?: Partial<TreeNode>): Tree
   const node: TreeNode = {
     id,
     label: `${id}Label`,
-    className: `${id}Class`,
+    className: "nodeIcon",
     iconSrc: `${id}Icon`,
     onClick: jest.fn().mockName(`${id}Click`),
     ...additionalProps,
@@ -20,7 +20,7 @@ function generateTestNode(id: string, additionalProps?: Partial<TreeNode>): Tree
 describe("TreeNodeComponent", () => {
   it("renders a single node", () => {
     const node = generateTestNode("root");
-    const component = shallow(<TreeNodeComponent node={node} treeNodeId={node.id} />);
+    const component = shallow(<TreeNodeComponent openItems={[]} node={node} treeNodeId={node.id} />);
     expect(component).toMatchSnapshot();
 
     // The "click" handler is actually attached to onOpenChange, with a type of "Click".
@@ -45,7 +45,7 @@ describe("TreeNodeComponent", () => {
         },
       ],
     });
-    const component = shallow(<TreeNodeComponent node={node} treeNodeId={node.id} />);
+    const component = shallow(<TreeNodeComponent openItems={[]} node={node} treeNodeId={node.id} />);
     expect(component).toMatchSnapshot();
   });
 
@@ -54,7 +54,7 @@ describe("TreeNodeComponent", () => {
     const node = generateTestNode("root", {
       onExpanded: () => loading.promise,
     });
-    const component = shallow(<TreeNodeComponent node={node} treeNodeId={node.id} />);
+    const component = shallow(<TreeNodeComponent openItems={[]} node={node} treeNodeId={node.id} />);
 
     act(() => {
       component
@@ -72,10 +72,7 @@ describe("TreeNodeComponent", () => {
     const node = generateTestNode("root", {
       isSelected: () => true,
     });
-    const component = shallow(<TreeNodeComponent node={node} treeNodeId={node.id} />);
-    expect(component.find(TreeItemLayout).props().style?.backgroundColor).toStrictEqual(
-      tokens.colorNeutralBackground1Selected,
-    );
+    const component = shallow(<TreeNodeComponent openItems={[]} node={node} treeNodeId={node.id} />);
     expect(component).toMatchSnapshot();
   });
 
@@ -89,10 +86,7 @@ describe("TreeNodeComponent", () => {
         generateTestNode("child2"),
       ],
     });
-    const component = shallow(<TreeNodeComponent node={node} treeNodeId={node.id} />);
-    expect(component.find(TreeItemLayout).props().style?.backgroundColor).toStrictEqual(
-      tokens.colorNeutralBackground1Selected,
-    );
+    const component = shallow(<TreeNodeComponent openItems={[]} node={node} treeNodeId={node.id} />);
     expect(component).toMatchSnapshot();
   });
 
@@ -111,7 +105,7 @@ describe("TreeNodeComponent", () => {
         generateTestNode("child2"),
       ],
     });
-    const component = shallow(<TreeNodeComponent node={node} treeNodeId={node.id} />);
+    const component = shallow(<TreeNodeComponent openItems={[]} node={node} treeNodeId={node.id} />);
     expect(component.find(TreeItemLayout).props().style?.backgroundColor).toBeUndefined();
     expect(component).toMatchSnapshot();
   });
@@ -120,7 +114,7 @@ describe("TreeNodeComponent", () => {
     const node = generateTestNode("root", {
       iconSrc: "the-icon.svg",
     });
-    const component = shallow(<TreeNodeComponent node={node} treeNodeId={node.id} />);
+    const component = shallow(<TreeNodeComponent openItems={[]} node={node} treeNodeId={node.id} />);
     expect(component).toMatchSnapshot();
   });
 
@@ -134,7 +128,7 @@ describe("TreeNodeComponent", () => {
         generateTestNode("child2"),
       ],
     });
-    const component = shallow(<TreeNodeComponent node={node} treeNodeId={node.id} />);
+    const component = shallow(<TreeNodeComponent openItems={[]} node={node} treeNodeId={node.id} />);
     expect(component).toMatchSnapshot();
   });
 
@@ -148,7 +142,7 @@ describe("TreeNodeComponent", () => {
         generateTestNode("child2"),
       ],
     });
-    const component = shallow(<TreeNodeComponent node={node} treeNodeId={node.id} />);
+    const component = shallow(<TreeNodeComponent openItems={[]} node={node} treeNodeId={node.id} />);
     expect(component).toMatchSnapshot();
   });
 
@@ -175,7 +169,7 @@ describe("TreeNodeComponent", () => {
         }),
       ],
     });
-    const component = mount(<TreeNodeComponent node={node} treeNodeId={node.id} />);
+    const component = mount(<TreeNodeComponent openItems={[]} node={node} treeNodeId={node.id} />);
 
     // Find and expand the child3Expanding node
     const expandingChild = component.find(TreeItem).filterWhere((n) => n.props().value === "root/child3ExpandingLabel");

src/Explorer/Controls/TreeComponent/TreeNodeComponent.tsx

@@ -11,11 +11,13 @@ import {
   Tree,
   TreeItem,
   TreeItemLayout,
+  TreeItemValue,
   TreeOpenChangeData,
   TreeOpenChangeEvent,
+  mergeClasses,
 } from "@fluentui/react-components";
-import { MoreHorizontal20Regular } from "@fluentui/react-icons";
-import { tokens } from "@fluentui/react-theme";
+import { ChevronDown20Regular, ChevronRight20Regular, MoreHorizontal20Regular } from "@fluentui/react-icons";
+import { TreeStyleName, useTreeStyles } from "Explorer/Controls/TreeComponent/Styles";
 import * as React from "react";
 import { useCallback } from "react";
 
@@ -32,15 +34,14 @@ export interface TreeNode {
   id?: string;
   children?: TreeNode[];
   contextMenu?: TreeNodeMenuItem[];
-  iconSrc?: string;
+  iconSrc?: string | JSX.Element;
   isExpanded?: boolean;
-  className?: string;
+  className?: TreeStyleName;
   isAlphaSorted?: boolean;
   // data?: any; // Piece of data corresponding to this node
   timestamp?: number;
   isLeavesParentsSeparate?: boolean; // Display parents together first, then leaves
   isLoading?: boolean;
-  isScrollable?: boolean;
   isSelected?: () => boolean;
   onClick?: () => void; // Only if a leaf, other click will expand/collapse
   onExpanded?: () => Promise<void>;
@@ -52,6 +53,7 @@ export interface TreeNodeComponentProps {
   node: TreeNode;
   className?: string;
   treeNodeId: string;
+  openItems: TreeItemValue[];
 }
 
 /** Function that returns true if any descendant (at any depth) of this node is selected. */
@@ -66,13 +68,13 @@ function isAnyDescendantSelected(node: TreeNode): boolean {
   );
 }
 
-const getTreeIcon = (iconSrc: string): JSX.Element => <img src={iconSrc} alt="" style={{ width: 16, height: 16 }} />;
-
 export const TreeNodeComponent: React.FC<TreeNodeComponentProps> = ({
   node,
   treeNodeId,
+  openItems,
 }: TreeNodeComponentProps): JSX.Element => {
   const [isLoading, setIsLoading] = React.useState<boolean>(false);
+  const treeStyles = useTreeStyles();
 
   const getSortedChildren = (treeNode: TreeNode): TreeNode[] => {
     if (!treeNode || !treeNode.children) {
@@ -145,45 +147,72 @@ export const TreeNodeComponent: React.FC<TreeNodeComponentProps> = ({
     </MenuItem>
   ));
 
+  // We use the expandIcon slot to hold the node icon too.
+  // We only show a node icon for leaf nodes, even if a branch node has an iconSrc.
+  const expandIcon = isLoading ? (
+    <Spinner size="extra-tiny" />
+  ) : !isBranch ? (
+    typeof node.iconSrc === "string" ? (
+      <img src={node.iconSrc} className={treeStyles.nodeIcon} alt="" />
+    ) : (
+      node.iconSrc
+    )
+  ) : openItems.includes(treeNodeId) ? (
+    <ChevronDown20Regular />
+  ) : (
+    <ChevronRight20Regular />
+  );
+
   const treeItem = (
     <TreeItem
       data-test={`TreeNodeContainer:${treeNodeId}`}
       value={treeNodeId}
       itemType={isBranch ? "branch" : "leaf"}
       onOpenChange={onOpenChange}
+      className={treeStyles.treeItem}
     >
       <TreeItemLayout
-        className={node.className}
+        className={mergeClasses(
+          treeStyles.treeItemLayout,
+          expandIcon ? undefined : treeStyles.treeItemLayoutNoIcon,
+          shouldShowAsSelected && treeStyles.selectedItem,
+          node.className && treeStyles[node.className],
+        )}
         data-test={`TreeNode:${treeNodeId}`}
         actions={
-          contextMenuItems.length > 0 && (
-            <Menu onOpenChange={onMenuOpenChange}>
-              <MenuTrigger disableButtonEnhancement>
-                <Button
-                  aria-label="More options"
-                  data-test="TreeNode/ContextMenuTrigger"
-                  appearance="subtle"
-                  icon={<MoreHorizontal20Regular />}
-                />
-              </MenuTrigger>
-              <MenuPopover data-test={`TreeNode/ContextMenu:${treeNodeId}`}>
-                <MenuList>{contextMenuItems}</MenuList>
-              </MenuPopover>
-            </Menu>
-          )
+          contextMenuItems.length > 0 && {
+            className: treeStyles.actionsButtonContainer,
+            children: (
+              <Menu onOpenChange={onMenuOpenChange}>
+                <MenuTrigger disableButtonEnhancement>
+                  <Button
+                    aria-label="More options"
+                    className={mergeClasses(treeStyles.actionsButton, shouldShowAsSelected && treeStyles.selectedItem)}
+                    data-test="TreeNode/ContextMenuTrigger"
+                    appearance="subtle"
+                    icon={<MoreHorizontal20Regular />}
+                  />
+                </MenuTrigger>
+                <MenuPopover data-test={`TreeNode/ContextMenu:${treeNodeId}`}>
+                  <MenuList>{contextMenuItems}</MenuList>
+                </MenuPopover>
+              </Menu>
+            ),
+          }
         }
-        expandIcon={isLoading ? <Spinner size="extra-tiny" /> : undefined}
-        iconBefore={node.iconSrc && getTreeIcon(node.iconSrc)}
-        style={{
-          backgroundColor: shouldShowAsSelected ? tokens.colorNeutralBackground1Selected : undefined,
-        }}
+        expandIcon={expandIcon}
       >
-        {node.label}
+        <span className={treeStyles.nodeLabel}>{node.label}</span>
       </TreeItemLayout>
       {!node.isLoading && node.children?.length > 0 && (
-        <Tree style={{ overflow: node.isScrollable ? "auto" : undefined }}>
+        <Tree className={treeStyles.tree}>
           {getSortedChildren(node).map((childNode: TreeNode) => (
-            <TreeNodeComponent key={childNode.label} node={childNode} treeNodeId={`${treeNodeId}/${childNode.label}`} />
+            <TreeNodeComponent
+              openItems={openItems}
+              key={childNode.label}
+              node={childNode}
+              treeNodeId={`${treeNodeId}/${childNode.label}`}
+            />
           ))}
         </Tree>
       )}