fix(graph): scope CSS to #graph and add fit-to-view for HTML viewer#4117
Open
A3Ackerman wants to merge 1 commit into
Open
fix(graph): scope CSS to #graph and add fit-to-view for HTML viewer#4117A3Ackerman wants to merge 1 commit into
A3Ackerman wants to merge 1 commit into
Conversation
The HTML graph viewer was unusable for non-trivial graphs. Two distinct
bugs in cmd/bd/graph_export.go template:
1. CSS selector "svg { width: 100vw; height: 100vh }" matched every
<svg> on the page, including the 30x10 legend swatches for "blocks"
and "parent-child" edge types. Each swatch ballooned to viewport
size and stretched the #legend container over the graph. Scope to
#graph instead.
2. Initial zoom was a hardcoded "translate(width/4, height/4).scale(0.8)"
that doesn't read node positions — multi-layer graphs ran off-screen
to the right. Replace with a real fitToView() that reads the post-
simulation bounding box and transforms to fit with padding. Also adds
a "Fit View" button and repoints resetZoom() to call fitToView()
(matches user expectation of the existing Reset View button).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Collaborator
|
Thanks for the report. I’m doing a repro sweep and couldn’t find a concrete reproduction recipe in the current description or thread. Could you add the smallest command sequence or setup that triggers this, including the codex-gpt-5.5-medium on behalf of matt wilkie |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bd graph --htmlis unusable on non-trivial graphs: the legend covers the viewport and graph nodes run off-screen even after Reset View. Two distinct bugs in the embedded HTML template:svg { width: 100vw; height: 100vh }matches every<svg>on the page — including the 30×10 legend swatches for "blocks" and "parent-child" edge types. Each swatch balloons to viewport size, stretching the#legendcontainer to ~1500×1600 and covering the graph. Fix: scope to#graph.d3.zoomIdentity.translate(width/4, height/4).scale(0.8)is a fixed transform that doesn't read node positions; multi-layer graphs (x = 150 + layer*220) push deep-layer nodes off-screen to the right. Fix: realfitToView()that reads the post-simulation bounding box and transforms to fit with padding. Adds a "Fit View" button and repointsresetZoom()to it so the existing Reset View button behaves as users expect.Template-only change. No public API surface, no behavior change for the DOT renderer.
Why
Continuation of recent graph-viewer fixes:
--all --html[]notnullfor empty linksThis addresses the remaining unusability of the rendered viewer itself. Followed CONTRIBUTING.md "one issue per PR" — did not bundle the adjacent observation that
--all --htmlstill emits N sub-graphs per connected component (that is a layout choice, not a CSS bug; happy to file separately).Verification
make build— cleangolangci-lint run --build-tags gms_pure_go ./cmd/bd/...—0 issues.make test— hits an environmental hang ininternal/utils/TestMain(testutil.EnsureDoltContainerForTestMain) on my local box, unrelated to a Go HTML template string change. I expect CI (with Docker) to run this cleanly; happy to investigate further if it doesn't.Notes for reviewers
setTimeout(fitToView, 1500)rather thansimulation.on("end", ...)— force simulations with strong charges don't naturally reachendwithout explicitalpha/alphaTargetdiscipline. The 1500ms heuristic works in practice; happy to revisit.fitToView()capsscaleat the existingzoom.scaleExtent([0.1, 4])upper bound (Math.min(width/bboxW, height/bboxH, 4)).🤖 Generated with Claude Code