Merge pull request #587 from amelioro/clutter-improvements

Clutter improvements

src/web/common/components/ContextMenu/ChangeEdgeTypeMenuItem.tsx

@@ -5,6 +5,7 @@ import { getSameCategoryEdgeTypes } from "@/common/edge";
 import { ContextMenuItem } from "@/web/common/components/ContextMenu/CloseOnClickMenuItem";
 import { useSessionUser } from "@/web/common/hooks";
 import { changeEdgeType } from "@/web/topic/store/actions";
+import { useIsTableEdge } from "@/web/topic/store/edgeHooks";
 import { useUserCanEditTopicData } from "@/web/topic/store/userHooks";
 import { Edge } from "@/web/topic/utils/graph";
 
@@ -17,7 +18,11 @@ export const ChangeEdgeTypeMenuItem = ({ edge, parentMenuOpen }: Props) => {
   const { sessionUser } = useSessionUser();
   const userCanEditTopicData = useUserCanEditTopicData(sessionUser?.username);
 
+  const isTableEdge = useIsTableEdge(edge.id);
+
   if (!userCanEditTopicData) return <></>;
+  // don't allow modifying edges that are part of the table, because they should always exist as long as their nodes do
+  if (isTableEdge) return <></>;
 
   return (
     <>

src/web/common/components/ContextMenu/ContextMenu.tsx

@@ -9,6 +9,9 @@ import { DeleteEdgeMenuItem } from "@/web/common/components/ContextMenu/DeleteEd
 import { DeleteNodeMenuItem } from "@/web/common/components/ContextMenu/DeleteNodeMenuItem";
 import { HideMenuItem } from "@/web/common/components/ContextMenu/HideMenuItem";
 import { OnlyShowNodeAndNeighborsMenuItem } from "@/web/common/components/ContextMenu/OnlyShowNodeAndNeighborsMenuItem";
+import { ViewContextMenuItem } from "@/web/common/components/ContextMenu/ViewContextMenuItem";
+import { ViewDetailsMenuItem } from "@/web/common/components/ContextMenu/ViewDetailsMenuItem";
+import { ViewTableMenuItem } from "@/web/common/components/ContextMenu/ViewTableMenuItem";
 import { closeContextMenu } from "@/web/common/store/contextMenuActions";
 import { useAnchorPosition, useContextMenuContext } from "@/web/common/store/contextMenuStore";
 
@@ -20,29 +23,28 @@ export const ContextMenu = () => {
 
   const isOpen = Boolean(anchorPosition);
 
+  const contextNode = contextMenuContext.node;
+  const contextEdge = contextMenuContext.edge;
+  const contextPart = contextNode ?? contextEdge;
+
   // create these based on what's set in the context
   const menuItems = [
+    // view actions (so that this functionality is still available if indicators are hidden)
+    contextPart && <ViewDetailsMenuItem graphPart={contextPart} key={1} />,
+    contextNode?.type === "problem" && <ViewTableMenuItem node={contextNode} key={2} />,
+    contextPart && <ViewContextMenuItem graphPart={contextPart} key={3} />,
+
     // CRUD actions
-    !contextMenuContext.node && !contextMenuContext.edge && (
-      <AddNodeMenuItem parentMenuOpen={isOpen} key={9} />
-    ),
-    contextMenuContext.node && (
-      <ChangeNodeTypeMenuItem node={contextMenuContext.node} parentMenuOpen={isOpen} key={7} />
-    ),
-    contextMenuContext.edge && (
-      <ChangeEdgeTypeMenuItem edge={contextMenuContext.edge} parentMenuOpen={isOpen} key={8} />
-    ),
-    contextMenuContext.node && <DeleteNodeMenuItem node={contextMenuContext.node} key={5} />,
-    contextMenuContext.edge && <DeleteEdgeMenuItem edge={contextMenuContext.edge} key={6} />,
+    contextPart === undefined && <AddNodeMenuItem parentMenuOpen={isOpen} key={9} />,
+    contextNode && <ChangeNodeTypeMenuItem node={contextNode} parentMenuOpen={isOpen} key={7} />,
+    contextEdge && <ChangeEdgeTypeMenuItem edge={contextEdge} parentMenuOpen={isOpen} key={8} />,
+    contextNode && <DeleteNodeMenuItem node={contextNode} key={5} />,
+    contextEdge && <DeleteEdgeMenuItem edge={contextEdge} key={6} />,
 
     // show/hide actions
-    contextMenuContext.node && (
-      <AlsoShowNodeAndNeighborsMenuItem node={contextMenuContext.node} key={11} />
-    ),
-    contextMenuContext.node && (
-      <OnlyShowNodeAndNeighborsMenuItem node={contextMenuContext.node} key={12} />
-    ),
-    contextMenuContext.node && <HideMenuItem node={contextMenuContext.node} key={13} />,
+    contextNode && <AlsoShowNodeAndNeighborsMenuItem node={contextNode} key={11} />,
+    contextNode && <OnlyShowNodeAndNeighborsMenuItem node={contextNode} key={12} />,
+    contextNode && <HideMenuItem node={contextNode} key={13} />,
 
     // ensure there's never an empty context menu; that shows an empty bubble and feels awkward
     <ContextMenuItem key={10}>Cancel</ContextMenuItem>,

src/web/common/components/ContextMenu/DeleteEdgeMenuItem.tsx

@@ -2,16 +2,21 @@ import { justificationRelationNames } from "@/common/edge";
 import { ContextMenuItem } from "@/web/common/components/ContextMenu/CloseOnClickMenuItem";
 import { useSessionUser } from "@/web/common/hooks";
 import { deleteEdge } from "@/web/topic/store/createDeleteActions";
+import { useIsTableEdge } from "@/web/topic/store/edgeHooks";
 import { useUserCanEditTopicData } from "@/web/topic/store/userHooks";
 import { Edge } from "@/web/topic/utils/graph";
 
 export const DeleteEdgeMenuItem = ({ edge }: { edge: Edge }) => {
   const { sessionUser } = useSessionUser();
   const userCanEditTopicData = useUserCanEditTopicData(sessionUser?.username);
 
+  const isTableEdge = useIsTableEdge(edge.id);
+
   if (!userCanEditTopicData) return <></>;
   // doesn't make sense to delete justification edges because they're a tree not a graph - just delete the nodes
   if (justificationRelationNames.includes(edge.label)) return <></>;
+  // don't allow modifying edges that are part of the table, because they should always exist as long as their nodes do
+  if (isTableEdge) return <></>;
 
   return <ContextMenuItem onClick={() => deleteEdge(edge.id)}>Delete edge</ContextMenuItem>;
 };

src/web/common/components/ContextMenu/ViewContextMenuItem.tsx

@@ -0,0 +1,12 @@
+import { ContextMenuItem } from "@/web/common/components/ContextMenu/CloseOnClickMenuItem";
+import { GraphPart, isNode } from "@/web/topic/utils/graph";
+import { contextMethods } from "@/web/topic/utils/partContext";
+
+export const ViewContextMenuItem = ({ graphPart }: { graphPart: GraphPart }) => {
+  const partType = isNode(graphPart) ? graphPart.type : graphPart.label;
+  const viewContext = contextMethods[partType]?.viewContext;
+
+  if (!viewContext) return <></>;
+
+  return <ContextMenuItem onClick={() => viewContext(graphPart.id)}>View context</ContextMenuItem>;
+};

src/web/common/components/ContextMenu/ViewDetailsMenuItem.tsx

@@ -0,0 +1,17 @@
+import { ContextMenuItem } from "@/web/common/components/ContextMenu/CloseOnClickMenuItem";
+import { emitter } from "@/web/common/event";
+import { GraphPart } from "@/web/topic/utils/graph";
+import { setSelected } from "@/web/view/currentViewStore/store";
+
+export const ViewDetailsMenuItem = ({ graphPart }: { graphPart: GraphPart }) => {
+  return (
+    <ContextMenuItem
+      onClick={() => {
+        setSelected(graphPart.id);
+        emitter.emit("viewTopicDetails");
+      }}
+    >
+      View details
+    </ContextMenuItem>
+  );
+};

src/web/common/components/ContextMenu/ViewTableMenuItem.tsx

@@ -0,0 +1,11 @@
+import { ContextMenuItem } from "@/web/common/components/ContextMenu/CloseOnClickMenuItem";
+import { Node } from "@/web/topic/utils/graph";
+import { viewCriteriaTable } from "@/web/view/currentViewStore/filter";
+
+export const ViewTableMenuItem = ({ node }: { node: Node }) => {
+  return (
+    <ContextMenuItem onClick={() => viewCriteriaTable(node.id)}>
+      View criteria table
+    </ContextMenuItem>
+  );
+};

src/web/topic/components/CriteriaTable/EdgeCell.tsx

@@ -1,10 +1,14 @@
+import { openContextMenu } from "@/web/common/store/contextMenuActions";
 import { CommonIndicators } from "@/web/topic/components/Indicator/CommonIndicators";
 import { ContentIndicators } from "@/web/topic/components/Indicator/ContentIndicators";
 import { Edge } from "@/web/topic/utils/graph";
 
 export const EdgeCell = ({ edge }: { edge: Edge }) => {
   return (
-    <div className="flex h-full flex-col items-center justify-center">
+    <div
+      className="flex h-full flex-col items-center justify-center"
+      onContextMenu={(event) => openContextMenu(event, { edge })}
+    >
       <CommonIndicators graphPart={edge} notes={edge.data.notes} />
       <ContentIndicators
         className="ml-0"

src/web/topic/components/Edge/ScoreEdge.tsx

@@ -125,6 +125,21 @@ export const ScoreEdge = ({ inReactFlow, ...flowEdge }: EdgeProps & Props) => {
     />
   );
 
+  /**
+   * Allow edges to be more-easily hovered/clicked, based on a wider width than what is visibly drawn.
+   * Taken from react flow's implementation https://github.com/xyflow/xyflow/blob/616d2665235447e0280368228ac64b987afecba0/packages/react/src/components/Edges/BaseEdge.tsx#L35-L43
+   */
+  const hiddenInteractivePath = (
+    <path
+      className="react-flow__edge-interaction"
+      d={pathDefinition}
+      fill="none"
+      strokeOpacity={0}
+      strokeWidth={20}
+      onClick={() => setSelected(edge.id)}
+    />
+  );
+
   const labelText = edge.data.customLabel ?? lowerCase(edge.label);
 
   const label = (
@@ -165,6 +180,7 @@ export const ScoreEdge = ({ inReactFlow, ...flowEdge }: EdgeProps & Props) => {
         {/* shouldn't need an svg marker def per edge, but it's easiest to just put here */}
         {svgMarkerDef(inReactFlow, spotlight)}
         {path}
+        {hiddenInteractivePath}
 
         {/* see for example usage https://reactflow.dev/docs/api/edges/edge-label-renderer/ */}
         <EdgeLabelRenderer>{label}</EdgeLabelRenderer>

src/web/topic/components/Node/NodeHandle.tsx

@@ -1,12 +1,13 @@
 import { Visibility } from "@mui/icons-material";
 import { IconButton, Tooltip, Typography } from "@mui/material";
 import { ReactNode, memo } from "react";
-import { Position } from "reactflow";
+import { Handle, Position } from "reactflow";
 
 import { nodeTypes } from "@/common/node";
-import { StyledHandle } from "@/web/topic/components/Node/NodeHandle.styles";
+import { useSessionUser } from "@/web/common/hooks";
 import { useHiddenNodes } from "@/web/topic/hooks/flowHooks";
 import { useNeighborsInDirection } from "@/web/topic/store/nodeHooks";
+import { useUserCanEditTopicData } from "@/web/topic/store/userHooks";
 import { Node, RelationDirection } from "@/web/topic/utils/graph";
 import { Orientation } from "@/web/topic/utils/layout";
 import { nodeDecorations } from "@/web/topic/utils/node";
@@ -33,6 +34,9 @@ interface Props {
 }
 
 const NodeHandleBase = ({ node, direction, orientation }: Props) => {
+  const { sessionUser } = useSessionUser();
+  const userCanEditTopicData = useUserCanEditTopicData(sessionUser?.username);
+
   const neighborsInDirection = useNeighborsInDirection(node.id, direction);
   const hiddenNeighbors = useHiddenNodes(neighborsInDirection);
 
@@ -42,6 +46,9 @@ const NodeHandleBase = ({ node, direction, orientation }: Props) => {
     return diff;
   });
 
+  const hasHiddenNeighbors = sortedHiddenNeighbors.length > 0;
+  const showHandle = userCanEditTopicData || hasHiddenNeighbors;
+
   const type = direction === "parent" ? "target" : "source";
 
   const position =
@@ -56,7 +63,7 @@ const NodeHandleBase = ({ node, direction, orientation }: Props) => {
   return (
     <Tooltip
       title={
-        sortedHiddenNeighbors.length > 0 ? (
+        hasHiddenNeighbors ? (
           <div className="space-y-2">
             {sortedHiddenNeighbors.map((neighbor) => (
               <NodeSummary
@@ -76,10 +83,14 @@ const NodeHandleBase = ({ node, direction, orientation }: Props) => {
       }
       disableFocusListener
     >
-      <StyledHandle
+      <Handle
         type={type}
         position={position}
-        hasHiddenComponents={sortedHiddenNeighbors.length > 0}
+        className={
+          "size-[10px]" +
+          (!showHandle ? " invisible" : "") +
+          (hasHiddenNeighbors ? " bg-info-main" : "")
+        }
       />
     </Tooltip>
   );

src/web/topic/components/TopicWorkspace/WorkspaceToolbar.tsx

@@ -19,6 +19,7 @@ import { useSessionUser } from "@/web/common/hooks";
 import { HelpMenu } from "@/web/topic/components/TopicWorkspace/HelpMenu";
 import { MoreActionsDrawer } from "@/web/topic/components/TopicWorkspace/MoreActionsDrawer";
 import { deleteGraphPart } from "@/web/topic/store/createDeleteActions";
+import { useIsTableEdge } from "@/web/topic/store/edgeHooks";
 import { useOnPlayground } from "@/web/topic/store/topicHooks";
 import { useUserCanEditTopicData } from "@/web/topic/store/userHooks";
 import { redo, undo } from "@/web/topic/store/utilActions";
@@ -44,15 +45,18 @@ import {
 export const WorkspaceToolbar = () => {
   const { sessionUser } = useSessionUser();
   const userCanEditTopicData = useUserCanEditTopicData(sessionUser?.username);
+
   const onPlayground = useOnPlayground();
   const [canUndo, canRedo] = useTemporalHooks();
   const [canGoBack, canGoForward] = useCanGoBackForward();
+
   const isComparingPerspectives = useIsComparingPerspectives();
   const flashlightMode = useFlashlightMode();
   const readonlyMode = useReadonlyMode();
   const [hasErrored, setHasErrored] = useState(false);
 
   const selectedGraphPart = useSelectedGraphPart();
+  const partIsTableEdge = useIsTableEdge(selectedGraphPart?.id ?? "");
 
   const [isMoreActionsDrawerOpen, setIsMoreActionsDrawerOpen] = useState(false);
   const [helpAnchorEl, setHelpAnchorEl] = useState<null | HTMLElement>(null);
@@ -124,7 +128,8 @@ export const WorkspaceToolbar = () => {
                   deleteGraphPart(selectedGraphPart);
                 }
               }}
-              disabled={!selectedGraphPart}
+              // don't allow modifying edges that are part of the table, because they should always exist as long as their nodes do
+              disabled={!selectedGraphPart || partIsTableEdge}
               className="hidden sm:flex"
             >
               <Delete />

src/web/topic/store/edgeHooks.ts

@@ -17,3 +17,17 @@ export const useIsNodeSelected = (edgeId: string) => {
 
   return useIsAnyGraphPartSelected(neighborNodes.map((node) => node.id));
 };
+
+export const useIsTableEdge = (edgeId: string) => {
+  return useTopicStore((state) => {
+    try {
+      const edge = findEdgeOrThrow(edgeId, state.edges);
+      if (edge.label !== "fulfills") return false;
+
+      const [parentNode, childNode] = nodes(edge, state.nodes);
+      return parentNode.type === "criterion" && childNode.type === "solution";
+    } catch {
+      return false;
+    }
+  });
+};

src/web/topic/utils/edge.ts

@@ -246,7 +246,7 @@ export const childNode = (edge: Edge, nodes: Node[]) => {
   return findNodeOrThrow(edge.target, nodes);
 };
 
-export const nodes = (edge: Edge, nodes: Node[]) => {
+export const nodes = (edge: Edge, nodes: Node[]): [Node, Node] => {
   return [parentNode(edge, nodes), childNode(edge, nodes)];
 };