Skip to content

DynamoDS/DYN-10149: As a Dynamo user, I want the home and end keys to navigate to notes as well.#17145

Closed
eamiri wants to merge 1 commit into
masterfrom
10149-extend-home-end-keys-notes
Closed

DynamoDS/DYN-10149: As a Dynamo user, I want the home and end keys to navigate to notes as well.#17145
eamiri wants to merge 1 commit into
masterfrom
10149-extend-home-end-keys-notes

Conversation

@eamiri

@eamiri eamiri commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Closes https://autodesk.atlassian.net/browse/DYN-10149

h1. Extend Home/End to Notes — Implementation Plan

h2. Overview

Extend the Home and End keyboard shortcuts (introduced by DYN-9637) so that orphaned notes and notes-only groups participate in the leftmost/rightmost ranking alongside nodes. Notes that live inside a node-containing group continue to ride along with their parent group rather than influencing the ranking themselves. Add NUnit coverage for both the legacy node behavior (currently untested) and the new note behavior.

h2. Current State Analysis

h3. Key Discoveries:

  • Home/End command code is fully isolated to {{src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs:4523-4568}}. Only {{CurrentSpaceViewModel.Nodes}} is considered today.
  • {{WorkspaceViewModel}} already exposes {{Nodes}}, {{Notes}}, and {{Annotations}} collections; no plumbing changes are required to reach the data.
  • {{AnnotationModel.ContainsModel(ModelBase)}} ({{AnnotationModel.cs:1172}}) and the collection extension {{IEnumerable.ContainsModel(ModelBase)}} ({{AnnotationModelExtensions.cs:18}}) are the canonical "is this model in any group" checks.
  • {{DynamoSelection.Instance.Selection}} accepts any {{ISelectable}}; {{NodeModel}}, {{NoteModel}}, and {{AnnotationModel}} are all directly addable.
  • {{NoteViewModel}} has {{Left}}/{{Top}} but no {{Width}}/{{ActualWidth}} — read {{Model.Width}} for the right edge. {{AnnotationViewModel}} has full {{Left}}/{{Top}}/{{Width}}/{{Height}}.
  • No tests exist for the existing DYN-9637 commands. The sibling branch {{origin/10149-home-end-navigate-notes}} has both a prototype implementation ({{5fb1993}}) and 12 NUnit tests ({{fae550b}}) that can inform — but should not be copied verbatim into — this branch.

h2. Desired End State

After this plan is complete:

Pressing {{Home}} on a workspace with notes outside groups, notes-only groups, and/or nodes selects the leftmost element(s) (any combination of node, orphaned-note, notes-only-group whose Left edge is within {{tolerance}} of the global min) and fits the canvas to that selection.

Pressing {{End}} does the symmetric thing for rightmost.

A note inside a node-containing group never directly influences the ranking — its parent group still wins via the node it contains.

A workspace with only notes (no nodes) still responds to Home/End.

The {{IsHomeAndEndKeyEnabled}} gate still disables both shortcuts when anything is selected (no behavior change there).

NUnit tests cover the legacy node-only path and all new note paths.

Verification: Build {{src/Dynamo.All.sln}} in Release; run the new {{Home/End}} NUnit suite in {{test/DynamoCoreWpfTests/CoreUITests.cs}}; manually open a graph with mixed nodes/notes/groups in DynamoSandbox and verify Home/End behavior matches the table in Task 1.

h2. What We're NOT Doing

  • Not renaming {{GoToLeftMostNode}} / {{GoToRightMostNode}} to {{GoToLeftMostElement}} / etc. The public command names stay the same to avoid churn for any external consumer that may have wired up the commands.
  • Not changing the {{KeyBinding}}s in {{DynamoView.xaml}} — {{Home}} and {{End}} continue to route to the same commands.
  • Not changing the {{IsHomeAndEndKeyEnabled}} gate. The current behavior (disable when anything is selected) is already correct for notes and groups.
  • Not making notes inside node-containing groups influence the ranking. The ticket text and the camera-stability argument both point to the conservative reading.
  • Not fixing the latent PublicAPI bookkeeping inconsistency where the {{GoToLeftMostNodeCommand}}/{{GoToRightMostNodeCommand}} getters/setters were dropped from both {{PublicAPI.Unshipped.txt}} and {{PublicAPI.Shipped.txt}} by commit {{66e6b46}}. That is a separate concern — DYN-10149 itself does not add new public members.
  • Not adding new public API surface. The change is contained to private members of {{DynamoViewModel}}.
  • Not picking up the sibling-branch commits ({{d0e34d0}}, {{5fb1993}}, {{fae550b}}) wholesale. They are prior art and reference points; the implementation here is authored fresh on the active branch.

