Skip to content

DYN-9202: Fix node overlays when opening file #16405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

zeusongit
Copy link
Contributor

Purpose

Fix node overlays when opening file, also added/updated the test

Declarations

Check these if you believe they are true

  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB
  • This PR introduces new feature code involve network connecting and is tested with no-network mode.

Release Notes

Fix node overlays when opening file, also added/updated the test

Reviewers

(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)

(FILL ME IN, optional) Any additional notes to reviewers or testers.

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

@zeusongit zeusongit requested review from a team and Copilot July 17, 2025 20:33
Copilot

This comment was marked as outdated.

Copy link

@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-9202

@zeusongit zeusongit added this to the 3.6 milestone Jul 17, 2025
@avidit avidit requested a review from Copilot July 18, 2025 16:09
Copy link
Contributor

@Copilot 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 fixes node overlays when opening files in Dynamo by correcting the visibility state of node overlays and improving the related test coverage. The fix addresses an issue where node overlays were not properly displayed when files were opened.

  • Removes the "Failure" category from a previously failing test to re-enable it
  • Adds validation for initial overlay state before zoom level adjustments
  • Corrects the assertion logic for zoom-in behavior to check visibility instead of opacity
  • Sets proper initial opacity for the zoom-in color overlay

Reviewed Changes

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

File Description
test/DynamoCoreWpf3Tests/NodeViewTests.cs Re-enables test and adds assertions for initial overlay state and corrects zoom-in visibility check
src/DynamoCoreWpf/Views/Core/NodeView.xaml.cs Sets initial opacity for nodeColorOverlayZoomIn to 0.5

@@ -404,7 +407,7 @@ public void ZoomChangeVisibilityTest()
// Zoom in, more than 0.4
wvm.Zoom = 0.6;
Assert.AreEqual(nodeViewWarningWarningFrozenHidden.zoomGlyphsGrid.Visibility, System.Windows.Visibility.Collapsed);
Assert.AreEqual(nodeViewWarningWarningFrozenHidden.nodeColorOverlayZoomOut.Opacity, 0);
Assert.AreEqual(nodeViewWarningWarningFrozenHidden.nodeColorOverlayZoomOut.Visibility, System.Windows.Visibility.Collapsed);
Copy link
Preview

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

This assertion is checking the visibility of nodeColorOverlayZoomOut, but based on the context and the previous line checking zoomGlyphsGrid visibility, this should likely be checking nodeColorOverlayZoomIn instead, as the test is verifying zoom-in behavior (zoom = 0.6).

Suggested change
Assert.AreEqual(nodeViewWarningWarningFrozenHidden.nodeColorOverlayZoomOut.Visibility, System.Windows.Visibility.Collapsed);
Assert.AreEqual(nodeViewWarningWarningFrozenHidden.nodeColorOverlayZoomIn.Visibility, System.Windows.Visibility.Collapsed);

Copilot uses AI. Check for mistakes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same question.

Copy link
Contributor

@aparajit-pratap aparajit-pratap Jul 18, 2025

Choose a reason for hiding this comment

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

You've changed nodeColorOverlayZoomIn, so the question also arises if there are tests to test this.

Copy link
Contributor Author

@zeusongit zeusongit Jul 18, 2025

Choose a reason for hiding this comment

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

nodeColorOverlayZoomOut is collapsed so it's opacity does not matter, that is what I updated in this test.

For nodeColorOverlayZoomIn that is always visible and set to 0.5 opacity since that is controlled by frozen state not zoom. We do not have any tests for the specific wpf control but there are tests for frozen state.

@zeusongit
Copy link
Contributor Author

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.

2 participants