Skip to content

DYN-8843 Code only Annotation View #16252

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

Merged
merged 46 commits into from
Jun 25, 2025
Merged

Conversation

zeusongit
Copy link
Contributor

@zeusongit zeusongit commented May 30, 2025

Purpose

This is built on top of #16188 which needs to me merged first and then rebased.

The PR changes how annotations are implemented using code behind logic, similar to what is proposed for nodes in the other PR.
The changes seems to have performance improvements in cases when graphs have a huge number of groups.

Results:

Manekin: 94s -> 84s (~7%)

Screenshot 2025-05-30 at 5 06 52 PM

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

(FILL ME IN) Brief description of the fix / enhancement. Use N/A to indicate that the changes in this pull request do not apply to Release Notes. Mandatory section

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

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

Copilot

This comment was marked as outdated.

@zeusongit zeusongit marked this pull request as ready for review June 6, 2025 15:42
Copilot

This comment was marked as outdated.

@zeusongit zeusongit marked this pull request as draft June 6, 2025 16:14
@zeusongit zeusongit marked this pull request as ready for review June 6, 2025 18:14
@zeusongit zeusongit requested a review from Copilot June 6, 2025 18:15
Copilot

This comment was marked as outdated.

Eliminates the code and event handlers related to the collapsedGroupThumb in AnnotationView.xaml.cs, simplifying the resize logic and removing unused functionality for collapsed annotations.
@zeusongit zeusongit requested a review from Copilot June 25, 2025 17:03
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 revises the annotation view implementation by transitioning to a code-only approach, which is expected to yield performance improvements in graphs with a high number of groups. Key changes include:

  • Updating test cases to retrieve context menu items through the AnnotationGrid rather than direct properties.
  • Removing numerous unused XML namespace declarations and XAML resource definitions in the AnnotationView, streamlining the view's implementation.
  • Adjusting the event invocation for ungrouping annotations in tests to match the new UI hierarchy.

Reviewed Changes

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

File Description
test/DynamoCoreWpfTests/GroupStylesTests.cs Modified context menu reference for group style menu item.
test/DynamoCoreWpfTests/AnnotationViewTests.cs Changed how the ungroup annotation menu item is retrieved and invoked.
src/DynamoCoreWpf/Views/Core/AnnotationView.xaml Removed unused XML namespace declarations and XAML resources to simplify markup.
Comments suppressed due to low confidence (2)

test/DynamoCoreWpfTests/GroupStylesTests.cs:111

  • The context menu is now accessed via 'AnnotationGrid.ContextMenu'. Ensure that this change is consistently applied across the project if other components rely on the previous 'AnnotationContextMenu' property.
            var groupStylesMenuItem = annotationView.AnnotationGrid.ContextMenu

src/DynamoCoreWpf/Views/Core/AnnotationView.xaml:4

  • Multiple unused XML namespace declarations and resource definitions have been removed. Confirm that these changes do not affect any code-behind references or dependent components.
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"

Copy link
Contributor

@reddyashish reddyashish left a comment

Choose a reason for hiding this comment

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

Looks good to me

@zeusongit zeusongit merged commit 9a3b570 into DynamoDS:master Jun 25, 2025
26 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants