Skip to content

Commit

Permalink
fix: criteria table edges can be deleted
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
keyserj committed Dec 3, 2024
1 parent e7bd593 commit e5919a0
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 2 deletions.
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
5 changes: 5 additions & 0 deletions src/web/common/components/ContextMenu/DeleteEdgeMenuItem.tsx
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>;
};
7 changes: 6 additions & 1 deletion src/web/topic/components/TopicWorkspace/WorkspaceToolbar.tsx
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

0 comments on commit e5919a0

Please sign in to comment.