Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clutter improvements #587

Merged
merged 6 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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 (
<>
Expand Down
38 changes: 20 additions & 18 deletions src/web/common/components/ContextMenu/ContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
};
12 changes: 12 additions & 0 deletions src/web/common/components/ContextMenu/ViewContextMenuItem.tsx
Original file line number Diff line number Diff line change
@@ -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>;
};
17 changes: 17 additions & 0 deletions src/web/common/components/ContextMenu/ViewDetailsMenuItem.tsx
Original file line number Diff line number Diff line change
@@ -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>
);
};
11 changes: 11 additions & 0 deletions src/web/common/components/ContextMenu/ViewTableMenuItem.tsx
Original file line number Diff line number Diff line change
@@ -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>
);
};
6 changes: 5 additions & 1 deletion src/web/topic/components/CriteriaTable/EdgeCell.tsx
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
16 changes: 16 additions & 0 deletions src/web/topic/components/Edge/ScoreEdge.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
Expand Down Expand Up @@ -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>
Expand Down
27 changes: 0 additions & 27 deletions src/web/topic/components/Node/NodeHandle.styles.tsx

This file was deleted.

21 changes: 16 additions & 5 deletions src/web/topic/components/Node/NodeHandle.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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);

Expand All @@ -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 =
Expand All @@ -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
Expand All @@ -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>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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);
Expand Down Expand Up @@ -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 />
Expand Down
14 changes: 14 additions & 0 deletions src/web/topic/store/edgeHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
});
};
2 changes: 1 addition & 1 deletion src/web/topic/utils/edge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)];
};

Expand Down
Loading