Skip to content

Commit 192a1ea

Browse files
committed
fix(visualize): address xhigh code-review findings
Correctness/UX: - search & inspector link-jump now set autoFit=false so centerOn() actually centres the node instead of being overwritten by the per-frame fit - setMode clears in-flight interaction (drag/orbiting/pendingInspect/cursor), releases pins, and syncs the search box — no stale state across views - cap fitStep zoom so a 1-node KB can't balloon past the zoom range - guard adj[focus] in the graph hl-loop (matches the radial branch) Cleanup (post-churn): - extract shared buildProvenance() (was duplicated in mind-map + radial) - drop dead MODE_LABEL, unused 'kn', tombstone comment, unused ROOT fields, and the redundant first-pass n.show reset in layoutRadial
1 parent 596c633 commit 192a1ea

1 file changed

Lines changed: 18 additions & 20 deletions

File tree

openkb/templates/graph.html

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -290,8 +290,7 @@
290290
let didDrag = false;
291291
let alpha = 1; // simulated-annealing "temperature"; cools to 0 so motion stops
292292
// synthetic centre for the radial view ("OpenKB" → documents → concepts)
293-
const ROOT = { id:"__openkb__", label:"OpenKB", type:"__root__", x:0, y:0, z:0, tx:0, ty:0,
294-
sx:0, sy:0, rz:0, pp:1, r:15, hl:1, alpha:1, depth:0, show:true };
293+
const ROOT = { id:"__openkb__", label:"OpenKB", type:"__root__", x:0, y:0, sx:0, sy:0, rz:0, pp:1, r:15, show:true };
295294
let treeEdges = []; // radial hierarchy edges (root→doc, doc→concept)
296295
const DEFAULT_SPREAD = 1.7; // open the nebula out by default so it isn't a clump
297296
let spread = DEFAULT_SPREAD; // spacing knob: >1 spreads nodes apart, <1 packs them tighter
@@ -327,9 +326,8 @@
327326
let mmQuery = "", mmRootY = 0;
328327
const mmResolveSrc = s => { const id = String(s).replace(/\.md$/i, ""); return (byId[id] && byId[id].id.startsWith("summaries/")) ? id : null; };
329328
const ROOT_W = 104, NODE_W = 184, COL = 234, ROW = 31, PADX = 34, PADY = 30, NH = 22;
330-
function buildMindmap(){
331-
const q = mmQuery.toLowerCase();
332-
const keep = n => !q || n.label.toLowerCase().includes(q) || n.id.toLowerCase().includes(q);
329+
// shared OpenKB → documents → concepts grouping (each concept under its primary source summary)
330+
function buildProvenance(){
333331
const docs = nodes.filter(n => n.id.startsWith("summaries/") && typeVisible(n.type));
334332
const kidsOf = {}; docs.forEach(d => kidsOf[d.id] = []);
335333
const rootDirect = [];
@@ -338,6 +336,12 @@
338336
const p = (n.sources || []).map(mmResolveSrc).filter(Boolean)[0]; // primary document
339337
if(p && kidsOf[p]) kidsOf[p].push(n); else rootDirect.push(n);
340338
});
339+
return { docs, kidsOf, rootDirect };
340+
}
341+
function buildMindmap(){
342+
const q = mmQuery.toLowerCase();
343+
const keep = n => !q || n.label.toLowerCase().includes(q) || n.id.toLowerCase().includes(q);
344+
const { docs, kidsOf, rootDirect } = buildProvenance();
341345
const mk = n => ({ id:n.id, label:n.label, type:n.type, kids:[] });
342346
const root = { id:"__openkb__", label:"OpenKB", type:"__root__", kids:[] };
343347
docs.forEach(d => {
@@ -374,7 +378,7 @@
374378
let html = "";
375379
flat.forEach(it => {
376380
if(it.id === "__openkb__"){ html += `<div class="mm-node mm-root" style="left:${it.x}px;top:${it.y - 4}px">OpenKB</div>`; return; }
377-
const col = colorOf(it.type), kn = it.kids.length || (mmCollapsed.has(it.id) ? 1 : 0);
381+
const col = colorOf(it.type);
378382
const hasKids = it.id.startsWith("summaries/") && (kidsOf[it.id] || []).length;
379383
html += `<div class="mm-node" data-id="${esc(it.id)}" title="${esc(it.label)}" `
380384
+ `style="left:${it.x}px;top:${it.y}px;border-color:rgba(${col},.5);background:rgba(${col},.12)">`
@@ -395,14 +399,7 @@
395399

396400
/* ===================== radial view: same OpenKB→docs→concepts tree, fanned in a circle ===================== */
397401
function layoutRadial(){
398-
const docs = nodes.filter(n => n.id.startsWith("summaries/") && typeVisible(n.type));
399-
const kidsOf = {}; docs.forEach(d => kidsOf[d.id] = []);
400-
const rootDirect = [];
401-
nodes.forEach(n => {
402-
if(n.id.startsWith("summaries/") || !typeVisible(n.type)){ n.show = false; return; }
403-
const p = (n.sources || []).map(mmResolveSrc).filter(Boolean)[0];
404-
if(p && kidsOf[p]) kidsOf[p].push(n); else rootDirect.push(n);
405-
});
402+
const { docs, kidsOf, rootDirect } = buildProvenance();
406403
nodes.forEach(n => { n.show = false; });
407404
const branches = docs.map(d => ({ node: d, kids: kidsOf[d.id], w: Math.max(1.6, kidsOf[d.id].length) }))
408405
.concat(rootDirect.map(c => ({ node: c, kids: [], w: 1 })));
@@ -476,14 +473,16 @@
476473
}
477474

478475
const spacingCtl = document.getElementById("spacingCtl");
479-
const MODE_LABEL = { mindmap: "mind-map", radial: "radial", graph: "3D" };
480476
function setMode(m){
481477
mode = m;
482478
const canvasMode = (m === "graph" || m === "radial"); // canvas: 3D nebula + radial. mind-map is DOM
483479
canvas.style.display = canvasMode ? "block" : "none";
484480
mmEl.style.display = (m === "mindmap") ? "block" : "none";
485481
spacingCtl.style.display = (m === "mindmap") ? "none" : "flex";
486482
hover = null; closePanel(); panX = 0; panY = 0; autoFit = true;
483+
drag = null; orbiting = false; pendingInspect = null; didDrag = false; canvas.style.cursor = "grab"; // clear in-flight interaction
484+
nodes.forEach(n => n.pinned = false); // pins don't carry across views / reseed
485+
search.value = ""; mmQuery = ""; // keep the search box in sync with the (cleared) filter
487486
if(m === "mindmap"){ buildMindmap(); mmEl.scrollTop = Math.max(0, mmRootY - mmEl.clientHeight / 2); }
488487
else if(m === "radial"){ yaw = 0; pitch = 0; scale = DEF_SCALE; layoutRadial(); alpha = 1; }
489488
else { yaw = DEF_YAW; pitch = DEF_PITCH; scale = DEF_SCALE; seed(); alpha = 1; }
@@ -540,7 +539,7 @@
540539
if(maxX <= minX || maxY <= minY) return;
541540
const cx = (minX + maxX) / 2, cy = (minY + maxY) / 2;
542541
const factor = Math.min(W() * 0.9 / (maxX - minX), H() * 0.88 / (maxY - minY));
543-
const tScale = scale * factor;
542+
const tScale = Math.min(scale * factor, 4); // cap so a tiny graph (e.g. 1 node) doesn't balloon past the zoom range
544543
const Xc = (cx - (W()/2 + panX)) / scale, Yc = (cy - (H()/2 + panY)) / scale;
545544
const tPanX = -Xc * tScale, tPanY = -Yc * tScale;
546545
const e = 0.07; // ease 7%/frame → smooth glide into frame
@@ -702,7 +701,6 @@
702701
ctx.beginPath(); ctx.arc(x - r*0.28, y - r*0.28, r*0.3, 0, TAU);
703702
ctx.fillStyle = `rgba(255,255,255,${0.35*fog})`; ctx.fill();
704703
}
705-
// (flat discs — no glossy core highlight)
706704
// label — declutter: hubs at rest; hovered node + neighbors; selection; zoomed in; near depth only
707705
const showLabel = (mode === "radial" && n.depth <= 1) || (scale > 1.4) || (n.id === selected) || (hover ? hl > 0.55 : labelIds.has(n.id));
708706
if(showLabel && fog > 0.55){
@@ -742,7 +740,7 @@
742740
if(alpha < 0.05 && idleFrames > 150) yaw += 0.0011; // gentle auto-rotation once settled & idle
743741
const focus = hover || selected; // hover/selection spotlights a node + its neighbours
744742
for(const n of nodes){
745-
const near = !focus || n.id===focus || adj[focus].has(n.id);
743+
const near = !focus || n.id===focus || (adj[focus] && adj[focus].has(n.id));
746744
n.hl += ((near ? 1 : 0.22) - n.hl) * 0.12;
747745
}
748746
ctx.setTransform(DPR,0,0,DPR,0,0);
@@ -832,7 +830,7 @@
832830
panel.querySelectorAll("[data-n]").forEach(a => a.onclick = () => {
833831
const m = byId[a.dataset.n];
834832
if(!m) return;
835-
if(mode === "graph" || mode === "radial"){ hover = m.id; centerOn(m); } // canvas views: bring it into view
833+
if(mode === "graph" || mode === "radial"){ hover = m.id; autoFit = false; centerOn(m); } // canvas views: bring it into view (stop auto-fit so the pan sticks)
836834
openPanel(m); // all modes: show its details
837835
});
838836
}
@@ -865,7 +863,7 @@
865863
if(!q){ hover = null; return; }
866864
const m = nodes.find(n => n.label.toLowerCase().includes(q))
867865
|| nodes.find(n => n.id.toLowerCase().includes(q));
868-
if(m){ hover = m.id; centerOn(m); }
866+
if(m){ hover = m.id; autoFit = false; centerOn(m); } // stop auto-fit so the centring sticks
869867
});
870868
search.addEventListener("keydown", e => {
871869
if(e.key === "Enter"){

0 commit comments

Comments
 (0)