Skip to content
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-6397 Fix crash when parent node is deleted while node autocomplete menu is displayed #14715

Merged
merged 33 commits into from
Dec 13, 2023

Conversation

zeusongit
Copy link
Contributor

@zeusongit zeusongit commented Dec 7, 2023

Purpose

https://jira.autodesk.com/browse/DYN-6397

When a user deletes a node that has the node autocomplete menu open, it leaves the menu dissociated with the node and when a user clicks on any item on the menu, Dynamo crashed.

This PR fixes this case by closing the associated node autocomplete menu when the parent node is deleted.
On delete key press event any open node autocomplete menu will be closed.

DynamoSandbox_CuQpqGJ8S5

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • 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
  • All tests pass using the self-service CI.
  • 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

Release Notes

Fix crash when parent node is deleted while node autocomplete menu is displayed

Reviewers

@DynamoDS/dynamo

This reverts commit a588d14.
Copy link

github-actions bot commented Dec 7, 2023

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

Copy link

github-actions bot commented Dec 7, 2023

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net6.0

@QilongTang QilongTang added this to the 3.0 milestone Dec 8, 2023
Copy link

github-actions bot commented Dec 8, 2023

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

@avidit avidit changed the title Fix crash when parent node is deleted while node autocomplete menu is displayed DYN-6397 Fix crash when parent node is deleted while node autocomplete menu is displayed Dec 8, 2023
Copy link

github-actions bot commented Dec 8, 2023

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

Copy link

github-actions bot commented Dec 8, 2023

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net6.0

@zeusongit
Copy link
Contributor Author

Added a test, and reviewed the comments

Copy link

github-actions bot commented Dec 8, 2023

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

Copy link

github-actions bot commented Dec 8, 2023

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net6.0

@QilongTang
Copy link
Contributor

QilongTang commented Dec 11, 2023

Are there any regressions?

@zeusongit
Copy link
Contributor Author

zeusongit commented Dec 11, 2023

Are there any regressions?

I see 18 test failed, but seems un-related, and passes locally.

Copy link

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

Copy link

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net6.0

@QilongTang
Copy link
Contributor

Maybe try the self serve on master-15? Even after I update your branch with master, there are still a bunch failures.

Copy link

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net6.0

Copy link

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

@zeusongit
Copy link
Contributor Author

Maybe try the self serve on master-15? Even after I update your branch with master, there are still a bunch failures.

Tried master-15 , but each run gets me a different result, not sure what the problem is, I will wait for the ongoing issue with async test runs to resolve.

}

internal void CloseAutoCompletion()
{
OnRequestShowNodeAutoCompleteSearch(ShowHideFlags.Hide);
ViewModel.OnNodeAutoCompleteWindowClosed();
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

Copy link

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net6.0

@zeusongit
Copy link
Contributor Author

No regression, all checks passed, merging.

@zeusongit zeusongit merged commit bcdeccb into DynamoDS:master Dec 13, 2023
22 checks passed
zeusongit added a commit to zeusongit/Dynamo that referenced this pull request Dec 13, 2023
…e menu is displayed (DynamoDS#14715)

* Fix PostDiff job

* fix

* Revert different commit

This reverts commit a588d14.

* comments

* add unsub

* add test

* Update NodeAutoCompleteSearchControl.xaml.cs

---------

Co-authored-by: Aaron (Qilong) <[email protected]>
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