Update on Diff tree UX/UI (#4180)

frontend/app/src/screens/diff/diff-tree.tsx

@@ -1,10 +1,12 @@
 import { Tree, TreeItemProps, TreeProps } from "@/components/ui/tree";
 import { useEffect, useState } from "react";
 import { addItemsToTree, EMPTY_TREE } from "@/screens/ipam/ipam-tree/utils";
-import { TREE_ROOT_ID } from "@/screens/ipam/constants";
 import { DiffBadge } from "@/screens/diff/node-diff/utils";
 import { DiffNode } from "@/screens/diff/node-diff/types";
 import { useLocation, useNavigate } from "react-router-dom";
+import { TREE_ROOT_ID } from "@/screens/ipam/constants";
+import { useSchema } from "@/hooks/useSchema";
+import { Icon } from "@iconify-icon/react";
 
 interface DiffTreeProps extends Omit<TreeProps, "data"> {
   nodes: Array<DiffNode>;
@@ -16,11 +18,16 @@ export default function DiffTree({ nodes, loading, ...props }: DiffTreeProps) {
   const navigate = useNavigate();
 
   useEffect(() => {
+    let kindToUUIDsMap: Record<string, Array<string>> = {};
+
     const formattedNodes: TreeProps["data"] = nodes.flatMap((node: DiffNode) => {
-      const newItem = {
+      const currentUUIDs = kindToUUIDsMap[node.kind];
+      kindToUUIDsMap[node.kind] = currentUUIDs ? [...currentUUIDs, node.uuid] : [node.uuid];
+
+      const newNode = {
         id: node.uuid,
         name: node.label,
-        parent: node.parent?.uuid ?? TREE_ROOT_ID,
+        parent: node.kind,
         children: [] as string[],
         isBranch: node.relationships.length > 0,
         metadata: {
@@ -30,11 +37,11 @@ export default function DiffTree({ nodes, loading, ...props }: DiffTreeProps) {
         },
       };
 
-      const newItemFromRelationships = node.relationships.flatMap((relationship) => {
-        const relationshipTreeItem = {
-          id: relationship?.name + newItem.id,
+      const nodesFromRelationships = node.relationships.flatMap((relationship) => {
+        const relationshipNode = {
+          id: relationship?.name + newNode.id,
           name: relationship?.label,
-          parent: newItem.id,
+          parent: newNode.id,
           isBranch: !!relationship?.elements?.length,
           children: [] as string[],
           metadata: {
@@ -43,39 +50,57 @@ export default function DiffTree({ nodes, loading, ...props }: DiffTreeProps) {
           },
         };
 
-        newItem.children.push(relationshipTreeItem.id);
+        newNode.children.push(relationshipNode.id);
 
-        const relationshipChildrenTreeItem =
+        const relationshipChildNodes =
           relationship?.elements?.map((element) => {
             const child = {
-              id: newItem.id + element.peer_label + element?.peer_id,
+              id: newNode.id + element.peer_label + element?.peer_id,
               name: element.peer_label,
-              parent: relationshipTreeItem.id,
+              parent: relationshipNode.id,
               isBranch: false,
               children: [],
               metadata: {
                 uuid: element?.peer_id,
-                status: element.status,
+                status: nodes.find(({ uuid }) => uuid === element.peer_id)?.status,
                 containsConflicts: element.contains_conflict,
               },
             };
-            relationshipTreeItem.children.push(child.id);
+            relationshipNode.children.push(child.id);
 
             return child;
           }) ?? [];
 
-        return [relationshipTreeItem, ...relationshipChildrenTreeItem];
+        return [relationshipNode, ...relationshipChildNodes];
       });
 
-      return [newItem, ...newItemFromRelationships];
+      return [newNode, ...nodesFromRelationships];
     });
 
-    setTreeData(addItemsToTree(EMPTY_TREE, formattedNodes));
+    const parents = Object.entries(kindToUUIDsMap).map(([kind, uuids]) => {
+      return {
+        id: kind,
+        name: kind,
+        parent: TREE_ROOT_ID,
+        children: uuids,
+        isBranch: true,
+        metadata: {
+          kind,
+        },
+      };
+    });
+    setTreeData(addItemsToTree(EMPTY_TREE, [...parents, ...formattedNodes]));
   }, [nodes.length]);
 
+  if (treeData.length <= 1) return null;
+
   return (
     <Tree
       itemContent={DiffTreeItem}
+      propagateSelectUpwards={false}
+      defaultExpandedIds={treeData
+        .filter(({ parent }) => parent === TREE_ROOT_ID)
+        .map(({ id }) => id)}
       loading={loading}
       data={treeData}
       onNodeSelect={({ element }) => {
@@ -91,12 +116,22 @@ export default function DiffTree({ nodes, loading, ...props }: DiffTreeProps) {
 
 const DiffTreeItem = ({ element }: TreeItemProps) => {
   const diffNode = element.metadata as DiffNode | undefined;
+  const { schema } = useSchema(diffNode?.kind);
+
+  if (schema) {
+    return (
+      <div className={"flex items-center gap-2 text-gray-800"} data-testid="hierarchical-tree-item">
+        <Icon icon={schema.icon as string} />
+        <span className="whitespace-nowrap">{schema.label}</span>
+      </div>
+    );
+  }
 
   return (
     <a
       href={"#" + diffNode?.uuid}
       tabIndex={-1}
-      className="flex items-center gap-2"
+      className="flex items-center gap-2 text-gray-800"
       data-testid="hierarchical-tree-item">
       <DiffBadge
         status={element.metadata?.status as string}

frontend/app/src/screens/diff/node-diff/index.tsx

@@ -17,7 +17,7 @@ import { StringParam, useQueryParam } from "use-query-params";
 import { QSP } from "@/config/qsp";
 import NoDataFound from "@/screens/errors/no-data-found";
 import { PcDiffUpdateButton } from "@/screens/proposed-changes/action-button/pc-diff-update-button";
-import { CardWithBorder } from "@/components/ui/card";
+import { Card } from "@/components/ui/card";
 import DiffTree from "@/screens/diff/diff-tree";
 import type { DiffNode as DiffNodeType } from "@/screens/diff/node-diff/types";
 import { Button } from "@/components/buttons/button-primitive";
@@ -114,9 +114,9 @@ export const NodeDiff = ({ filters }: NodeDiffProps) => {
       </div>
 
       <div className="flex-grow grid grid-cols-4 overflow-hidden">
-        <CardWithBorder className="col-span-1 my-2.5 ml-2.5">
+        <Card className="col-span-1 my-2.5 ml-2.5 overflow-auto">
           <DiffTree nodes={nodes} className="p-2 w-full" />
-        </CardWithBorder>
+        </Card>
 
         <div className="space-y-4 p-2.5 col-start-2 col-end-5 overflow-auto">
           {nodes.map((node) => (

frontend/app/src/screens/diff/node-diff/node-attribute.tsx

@@ -1,15 +1,15 @@
 import { useParams } from "react-router-dom";
-import { Badge } from "@/components/ui/badge";
 import { DiffRow, formatValue } from "@/screens/diff/node-diff/utils";
-import { DiffAttribute } from "@/screens/diff/node-diff/types";
+import { DiffAttribute, DiffStatus } from "@/screens/diff/node-diff/types";
 import { DiffThread } from "@/screens/diff/node-diff/thread";
 import { DiffNodeProperty } from "@/screens/diff/node-diff/node-property";
 
 type DiffNodeAttributeProps = {
   attribute: DiffAttribute;
+  status: DiffStatus;
 };
 
-export const DiffNodeAttribute = ({ attribute }: DiffNodeAttributeProps) => {
+export const DiffNodeAttribute = ({ attribute, status }: DiffNodeAttributeProps) => {
   const { "*": branchName } = useParams();
 
   const valueProperty = attribute.properties.find(
@@ -18,6 +18,7 @@ export const DiffNodeAttribute = ({ attribute }: DiffNodeAttributeProps) => {
 
   return (
     <DiffRow
+      status={status}
       hasConflicts={attribute.contains_conflict}
       title={
         <div className="flex justify-between items-center pr-2">
@@ -28,21 +29,11 @@ export const DiffNodeAttribute = ({ attribute }: DiffNodeAttributeProps) => {
           )}
         </div>
       }
-      left={
-        valueProperty?.previous_value && (
-          <Badge variant="green" className="font-medium">
-            {formatValue(valueProperty?.previous_value)}
-          </Badge>
-        )
-      }
-      right={
-        <Badge variant="blue" className="font-medium">
-          {formatValue(valueProperty?.new_value)}
-        </Badge>
-      }>
+      left={valueProperty?.previous_value && formatValue(valueProperty?.previous_value)}
+      right={status !== "REMOVED" && formatValue(valueProperty?.new_value)}>
       <div className="divide-y border-t">
         {attribute.properties.map((property, index: number) => (
-          <DiffNodeProperty key={index} property={property} />
+          <DiffNodeProperty key={index} property={property} status={status} />
         ))}
       </div>
     </DiffRow>

frontend/app/src/screens/diff/node-diff/node-property.tsx

@@ -5,32 +5,27 @@ import { Icon } from "@iconify-icon/react";
 import { Conflict } from "./conflict";
 import { DiffRow, formatPropertyName, formatValue } from "@/screens/diff/node-diff/utils";
 import { BadgeConflict } from "@/screens/diff/diff-badge";
-import { DiffProperty } from "@/screens/diff/node-diff/types";
+import { DiffProperty, DiffStatus } from "@/screens/diff/node-diff/types";
 import { classNames } from "@/utils/common";
 
 type DiffNodePropertyProps = {
-  property: DiffProperty;
   className?: string;
+  property: DiffProperty;
+  status: DiffStatus;
 };
 
 const getPreviousValue = (property: DiffProperty) => {
   const previousValue = formatValue(property.previous_value);
   if (previousValue === null) return null;
 
   if (!property.conflict) {
-    return (
-      <Badge variant="green" className="bg-transparent font-medium">
-        {previousValue}
-      </Badge>
-    );
+    return previousValue;
   }
 
   const conflictValue = formatValue(property.conflict.base_branch_value);
   return (
     <div className="flex items-center gap-2">
-      <Badge variant="green" className="bg-transparent font-medium">
-        {previousValue}
-      </Badge>
+      {previousValue}
       <Icon icon="mdi:chevron-right" />
       <Badge variant="yellow" className="font-medium">
         {conflictValue}
@@ -44,11 +39,7 @@ const getNewValue = (property: DiffProperty) => {
   if (newValue === null) return null;
 
   if (!property.conflict) {
-    return (
-      <Badge variant="blue" className="bg-transparent font-medium">
-        {newValue}
-      </Badge>
-    );
+    return newValue;
   }
 
   const conflictValue = formatValue(property.conflict.diff_branch_value);
@@ -59,11 +50,12 @@ const getNewValue = (property: DiffProperty) => {
   );
 };
 
-export const DiffNodeProperty = ({ property, className }: DiffNodePropertyProps) => {
+export const DiffNodeProperty = ({ status, property, className }: DiffNodePropertyProps) => {
   const { "*": branchName } = useParams();
 
   return (
     <DiffRow
+      status={status}
       iconClassName="left-4"
       hasConflicts={!!property.conflict}
       title={
@@ -79,6 +71,8 @@ export const DiffNodeProperty = ({ property, className }: DiffNodePropertyProps)
         </div>
       }
       left={getPreviousValue(property)}
+      leftClassName="font-normal"
+      rightClassName="font-normal"
       right={getNewValue(property)}>
       {property.conflict && <Conflict conflict={property.conflict} />}
     </DiffRow>

frontend/app/src/screens/diff/node-diff/node-relationship-element.tsx

@@ -1,20 +1,21 @@
 import { DiffNodeProperty } from "./node-property";
-import { DiffRelationshipElement } from "@/screens/diff/node-diff/types";
+import { DiffRelationshipElement, DiffStatus } from "@/screens/diff/node-diff/types";
 import { DiffBadge, DiffRow } from "@/screens/diff/node-diff/utils";
 import { BadgeConflict } from "@/screens/diff/diff-badge";
 import { DiffThread } from "@/screens/diff/node-diff/thread";
 import { useParams } from "react-router-dom";
-import { Badge } from "@/components/ui/badge";
 
 type DiffNodeElementProps = {
   element: DiffRelationshipElement;
+  status: DiffStatus;
 };
 
-export const DiffNodeRelationshipElement = ({ element }: DiffNodeElementProps) => {
+export const DiffNodeRelationshipElement = ({ element, status }: DiffNodeElementProps) => {
   const { "*": branchName } = useParams();
 
   return (
     <DiffRow
+      status={status}
       iconClassName="left-4"
       title={
         <div className="flex items-center justify-between pl-4 pr-2">
@@ -26,25 +27,13 @@ export const DiffNodeRelationshipElement = ({ element }: DiffNodeElementProps) =
           {!branchName && element.path_identifier && <DiffThread path={element.path_identifier} />}
         </div>
       }
-      right={
-        element.status === "ADDED" && (
-          <Badge variant="blue" className="bg-transparent">
-            {element.peer_label}
-          </Badge>
-        )
-      }
-      left={
-        element.status === "REMOVED" && (
-          <Badge variant="green" className="bg-transparent">
-            {element.peer_label}
-          </Badge>
-        )
-      }>
+      right={element.status === "ADDED" && element.peer_label}
+      left={element.status === "REMOVED" && element.peer_label}>
       <div className="divide-y border-t">
         {element.properties
           .filter((property) => property.status !== "UNCHANGED")
           .map((property, index) => (
-            <DiffNodeProperty key={index} property={property} className="pl-8" />
+            <DiffNodeProperty key={index} property={property} status={status} className="pl-8" />
           ))}
       </div>
     </DiffRow>

frontend/app/src/screens/diff/node-diff/node-relationship.tsx

@@ -2,22 +2,24 @@ import { DiffNodeRelationshipElement } from "./node-relationship-element";
 import { useParams } from "react-router-dom";
 import { DiffThread } from "@/screens/diff/node-diff/thread";
 import { DiffRow } from "@/screens/diff/node-diff/utils";
-import { DiffRelationship } from "@/screens/diff/node-diff/types";
+import { DiffRelationship, DiffStatus } from "@/screens/diff/node-diff/types";
 import { Badge } from "@/components/ui/badge";
 import { Icon } from "@iconify-icon/react";
 
 type DiffNodeRelationshipProps = {
   relationship: DiffRelationship;
+  status: DiffStatus;
 };
 
-export const DiffNodeRelationship = ({ relationship }: DiffNodeRelationshipProps) => {
+export const DiffNodeRelationship = ({ status, relationship }: DiffNodeRelationshipProps) => {
   const { "*": branchName } = useParams();
 
   const AddedCount = relationship.elements.filter(({ status }) => status === "ADDED").length;
   const RemovedCount = relationship.elements.filter(({ status }) => status === "REMOVED").length;
   const UpdatedCount = relationship.elements.filter(({ status }) => status === "UPDATED").length;
   return (
     <DiffRow
+      status={status}
       hasConflicts={relationship.contains_conflict}
       title={
         <div className="flex justify-between items-center pr-2">
@@ -52,7 +54,7 @@ export const DiffNodeRelationship = ({ relationship }: DiffNodeRelationshipProps
       }>
       <div className="divide-y border-t">
         {relationship.elements.map((element, index: number) => (
-          <DiffNodeRelationshipElement key={index} element={element} />
+          <DiffNodeRelationshipElement key={index} element={element} status={status} />
         ))}
       </div>
     </DiffRow>

frontend/app/src/screens/diff/node-diff/node.tsx

@@ -65,10 +65,10 @@ export const DiffNode = ({ sourceBranch, destinationBranch, node }: DiffNodeProp
           className="bg-gray-100 border rounded-md">
           <div className="divide-y border-t">
             {node.attributes.map((attribute: any, index: number) => (
-              <DiffNodeAttribute key={index} attribute={attribute} />
+              <DiffNodeAttribute key={index} attribute={attribute} status={node.status} />
             ))}
             {node.relationships.map((relationship: any, index: number) => (
-              <DiffNodeRelationship key={index} relationship={relationship} />
+              <DiffNodeRelationship key={index} relationship={relationship} status={node.status} />
             ))}
           </div>
         </Accordion>