| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 658
DYN-8576 - change autocomplete popup to a Window to improve behavior #16208
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
DYN-8576 - change autocomplete popup to a Window to improve behavior #16208
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created a new constructor here, old one was already public and is currently marked as obsolete.
johnpierson
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this 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-8576
johnpierson
requested review from
a team,
johnpierson and
BogdanZavu
and removed request for
a team
| @@ -57,6 +57,23 @@ public NodeAutoCompleteSearchControl() | |||
| HomeWorkspaceModel.WorkspaceClosed += this.CloseAutoCompletion; | |||
| } | |||
|
|
|||
| public NodeAutoCompleteSearchControl(Window window, NodeAutoCompleteSearchViewModel viewModel) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be internal ? I don't think we want it in the API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
Let's remove OnNodeAutoCompleteSearchControlVisibilityChanged Previously we were subscribing on visibility changed but did not unsubscribe on most closing contexts. |
| @@ -72,12 +89,13 @@ private void NodeAutoCompleteSearchControl_Unloaded(object sender, System.Compon | |||
|
|
|||
| private void CurrentApplicationDeactivated(object sender, EventArgs e) | |||
| { | |||
| OnRequestShowNodeAutoCompleteSearch(ShowHideFlags.Hide); | |||
| OnRequestHideNodeAutoCompleteSearch(); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't want to hide the window on app deactivation anymore ; previously it was a popup and it was hiding by itself in many contexts.
Actually @chubakueno we can do this as part of another PR. So up to you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i agree with @BogdanZavu - lets try to get this merged and worry about the other comments in another pr. getting this existing dna node type match work implemented is valuable to move on to the next thing.
|
Improved the layout algorithm a bit so the window doesnt appear on different places depending on whether the AI sparkle is present to prepare to rebase onto 3.5.1 hotfix, feel free to merge when reviewed. |
Purpose
Change autocomplete Popup to a Window to improve control behavior. The main consequence of this is the control no longer being Topmost and not remaining on top of other programs or when Dynamo is minimized.
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Revised existing Node AutoComplete Popup to window
Reviewers
@johnpierson
@BogdanZavu
FYIs
Change autocomplete Popup to a Window to improve control behavior.