fix: widen anchor-jump zoom margin and apply it to all anchor types - #74
Merged
Merged
Conversation
navigate_to_anchor() only zoomed out for margin when the target was a Dot anchor; other anchor types kept zoomToFitSelected()'s edge-to-edge fit, clipping the anchor and outermost upstream nodes. Apply the same post-fit margin to every anchor type, and widen it from ~7.5% to ~12.5% per side (MODULE_ZOOM_MARGIN_FACTOR 0.85 -> 0.75) so it lands in the requested ~200-300px range, matching the margin navigate_to_backdrop already gets from the backdrop's own bounds.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The behavior change is small and well-scoped with updated tests; only a minor comment-accuracy nit was identified.
Pull request overview
This PR adjusts DAG navigation framing so navigate_to_anchor() consistently applies a post-fit zoom-out margin for all anchor types (not just Dot anchors), reducing edge clipping when jumping to anchors.
Changes:
- Widen
MODULE_ZOOM_MARGIN_FACTORfrom0.85to0.75and update its explanatory comment. - Remove the Dot-only gating in
navigate_to_anchor()so the margin zoom-out runs for every anchor type. - Update/rename tests to assert non-Dot anchors also receive the margin zoom-out.
File summaries
| File | Description |
|---|---|
anchor.py |
Applies the post-fit margin zoom-out unconditionally after nuke.zoomToFitSelected(). |
constants.py |
Adjusts the margin factor constant and updates the documentation comment describing its effect. |
tests/test_anchor_navigation.py |
Updates tests to reflect the new “margin applies to all anchor types” behavior. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+40
to
+43
| # type (issue #61, widened by issue #73). Nuke's zoomToFitSelected() has no | ||
| # padding parameter, so the margin is applied as a post-fit zoom-out: 0.75 leaves | ||
| # ~12.5% of the viewport as margin per side, which lands in the requested | ||
| # ~200-300px range on a typical DAG panel. |
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
navigate_to_anchor()only applied a post-fit margin zoom-out when the target was a Dot anchor (issue Need margin around modules when zooming #61). Every other anchor type keptnuke.zoomToFitSelected()'s tight, edge-to-edge fit, which clips the anchor node and the furthest-out upstream nodes at the viewport edge.MODULE_ZOOM_MARGIN_FACTOR = 0.85(~7.5% per side) to0.75(~12.5% per side), landing in the requested ~200-300px range on a typical DAG panel, and now apply it to all anchor types — matching the marginnavigate_to_backdrop()already gets for free from a backdrop's own (larger) bounds.Root cause
anchor.pygated the margin zoom-out behindif anchor_node.Class() == 'Dot':, with the (now outdated) reasoning that other anchor types' tight fit was "already correct." That's the behavior reported in the issue: everydayAnchor_*(NoOp) jumps got zero margin and cropped the outermost nodes.Changes
constants.py:MODULE_ZOOM_MARGIN_FACTOR0.85 → 0.75, comment updated.anchor.py: removed theClass() == 'Dot'gate so the margin zoom-out runs for every anchor type.tests/test_anchor_navigation.py: updated/renamed the test that previously asserted non-Dot anchors get no margin to instead assert they do get the margin (same assertions the Dot-anchor test uses).Docs
Checked
docs/user-guide.mdand the DAG screenshots (jump-target,jump-to-source): those are hand-positioned backdrop framings captured directly by the screenshotter tool from the.nksource files, not live captures ofnavigate_to_anchor()'s runtime zoom, so they don't depict this margin and don't need regeneration. The prose doesn't quantify the margin either.Test plan
pytest tests/— 486 passedCloses #73