Merge pull request #144 from UW-Macrostrat/feedback

Feedback update

Author: davidsklar99

packages/feedback-components/CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format
 is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this
 project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [unreleased]
+
+- Simplified match logic
+
 ## [1.1.4] - 2025-07-16
 
 - Add and remove match feature added

packages/feedback-components/src/feedback/edit-state.ts

@@ -17,6 +17,8 @@ interface TreeState {
   lastInternalId: number;
   isSelectingEntityType: boolean;
   viewMode: ViewMode;
+  viewOnly: boolean;
+  matchMode: boolean;
 }
 
 type TextRange = {
@@ -50,13 +52,16 @@ type TreeAction =
     }
   | { type: "select-range"; payload: { ids: number[] } }
   | { type: "add-match"; payload: { id: number; payload: any } }
-  | { type: "remove-match"; payload: { id: number } };
+  | { type: "remove-match"; payload: { id: number } }
+  | { type: "toggle-match-mode" };
 
 export type TreeDispatch = Dispatch<TreeAction>;
 
 export function useUpdatableTree(
   initialTree: TreeData[],
   entityTypes: Map<number, EntityType>,
+  viewOnly: boolean,
+  matchMode: boolean,
 ): [TreeState, TreeDispatch] {
   // Get the first entity type
   // issue: grabs second entity instead of selected one
@@ -71,6 +76,8 @@ export function useUpdatableTree(
     lastInternalId: 0,
     isSelectingEntityType: false,
     viewMode: ViewMode.Tree,
+    viewOnly,
+    matchMode,
   });
 }
 
@@ -85,6 +92,14 @@ export function useTreeDispatch() {
 }
 
 function treeReducer(state: TreeState, action: TreeAction) {
+  if (action.type === "toggle-match-mode") {
+    return { ...state, matchMode: !state.matchMode };
+  }
+
+  if (state.viewOnly) return viewMode(state, action);
+
+  if (state.matchMode) return matchMode(state, action);
+
   switch (action.type) {
     case "add-entity-type": {
       // Add a new entity type to the map
@@ -264,54 +279,6 @@ function treeReducer(state: TreeState, action: TreeAction) {
       };
     }
 
-    case "add-match": {
-      const { id } = action.payload;
-
-      // Find the node path
-      const keyPath = findNode(state.tree, id);
-      if (!keyPath) {
-        console.warn(`Node with id ${id} not found`);
-        return state;
-      }
-
-      // Build update spec to set the `match` property
-      const matchUpdateSpec = buildNestedSpec(keyPath, {
-        match: { $set: action.payload.payload },
-      });
-
-      const updatedTree = update(state.tree, matchUpdateSpec);
-
-      return {
-        ...state,
-        tree: updatedTree,
-      };
-    }
-
-    case "remove-match": {
-      const { id } = action.payload;
-
-      console.log("Removing match for node with id:", id);
-
-      // Find the node path
-      const keyPath = findNode(state.tree, id);
-      if (!keyPath) {
-        console.warn(`Node with id ${id} not found`);
-        return state;
-      }
-
-      // Build update spec to unset the `match` property
-      const matchUpdateSpec = buildNestedSpec(keyPath, {
-        match: { $set: null },
-      });
-
-      const updatedTree = update(state.tree, matchUpdateSpec);
-
-      return {
-        ...state,
-        tree: updatedTree,
-      };
-    }
-
     /** Entity type selection */
     case "toggle-entity-type-selector":
       return {
@@ -526,3 +493,83 @@ function flattenAndSort(nodes) {
   // sort by start
   return result.sort((a, b) => a.indices[0] - b.indices[0]);
 }
+
+function matchMode(state, action) {
+  if (action.type === "select-node" || action.type === "toggle-node-selected") {
+    const { ids } = action.payload;
+
+    if (ids.length != 1) return state;
+
+    if (state.selectedNodes.length === 1) {
+      if (ids[0] === state.selectedNodes[0]) {
+        // If the selected node is the same as the current selection, deselect it
+        return { ...state, selectedNodes: [] };
+      }
+    }
+
+    const type =
+      action.payload.ids.length > 0
+        ? findNodeById(state.tree, ids[0])?.type
+        : null;
+
+    return { ...state, selectedNodes: ids, selectedEntityType: type };
+  }
+
+  if (action.type === "add-match") {
+    const { id } = action.payload;
+
+    // Find the node path
+    const keyPath = findNode(state.tree, id);
+    if (!keyPath) {
+      console.warn(`Node with id ${id} not found`);
+      return state;
+    }
+
+    // Build update spec to set the `match` property
+    const matchUpdateSpec = buildNestedSpec(keyPath, {
+      match: { $set: action.payload.payload },
+    });
+
+    const updatedTree = update(state.tree, matchUpdateSpec);
+
+    return {
+      ...state,
+      tree: updatedTree,
+    };
+  }
+
+  if (action.type === "remove-match") {
+    const { id } = action.payload;
+
+    console.log("Removing match for node with id:", id);
+
+    // Find the node path
+    const keyPath = findNode(state.tree, id);
+    if (!keyPath) {
+      console.warn(`Node with id ${id} not found`);
+      return state;
+    }
+
+    // Build update spec to unset the `match` property
+    const matchUpdateSpec = buildNestedSpec(keyPath, {
+      match: { $set: null },
+    });
+
+    const updatedTree = update(state.tree, matchUpdateSpec);
+
+    return {
+      ...state,
+      tree: updatedTree,
+    };
+  }
+
+  return state;
+}
+
+function viewMode(state, action) {
+  if (action.type === "set-view-mode") {
+    return { ...state, viewMode: action.payload };
+  }
+
+  return state;
+}

packages/feedback-components/src/feedback/feedback.module.sass

@@ -31,7 +31,6 @@
   cursor: pointer
 
 circle
-  cursor: pointer
   border: 1px solid black
 
 .selected
@@ -79,12 +78,14 @@ mark
   width: auto !important
 
 .highlight
-  cursor: pointer
   padding: .2em 0
   border-radius: .2em
   position: relative
   zIndex: 10
 
+.clickable
+  cursor: pointer
+
 .feedback-text-wrapper 
   position: relative
   z-index: 0
@@ -115,7 +116,7 @@ mark
   align-items: center
 
   .add-type-overlay
-    background-color: var(--secondary-color)
+    background-color: var(--tertiary-background)
     padding: .5em 1em
     display: flex
     flex-direction: column
@@ -149,17 +150,20 @@ mark
   pointerEvents: none
 
 .match-item
-  background-color: var(--background-color)
   color: var(--text-emphasized-color)
   padding: .1em .2em
   border-radius: .2em
   margin-bottom: 4px
   cursor: pointer
+  
+  .match-label
+    margin: 0
+    color: var(--text-color)
 
 .match-container
   display: flex
   justify-content: space-between
   align-items: center
 
   .close-btn
-    cursor: pointer
+    cursor: pointer

packages/feedback-components/src/feedback/graph.ts

@@ -24,10 +24,11 @@ export function GraphView(props: {
   height: number;
   dispatch: (action: any) => void;
   selectedNodes: number[];
+  viewOnly?: boolean;
 }) {
   // A graph view with react-flow
   // Get positions of nodes using force simulation
-  const { tree, width, height, dispatch, selectedNodes } = props;
+  const { tree, width, height, dispatch, selectedNodes, viewOnly } = props;
 
   const [nodes, setNodes] = useState<SimulationNodeDatum[]>(null);
   const [links, setLinks] = useState<SimulationLinkDatum[]>(null);
@@ -134,7 +135,9 @@ export function GraphView(props: {
                     });
                   }
                 },
-                className: active ? "selected" : "",
+                className: active
+                  ? "selected"
+                  : "" + (viewOnly ? "" : " clickable"),
                 stroke,
                 strokeWidth: 2,
               }),

