Skip to content

Commit

Permalink
fix: chained effects partition incorrectly
Browse files Browse the repository at this point in the history
e.g. if an effect is not directly tied to a problem or solution, it
wouldn't have been partitioned as expected.
  • Loading branch information
keyserj committed Feb 11, 2024
1 parent e58de58 commit e638c18
Showing 1 changed file with 20 additions and 6 deletions.
26 changes: 20 additions & 6 deletions src/web/topic/utils/layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import ELK, { ElkNode, LayoutOptions } from "elkjs";

import { NodeType, nodeTypes } from "../../../common/node";
import { nodeHeightPx, nodeWidthPx } from "../components/Node/EditableNode.styles";
import { type Edge, type Node } from "./graph";
import { children, parents } from "./node";
import { type Edge, type Node, ancestors, descendants } from "./graph";

export type Orientation = "DOWN" | "UP" | "RIGHT" | "LEFT";

Expand Down Expand Up @@ -64,10 +63,25 @@ const calculatePartition = (node: Node, nodes: Node[], edges: Edge[]) => {
const topicGraph = { nodes, edges };

if (["effect", "benefit", "detriment"].includes(node.type)) {
const hasProblemSource = parents(node, topicGraph).some((parent) => parent.type === "problem");
const hasSolutionTarget = children(node, topicGraph).some((child) => child.type === "solution");
if (hasProblemSource && hasSolutionTarget) return "null";
else if (hasProblemSource) return "0";
const nodeAncestors = ancestors(node, topicGraph);
const nodeDescendants = descendants(node, topicGraph);

const hasProblemAncestor = nodeAncestors.some((ancestor) => ancestor.type === "problem");
const hasCriterionAncestor = nodeAncestors.some((ancestor) => ancestor.type === "criterion");
const hasSolutionDescendant = nodeDescendants.some(
(descendant) => descendant.type === "solution"
);
const hasCriterionDescendant = nodeDescendants.some(
(descendant) => descendant.type === "criterion"
);

// this implementation seems solid but if it becomes unperformant, it might be good enough to
// just check if this node has a "created by" vs "creates" relation
const shouldBeAboveCriteria = hasProblemAncestor && !hasCriterionAncestor; // problem is _not_ through criterion
const shouldBeBelowCriteria = hasSolutionDescendant && !hasCriterionDescendant; // solution is _not_ through criterion

if (shouldBeAboveCriteria && shouldBeBelowCriteria) return "null";
else if (shouldBeAboveCriteria) return "0";
else return "2";
} else {
return partitionOrders[node.type];
Expand Down

0 comments on commit e638c18

Please sign in to comment.