h2. Implementation Approach

Introduce a private {{HomeEndCandidate}} shape (struct or local record) inside {{DynamoViewModel}} that captures {{(Left, Right, Models)}} — {{Models}} is the set of {{ISelectable}} items to add to the selection when that candidate wins. Build the candidate set once, find the min-Left (or max-Right) within {{tolerance}}, then select and {{FitViewCommand.Execute(true)}}. The shape lets the Home and End handlers share the candidate-builder and only differ in the ranking projection.

Keep the change footprint small: one file ({{DynamoViewModel.cs}}) + one test file ({{CoreUITests.cs}}). Do not refactor unrelated code.


h3. TASK 1: Replace the Home/End candidate-building logic in DynamoViewModel [HIGH PRIORITY]

Status: NOT STARTED
Milestone: {{Home}} and {{End}} keys navigate to orphaned notes and notes-only groups in addition to nodes; notes inside node-containing groups continue to ride along with the group; notes-only workspaces still respond.

  • Add {{using Dynamo.Graph.Notes;}} to the file imports if not already present.
  • Introduce a private {{HomeEndCandidate}} shape (struct or readonly struct) inside the {{"Home And End key press events in Canvas"}} region with {{Left}}, {{Right}}, and {{Models}} ({{IReadOnlyList}}).
  • Add a private {{BuildHomeEndCandidates()}} method that yields one candidate per {{NodeViewModel}} (with its containing groups), one per orphaned {{NoteViewModel}}, and one per notes-only {{AnnotationViewModel}}.
  • Replace {{FitCanvasToSelectedNodes(List)}} with a candidate-list-based helper (e.g. {{FitCanvasToSelectedCandidates(IEnumerable)}}) that adds each winning candidate's models to {{DynamoSelection.Instance.Selection}}, calls {{FitViewCommand.Execute(true)}}, and then {{ClearSelection()}}.
  • Rewrite {{GoToLeftMostNode}} to: build candidates; early-return when empty; compute {{minLeft = candidates.Min(c => c.Left)}}; select candidates within {{tolerance}} of {{minLeft}}; fit.
  • Rewrite {{GoToRightMostNode}} symmetrically using {{Right}} and {{maxRight - tolerance}}.
  • Leave {{IsHomeAndEndKeyEnabled}}, {{CanGoToLeftMostNode}}, {{CanGoToRightMostNode}}, and the {{tolerance}} field unchanged.
  • Confirm the file still compiles in {{src/Dynamo.All.sln}} and {{src/DynamoCore.sln}} Release builds.

Validation:

{code:bash}dotnet build src/DynamoCore.sln -c Release
msbuild src/Dynamo.All.sln /p:Configuration=Release # Windows{code}

Behavior table to verify manually in DynamoSandbox:

||Workspace contents||Home target||End target||
|Only nodes (legacy)|Leftmost node + its group(s)|Rightmost node + its group(s)|
|Nodes + orphaned note left of all nodes|The orphaned note|Rightmost node|
|Nodes + note inside node-containing group, note further left than any node|Leftmost node (note rides via parent group)|Rightmost node|
|Notes-only group left of all nodes|The notes-only group|Rightmost node|
|Only orphaned notes|Leftmost note|Rightmost note|
|Only notes-only groups|Leftmost group|Rightmost group|
|Empty workspace|No-op|No-op|
|Any selection already present|Disabled (no-op)|Disabled (no-op)|

Requirements from spec:

  • Home/End must work with notes in groups alone (notes-only groups).
  • Home/End must work with orphaned notes.
  • Continue to work with nodes (DYN-9637 baseline preserved).

Files to Create: None.

Files to Modify:

  • {{src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs}} — lines 4523-4568 (the {{"Home And End key press events in Canvas"}} region), plus one new {{using Dynamo.Graph.Notes;}} near the top of the using block if missing.

Implementation Details:

  • Use {{nvm.X}} and {{nvm.X + nvm.ActualWidth}} for node candidates (matches existing math).
  • Use {{noteVM.Left}} and {{noteVM.Left + noteVM.Model.Width}} for orphaned-note candidates ({{NoteViewModel}} has no {{Width}} property — read {{Model.Width}}).
  • Use {{avm.Left}} and {{avm.Left + avm.Width}} for notes-only group candidates.
  • Orphaned check: {{!CurrentSpaceViewModel.Annotations.Any(a => a.AnnotationModel.ContainsModel(noteVM.Model))}}.
  • Notes-only check: {{!avm.Nodes.OfType().Any() && avm.Nodes.OfType().Any()}} — the {{Nodes}} enum is the mixed {{IEnumerable}} exposed by {{AnnotationViewModel}}.
  • Selection: add {{nvm.NodeModel}}, {{noteVM.Model}}, or {{avm.AnnotationModel}} (always the model, never the VM, matching {{WorkspaceViewModel.SelectAll}}).
  • Keep the existing {{tolerance = 2}} field at line 80.

h3. TASK 2: Add NUnit coverage for legacy and new Home/End behavior [HIGH PRIORITY]

Status: NOT STARTED
Milestone: Every behavior cell in the Task 1 table is exercised by an automated test; both the legacy DYN-9637 node behavior and the new note behavior are protected against regression.

  • Open {{test/DynamoCoreWpfTests/CoreUITests.cs}} and add a new {{#region Home and End}} (immediately after the {{Fit to View}} region around line 440, or after the {{Notes}} region — the exact placement is a code-style choice and not load-bearing).
  • Use the existing harness: {{CoreUserInterfaceTests : SystemTestBase}}, {{[Test, Apartment(ApartmentState.STA)] [Category("DynamoUI")]}}.
  • Hook {{DynamoSelection.Instance.Selection.CollectionChanged}} to capture which models were added during the command (the implementation clears the selection at the end, so a post-hoc read of {{Selection}} will be empty). Pattern: subscribe with a {{NotifyCollectionChangedEventHandler}}, accumulate {{e.NewItems}} into a list, unsubscribe after the command executes.
  • Build workspaces via {{Model.CurrentWorkspace.AddAndRegisterNode(new DoubleInput { X = ..., Y = ... }, true)}} for nodes and {{ViewModel.AddNoteCommand.Execute(null)}} for notes (existing patterns at {{CoreUITests.cs:437}} and {{:827}}). For positions, set {{note.X}} / {{note.Y}} directly on the {{NoteModel}} after {{AddNoteCommand}}.
  • For groups, follow the pattern in {{test/DynamoCoreWpfTests/AnnotationViewModelTests.cs}} (e.g. line 193 uses {{ViewModel.AddNoteCommand.Execute(null)}} then {{DynamoSelection.Instance.Selection.Add(...)}} then {{ViewModel.AddAnnotationCommand.Execute(null)}}).
  • One test per row of the behavior table in Task 1 (target ≥ 8 tests):
    *# {{WhenWorkspaceIsEmptyThenGoToLeftMostNodeIsNoOp}}
    *# {{WhenWorkspaceIsEmptyThenGoToRightMostNodeIsNoOp}}
    *# {{WhenOnlyNodesPresentThenGoToLeftMostNodeSelectsLeftmost}}
    *# {{WhenOnlyNodesPresentThenGoToRightMostNodeSelectsRightmost}}
    *# {{WhenOrphanedNoteIsLeftOfNodesThenGoToLeftMostNodeTargetsTheNote}}
    *# {{WhenNoteIsInsideNodeGroupAndFurtherLeftThenGoToLeftMostNodeTargetsTheNode}}
    *# {{WhenNotesOnlyGroupIsLeftmostThenGoToLeftMostNodeTargetsTheGroup}}
    *# {{WhenWorkspaceHasOnlyOrphanedNotesThenGoToLeftMostNodeStillWorks}}
    *# {{WhenSelectionExistsThenHomeAndEndAreDisabled}} (covers all three selectable types).
    *# Symmetric End variants where they add coverage that the Home variant did not.
  • Each test must call {{DynamoSelection.Instance.ClearSelection()}} in a {{[TearDown]}} or at the end to avoid polluting later tests.
  • Reset {{ViewModel.CurrentSpaceViewModel.Model.HasUnsavedChanges = false;}} at the end of each test (existing convention in the Fit to View region).

Validation:

{code:bash}dotnet test test/DynamoCoreWpfTests/DynamoCoreWpfTests.csproj --filter "NameHome|NameEnd&Category=DynamoUI"{code}

All new tests should pass; existing tests in {{CoreUITests.cs}} (Fit to View, Notes) should continue to pass.

Requirements from spec:

  • Cover all rows of the Task 1 behavior table.
  • Cover both Home and End where the variant adds meaningful coverage.
  • Cover the legacy DYN-9637 path (currently untested).

Files to Create: None.

Files to Modify:

  • {{test/DynamoCoreWpfTests/CoreUITests.cs}} — add new {{#region Home and End}} with the tests listed above.

Implementation Details:

  • The sibling-branch commit {{fae550b}} is a reference implementation for the test pattern (especially the {{NotifyCollectionChanged}} hook trick). Read it, but author the tests fresh on this branch so the diff is clean and reviewable.
  • Use {{Category("DynamoUI")}} to match the surrounding tests and let CI filter them with the existing Fit to View tests.
  • Avoid asserting on {{workspaceVM.Zoom}} / {{X}} / {{Y}} directly — {{FitViewInternal}} has its own coverage in the {{Fit to View}} region. Here, assert on what ends up in {{DynamoSelection.Instance.Selection}} during the command, which is the contract we own.

h2. Testing Strategy

h3. Unit Tests:

  • All cases in the Task 1 behavior table, implemented as NUnit tests in {{CoreUITests.cs}}.
  • Edge cases:
    ** Multiple candidates tied within {{tolerance = 2}} pixels — all should be added to the selection.
    ** A workspace containing a notes-only group whose Left edge is exactly equal to a node's Left edge (tolerance behavior is preserved).
    ** A note pinned to a node (note position is computed from the node) — should be carried by the node's containing group via the existing path; not directly considered in the ranking.

h3. Integration Tests:

  • No new integration tests required. The Fit to View region already exercises {{FitViewInternal}} / {{FitViewCommand}}; the new code merely changes what gets selected before {{FitViewCommand.Execute(true)}}.

h3. Manual Testing Steps:

Build {{src/Dynamo.All.sln}} Release and launch DynamoSandbox.

Open a fresh workspace. Press {{Home}}/{{End}} — no-op (empty workspace).

Add three number nodes spread horizontally. Press {{Home}} — leftmost node selected + fit. Press {{End}} — rightmost node selected + fit.

Add an orphaned note far to the left of all nodes. Press {{Home}} — note selected + fit (camera should center on the note, not the leftmost node).

Group two nodes together and add a note inside that group, positioned further left than the leftmost ungrouped node. Press {{Home}} — the leftmost ungrouped node (or the group containing the node, if it tied) is targeted; the note inside the group does not drag the camera leftward beyond its parent group.

Create a notes-only group at the far left. Press {{Home}} — that group is selected + fit.

Select any node or note or group manually, then press {{Home}}/{{End}} — both are no-ops (disabled by the gate).

Repeat the symmetric scenarios for {{End}} (rightmost).

h2. Performance Considerations

  • {{BuildHomeEndCandidates}} walks {{Nodes}} once, {{Notes}} once, and {{Annotations}} once per Home/End press. For each note and notes-only group it also scans {{Annotations}} to check membership — O(notes × annotations). At realistic workspace sizes (≤ 10k models is unusual; ≤ 1k is common) this is well under a millisecond and runs only on a single keypress, so optimization is not warranted.
  • If profiling later shows hot spots, building a {{HashSet}} of "all models in any group" once per call would reduce note-orphan checks from O(annotations) to O(1).

h2. Migration Notes

  • No data migration. {{.dyn}} file format is unchanged. Backward compatibility is preserved: pre-existing graphs and existing keybindings keep working.
  • No public API additions or removals, so consumers of the Dynamo NuGet packages are unaffected.
  • The sibling branch {{origin/10149-home-end-navigate-notes}} should be deleted (or rebased onto the active branch as a reference) once this work lands, to avoid drift between two parallel implementations. That cleanup is bookkeeping, not a code change here.

@eamiri eamiri added the kiln Created by kiln label Jun 8, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-10149

@eamiri eamiri added the implementation_failed Implementation workflow failed after retries label Jun 8, 2026
@eamiri eamiri closed this Jun 8, 2026
@eamiri eamiri deleted the 10149-extend-home-end-keys-notes branch June 8, 2026 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

implementation_failed Implementation workflow failed after retries kiln Created by kiln

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant