Skip to content

DYN-8535 : improvements to existing dna#16232

Merged
BogdanZavu merged 3 commits intoDynamoDS:masterfrom
BogdanZavu:DYN-8535
May 28, 2025
Merged

DYN-8535 : improvements to existing dna#16232
BogdanZavu merged 3 commits intoDynamoDS:masterfrom
BogdanZavu:DYN-8535

Conversation

@BogdanZavu
Copy link
Contributor

@BogdanZavu BogdanZavu commented May 19, 2025

Purpose

Improve existing node auto complete. We can also integrate these improvements into the new flyout as we see fit.

  • reuse the window (as much as possible)
  • add a time limit to ml service request
  • improve handling of event subscription/un-subscription so that we don't continue to pile up event subscriptions
  • handle multiple workspaces - you can have multiple tabs as custom nodes

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

N/A

Reviewers

@DynamoDS/synapse

FYI

@QilongTang

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.

Copy link
Member

@johnpierson johnpierson left a comment

Choose a reason for hiding this comment

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

lgtm

@johnpierson johnpierson requested a review from Copilot May 27, 2025 17:10
Copy link
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 improves the node auto complete functionality by reusing the auto complete window, adding a timeout to the ML service request, and refining event subscription management for multiple workspaces.

  • Removed redundant check in WorkspaceView before showing the auto complete control and replaced window instantiation with a reusable static call.
  • Updated ML service request configuration with a specified timeout and refactored event subscription handling in both the ViewModel and Control.
  • Introduced a static instance management approach to reuse the NodeAutoCompleteSearchControl window across workspaces.

Reviewed Changes

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

File Description
src/DynamoCoreWpf/Views/Core/WorkspaceView.xaml.cs Updates to use the reusable auto complete search control window.
src/DynamoCoreWpf/ViewModels/Search/NodeAutoCompleteSearchViewModel.cs Added timeout configuration for ML service request and adjusted event subscription management.
src/DynamoCoreWpf/Controls/NodeAutoCompleteSearchControl.xaml.cs Refactored auto complete control to utilize a single static instance with improved event subscription/unsubscription logic.
Files not reviewed (1)
  • src/DynamoCoreWpf/Controls/NodeAutoCompleteSearchControl.xaml: Language not supported

@BogdanZavu BogdanZavu merged commit c6b36c2 into DynamoDS:master May 28, 2025
26 of 27 checks passed
@BogdanZavu
Copy link
Contributor Author

@johnpierson johnpierson added the ai/ml 🧠 synapse team related PRs label Jun 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai/ml 🧠 synapse team related PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants