Skip to content

DYN-10261 Deselect note and node when unpinning#16953

Merged
jasonstratton merged 4 commits into
DynamoDS:masterfrom
jasonstratton:DYN-10261-Deselect-When-Unpinning-Note
Mar 11, 2026
Merged

DYN-10261 Deselect note and node when unpinning#16953
jasonstratton merged 4 commits into
DynamoDS:masterfrom
jasonstratton:DYN-10261-Deselect-When-Unpinning-Note

Conversation

@jasonstratton
Copy link
Copy Markdown
Contributor

@jasonstratton jasonstratton commented Mar 10, 2026

Purpose

DYN-10261 — When unpinning a note from a node, both remain selected, which is confusing since they are no longer a unit. This PR clears the selection on unpin and ensures undo/redo maintain correct selection state.

Acceptance Criteria:

  • Clear the selection when a note is unpinned
  • Confirm undo repins and selects both
  • Confirm redo unpins and clears the selection

Declarations

Changes

NoteViewModel.cs (src/DynamoCoreWpf/ViewModels/Core/)

  • UnpinFromNode: Wrapped upstream's multi-model undo recording inside a SuppressUndoRecording guard so undo/redo paths don't push duplicate entries. Added ClearSelection() after unpin.
  • note_PinUnpinToNode (Unpin branch): Set SuppressUndoRecording = true before calling UnpinFromNode from undo/redo.
  • note_PinUnpinToNode (Pin branch): Added Selection.AddUnique(Model) so both note and node are selected when undo re-pins.
  • note_PropertyChanged (PinnedNode case): Added selection handling for the DeserializeCore undo/redo path — select both when re-pinned, clear when unpinned.

WorkspaceModel.cs (src/DynamoCore/Graph/Workspaces/)

  • Constructor: Set note.Workspace = this for notes passed via the constructor. Previously only AddNote() set this reference, so notes loaded from .dyn files had Workspace == null, causing DeserializeCore to fail to restore PinnedNode during undo. This was a pre-existing bug exposed by the new undo tests.

NoteViewTests.cs (test/DynamoCoreWpf3Tests/)

  • Added UnpinClearsSelection_UndoRedoSelectsAndClearsCorrectly covering all three acceptance criteria in a single unpin → undo → redo flow.

Release Notes

Clear the selection when unpinning a note from a node. Undo re-pins and re-selects both; redo unpins and clears the selection. Also fixed a pre-existing bug where NoteModel.Workspace was not set for notes loaded from file, which prevented undo of pin/unpin operations from restoring the pinned state.

Developer Notes

DynaNotes DYN-10261

Reviewers

(Assign reviewer)

FYIs

N/A

@jasonstratton jasonstratton requested review from a team and Copilot March 10, 2026 23:09
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

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-10261

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses DYN-10261 by ensuring the workspace selection state is updated appropriately when a note is unpinned from a node, including consistent behavior across undo/redo.

Changes:

  • Clear selection when unpinning a note from a node.
  • On undo (re-pin), ensure both the node and note are selected; on redo (unpin), selection is cleared again.
  • Add an NUnit UI test covering unpin + undo/redo selection behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/DynamoCoreWpf3Tests/NoteViewTests.cs Adds a regression test validating selection state for unpin/undo/redo.
src/DynamoCoreWpf/ViewModels/Core/NoteViewModel.cs Updates unpin logic to clear selection and records undo for unpin; adjusts undo/redo selection behavior.

Comment thread src/DynamoCoreWpf/ViewModels/Core/NoteViewModel.cs
@jasonstratton jasonstratton force-pushed the DYN-10261-Deselect-When-Unpinning-Note branch from 41cf175 to b7a4b38 Compare March 11, 2026 01:40
The Pin branch of note_PinUnpinToNode called PinToNode without
setting SuppressUndoRecording. Since PinToNode records a modification
via UndoRecorder and RecordModificationForUndo clears the redo stack,
this could corrupt the undo/redo stack in longer undo chains by
pushing a duplicate entry and wiping redo history.

Set SuppressUndoRecording = true before calling PinToNode, matching
the pattern already used in the Unpin branch.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

@zeusongit
Copy link
Copy Markdown
Contributor

passed here: https://master-5.jenkins.autodesk.com/job/Dynamo/job/DynamoSelfServe/job/pullRequestValidation/19408/

@jasonstratton jasonstratton merged commit 1fa87a1 into DynamoDS:master Mar 11, 2026
28 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants