recording: node-editor tutorial sweep (final) — styling + navigation (+ center_node_on_screen footgun fix)#29
Merged
Conversation
Replaces the legacy zero-verification static tour. Six beats sweep left to right across all five styling layers the tutorial teaches - canvas theme, per-node tint (with the default node for contrast), pin pivot, scoped with_style_var + with_style_color, and the background draw-list accent stripe - then close by pulsing the link to show the styled graph is still live. Captions are terse ASCII decoupled from natural voice; every narrated node, pin and link is asserted rendered (record_check_rendered), so a broken example aborts the recording at teardown. Styling is declarative, so there is nothing to click - the cursor points at each layer as the voice names it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e_on_screen footgun Rewrites the legacy zero-verification driver to the voiced, self-verifying bar - and the verification caught a real documentation bug. The tutorial taught three 'view ops' that 'move no node', but center_node_on_screen is the odd one out: upstream CenterNodeOnScreen translates the node's bounds to the view center (MakeDirty Position|User) - it MOVES the node, not the view. The name lies. Reframed per that finding (kept all three ops, teach the gotcha): - record_navigation.das: Fit All / Frame Selection asserted view-only (screen bbox changed, canvas bbox held); Center #1 asserted to MOVE node 1's canvas bbox. Fit All also asserted to REVEAL the off-view node 3. - test_navigation.das: docstring corrected + a CI assertion added that Center #1 moves node 1's canvas bbox (headless: PASS) - the footgun is now CI-enforced. - imgui_node_editor_boost_v2.das: center_node_on_screen docstring corrected (it moves the node; NavigateToContent / navigate_to_selection are the view-only ops). - navigation.das + navigation.rst: reframed to two view ops + one node move. record_navigation.das uses record_check_unchanged (dasImgui #161), so CI is red until that merges, same as the groups PR. The headless test_navigation.das is #161-independent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR completes the final two node-editor tutorial recordings in the “voiced + self-verifying” sweep by updating the styling and navigation tutorial drivers, strengthening the headless navigation regression, and correcting documentation/API guidance around the center_node_on_screen behavior (it moves the node, not the view).
Changes:
- Reworks
record_styling.dasinto a narrated, self-verifying sweep that asserts rendered nodes/pins/link and ends with aflow()pulse. - Reframes navigation materials (recording, tutorial source, and docs) to distinguish two view ops from the Center #1 node-move footgun, and adds a CI assertion for the footgun in
test_navigation.das. - Updates
center_node_on_screendocumentation in the v2 boost wrapper to reflect actual upstream semantics.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integration/test_navigation.das | Updates headless regression docstring and adds an assertion that “Center #1” changes node 1’s canvas bbox. |
| tests/integration/record_styling.das | Converts styling recording into a layered, self-verifying narration with rendered assertions and a link pulse. |
| tests/integration/record_navigation.das | Reframes recording as “two view ops + one node move” and adds checks for screen-vs-canvas behavior. |
| examples/tutorial/navigation.das | Updates tutorial framing/comments and toolbar note to reflect the Center #1 footgun. |
| doc/source/tutorials/navigation.rst | Updates narrative and “Driving it from a test” section to reflect view-only ops vs node-moving Center. |
| daslib/imgui_node_editor_boost_v2.das | Corrects the center_node_on_screen docstring to state it moves the node. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+78
to
+80
| record_check_changed(app, T_NODE1, "bbox", n1_bbox0) | ||
| record_check_unchanged(app, T_NODE1, "canvas_bbox", n1_canvas0) | ||
| say(app, "Frame Selection: frame the selected", FRAME, |
| wait_for_mouse_idle(app) | ||
| record_check_rendered(app, T_NODE3, true) | ||
| record_check_changed(app, T_NODE3, "bbox", n3_bbox0) | ||
| record_check_unchanged(app, T_NODE3, "canvas_bbox", n3_canvas0) |
| // ---- Beat 5: the closer - view ops vs the node move ---- | ||
| move_to(app, widget_center(snapshot(app), T_NODE3), 600) | ||
| wait_for_mouse_idle(app) | ||
| record_check_unchanged(app, T_NODE3, "canvas_bbox", n3_canvas0) |
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.
The final two tutorials of the node-editor recording sweep (after #26 first_graph/connect/create and #27 delete/context/clipboard, both merged). Same bar: voiced (terse ASCII caption decoupled from natural voice), self-verifying (every interaction asserted; a no-op aborts the recording at teardown).
Landing eyeball-approved.
Included
with_style_var+with_style_color, background draw-list accent stripe), closing with aflow()pulse on the link. Styling is declarative, so the self-verification asserts every narrated node, pin and link actually rendered (record_check_rendered) — a broken example aborts the recording. Replaces the legacy zero-verification static tour.center_node_on_screenis the odd one out — upstreamCenterNodeOnScreentranslates the node's bounds to the view center (MakeDirty(Position|User)); it moves the node, not the view. The name lies.The footgun fix (navigation)
Reframed to keep all three ops but teach the gotcha:
record_navigation.das— Fit All / Frame Selection asserted view-only (screenbboxchanged,canvas_bboxheld viarecord_check_changed+record_check_unchanged); Center v2 boost API, live commands, topology-edit, integration test + CI #1 asserted to move node 1'scanvas_bbox. Fit All also asserted to reveal the off-view node 3.test_navigation.das— docstring corrected + a CI assertion added that Center v2 boost API, live commands, topology-edit, integration test + CI #1 moves node 1's canvas bbox (ran headless: PASS). The footgun is now CI-enforced.imgui_node_editor_boost_v2.das—center_node_on_screendocstring corrected (it moves the node;NavigateToContent/navigate_to_selectionare the view-only ops).navigation.das+navigation.rst— reframed to two view ops + one node move.Per-tutorial cleanups
set_user_controltoggles (with_node_editor_recording_appowns it) and strayprint()lines.Notes
record_navigation.dasusesrecord_check_unchanged, which rides dasImgui borisbat/dasImgui#161 — so CI is red until #161 merges, same as the groups PR (#28).stylingis #161-independent, and so is the headlesstest_navigation.das. The.apng/ wavs / manifest / sidecar are gitignored intermediates; only the.mp4is tracked.🤖 Generated with Claude Code