packages/feedback-components/src/feedback/index.ts

@@ -56,13 +56,19 @@ export function FeedbackComponent({
   onSave,
   allowOverlap,
   matchLinks,
+  view = false,
 }) {
+  const [viewOnly, setViewOnly] = useState(view);
+  const [match, setMatchLinks] = useState(matchLinks);
+  const matchMode = match !== null;
+
   // Get the input arguments
   const [state, dispatch] = useUpdatableTree(
     entities.map(processEntity) as any,
     entityTypes,
+    viewOnly,
+    matchMode,
   );
-  const [match, setMatchLinks] = useState(matchLinks || {});
 
   const {
     selectedNodes,
@@ -78,6 +84,18 @@ export function FeedbackComponent({
     h(
       "div.feedback-container",
       h(TreeDispatchContext.Provider, { value: dispatch }, [
+        h.if(!view)(SegmentedControl, {
+          options: [
+            { label: "View", value: "view" },
+            { label: "Edit", value: "edit" },
+          ],
+          value: viewOnly ? "view" : "edit",
+          small: true,
+          onValueChange() {
+            setViewOnly(!viewOnly);
+          },
+          role: "toolbar",
+        }),
         h(
           ErrorBoundary,
           {
@@ -92,6 +110,7 @@ export function FeedbackComponent({
             selectedNodes,
             allowOverlap,
             matchLinks: match,
+            viewOnly,
           }),
         ),
         h(
@@ -133,14 +152,15 @@ export function FeedbackComponent({
               height,
               dispatch,
               selectedNodes,
+              viewOnly,
             }),
           ],
         ),
       ]),
     ),
     h(Card, { className: "control-panel" }, [
       h("div.control-content", [
-        h(
+        h.if(!viewOnly)(
           ButtonGroup,
           {
             vertical: true,
@@ -172,15 +192,15 @@ export function FeedbackComponent({
             ),
           ],
         ),
-        h(Matches, {
+        h.if(!viewOnly)(Matches, {
           match,
           setMatchLinks,
           matchLinks,
           selectedNodes,
           tree,
           dispatch,
         }),
-        h(Divider),
+        h.if(!viewOnly)(Divider),
         h(EntityTypeSelector, {
           entityTypes: entityTypesMap,
           selected: selectedEntityType,
@@ -196,6 +216,8 @@ export function FeedbackComponent({
               type: "toggle-entity-type-selector",
               payload: isOpen,
             }),
+          viewOnly,
+          matchMode,
         }),
       ]),
     ]),
@@ -222,6 +244,8 @@ function EntityTypeSelector({
   tree,
   dispatch,
   selectedNodes = [],
+  viewOnly,
+  matchMode,
 }) {
   // Show all entity types when selected is null
   const _selected = selected != null ? selected : undefined;
@@ -242,6 +266,7 @@ function EntityTypeSelector({
       dispatch,
       selectedNodes,
       tree,
+      viewOnly: viewOnly || matchMode,
     }),
     h(OmniboxSelector, {
       isOpen,