From 858a051d9eeb9684ad223ea20a1caaabe9467f4c Mon Sep 17 00:00:00 2001 From: Joel Keyser Date: Tue, 3 Dec 2024 10:01:07 -0600 Subject: [PATCH 1/6] touchup: make it easier to click edges --- src/web/topic/components/Edge/ScoreEdge.tsx | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/web/topic/components/Edge/ScoreEdge.tsx b/src/web/topic/components/Edge/ScoreEdge.tsx index b05153e2..7d3cdda3 100644 --- a/src/web/topic/components/Edge/ScoreEdge.tsx +++ b/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 = ( + 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/ */} {label} From 7d87660ee1aea3d9b2d67436e1c1f7a7abf2fcf4 Mon Sep 17 00:00:00 2001 From: Joel Keyser Date: Tue, 3 Dec 2024 15:22:19 -0600 Subject: [PATCH 2/6] touchup: hide node handles if they aren't useful i.e. if they aren't indicating hidden neighbors and user can't edit --- .../components/Node/NodeHandle.styles.tsx | 27 ------------------- src/web/topic/components/Node/NodeHandle.tsx | 21 +++++++++++---- 2 files changed, 16 insertions(+), 32 deletions(-) delete mode 100644 src/web/topic/components/Node/NodeHandle.styles.tsx diff --git a/src/web/topic/components/Node/NodeHandle.styles.tsx b/src/web/topic/components/Node/NodeHandle.styles.tsx deleted file mode 100644 index e2eb88ad..00000000 --- a/src/web/topic/components/Node/NodeHandle.styles.tsx +++ /dev/null @@ -1,27 +0,0 @@ -import { css } from "@emotion/react"; -import styled from "@emotion/styled"; -import { Handle } from "reactflow"; - -interface Props { - hasHiddenComponents: boolean; -} - -const options = { - shouldForwardProp: (prop: string) => !["hasHiddenComponents"].includes(prop), -}; - -export const StyledHandle = styled(Handle, options)` - // two selectors to override react-flow's styles - &.react-flow__handle { - width: 10px; - height: 10px; - - ${({ theme, hasHiddenComponents }) => { - if (hasHiddenComponents) { - return css` - background-color: ${theme.palette.info.main}; - `; - } - }} - } -`; diff --git a/src/web/topic/components/Node/NodeHandle.tsx b/src/web/topic/components/Node/NodeHandle.tsx index 7950d798..c03e7e05 100644 --- a/src/web/topic/components/Node/NodeHandle.tsx +++ b/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 ( 0 ? ( + hasHiddenNeighbors ? (
{sortedHiddenNeighbors.map((neighbor) => ( { } disableFocusListener > - 0} + className={ + "size-[10px]" + + (!showHandle ? " invisible" : "") + + (hasHiddenNeighbors ? " bg-info-main" : "") + } /> ); From 5c440a00a3c3c272c17f590434aa4c924c4e1b8d Mon Sep 17 00:00:00 2001 From: Joel Keyser Date: Tue, 3 Dec 2024 15:33:36 -0600 Subject: [PATCH 3/6] refactor: extract variables for context menu parts code is a little more concise this way --- .../components/ContextMenu/ContextMenu.tsx | 30 ++++++++----------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/web/common/components/ContextMenu/ContextMenu.tsx b/src/web/common/components/ContextMenu/ContextMenu.tsx index 1004c7db..b586aac8 100644 --- a/src/web/common/components/ContextMenu/ContextMenu.tsx +++ b/src/web/common/components/ContextMenu/ContextMenu.tsx @@ -20,29 +20,23 @@ 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 = [ // CRUD actions - !contextMenuContext.node && !contextMenuContext.edge && ( - - ), - contextMenuContext.node && ( - - ), - contextMenuContext.edge && ( - - ), - contextMenuContext.node && , - contextMenuContext.edge && , + contextPart === undefined && , + contextNode && , + contextEdge && , + contextNode && , + contextEdge && , // show/hide actions - contextMenuContext.node && ( - - ), - contextMenuContext.node && ( - - ), - contextMenuContext.node && , + contextNode && , + contextNode && , + contextNode && , // ensure there's never an empty context menu; that shows an empty bubble and feels awkward Cancel, From 97c74e6a6fe6f183470e986b7ae25678d185bb0e Mon Sep 17 00:00:00 2001 From: Joel Keyser Date: Tue, 3 Dec 2024 16:28:36 -0600 Subject: [PATCH 4/6] touchup: add "view" context menu items to prepare for allowing indicators to be hidden - this way, indicator functionality is still enabled via the context menu. --- .../components/ContextMenu/ContextMenu.tsx | 8 ++++++++ .../ContextMenu/ViewContextMenuItem.tsx | 12 ++++++++++++ .../ContextMenu/ViewDetailsMenuItem.tsx | 17 +++++++++++++++++ .../ContextMenu/ViewTableMenuItem.tsx | 11 +++++++++++ 4 files changed, 48 insertions(+) create mode 100644 src/web/common/components/ContextMenu/ViewContextMenuItem.tsx create mode 100644 src/web/common/components/ContextMenu/ViewDetailsMenuItem.tsx create mode 100644 src/web/common/components/ContextMenu/ViewTableMenuItem.tsx diff --git a/src/web/common/components/ContextMenu/ContextMenu.tsx b/src/web/common/components/ContextMenu/ContextMenu.tsx index b586aac8..c39cfc3e 100644 --- a/src/web/common/components/ContextMenu/ContextMenu.tsx +++ b/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"; @@ -26,6 +29,11 @@ export const ContextMenu = () => { // 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 && , + contextNode?.type === "problem" && , + contextPart && , + // CRUD actions contextPart === undefined && , contextNode && , diff --git a/src/web/common/components/ContextMenu/ViewContextMenuItem.tsx b/src/web/common/components/ContextMenu/ViewContextMenuItem.tsx new file mode 100644 index 00000000..95a8c9ee --- /dev/null +++ b/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 viewContext(graphPart.id)}>View context; +}; diff --git a/src/web/common/components/ContextMenu/ViewDetailsMenuItem.tsx b/src/web/common/components/ContextMenu/ViewDetailsMenuItem.tsx new file mode 100644 index 00000000..04741623 --- /dev/null +++ b/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 ( + { + setSelected(graphPart.id); + emitter.emit("viewTopicDetails"); + }} + > + View details + + ); +}; diff --git a/src/web/common/components/ContextMenu/ViewTableMenuItem.tsx b/src/web/common/components/ContextMenu/ViewTableMenuItem.tsx new file mode 100644 index 00000000..78f55562 --- /dev/null +++ b/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 ( + viewCriteriaTable(node.id)}> + View criteria table + + ); +}; From e7bd593c59b306c8b0513043ed3a4cb68a6bb779 Mon Sep 17 00:00:00 2001 From: Joel Keyser Date: Tue, 3 Dec 2024 16:50:18 -0600 Subject: [PATCH 5/6] touchup: allow edge cell to be right-clicked is prep so that indicator functionality is still enabled if indicators are being hidden. --- src/web/topic/components/CriteriaTable/EdgeCell.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/web/topic/components/CriteriaTable/EdgeCell.tsx b/src/web/topic/components/CriteriaTable/EdgeCell.tsx index e3487ffc..73be6ae5 100644 --- a/src/web/topic/components/CriteriaTable/EdgeCell.tsx +++ b/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 ( -
+
openContextMenu(event, { edge })} + > Date: Tue, 3 Dec 2024 16:52:29 -0600 Subject: [PATCH 6/6] fix: criteria table edges can be deleted this breaks things because solutions should always be compared against all criteria. maybe these shouldn't even be treated as edges? hard to say. but for now we can at least remove the ability to delete them. --- .../ContextMenu/ChangeEdgeTypeMenuItem.tsx | 5 +++++ .../components/ContextMenu/DeleteEdgeMenuItem.tsx | 5 +++++ .../components/TopicWorkspace/WorkspaceToolbar.tsx | 7 ++++++- src/web/topic/store/edgeHooks.ts | 14 ++++++++++++++ src/web/topic/utils/edge.ts | 2 +- 5 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/web/common/components/ContextMenu/ChangeEdgeTypeMenuItem.tsx b/src/web/common/components/ContextMenu/ChangeEdgeTypeMenuItem.tsx index e1365092..baa38fa3 100644 --- a/src/web/common/components/ContextMenu/ChangeEdgeTypeMenuItem.tsx +++ b/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 ( <> diff --git a/src/web/common/components/ContextMenu/DeleteEdgeMenuItem.tsx b/src/web/common/components/ContextMenu/DeleteEdgeMenuItem.tsx index a237b37b..c9d387d3 100644 --- a/src/web/common/components/ContextMenu/DeleteEdgeMenuItem.tsx +++ b/src/web/common/components/ContextMenu/DeleteEdgeMenuItem.tsx @@ -2,6 +2,7 @@ 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"; @@ -9,9 +10,13 @@ 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 deleteEdge(edge.id)}>Delete edge; }; diff --git a/src/web/topic/components/TopicWorkspace/WorkspaceToolbar.tsx b/src/web/topic/components/TopicWorkspace/WorkspaceToolbar.tsx index 406b4761..9889967b 100644 --- a/src/web/topic/components/TopicWorkspace/WorkspaceToolbar.tsx +++ b/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); @@ -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" > diff --git a/src/web/topic/store/edgeHooks.ts b/src/web/topic/store/edgeHooks.ts index 0511472b..2fd886c8 100644 --- a/src/web/topic/store/edgeHooks.ts +++ b/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; + } + }); +}; diff --git a/src/web/topic/utils/edge.ts b/src/web/topic/utils/edge.ts index 340e8f5f..9b080b27 100644 --- a/src/web/topic/utils/edge.ts +++ b/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)]; };