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

Prevent node autocomplete from autohiding #11487

Merged
merged 3 commits into from
Feb 11, 2021

Conversation

mmisol
Copy link
Collaborator

@mmisol mmisol commented Feb 10, 2021

Purpose

This happened when the amount of suggestions for an input surpassed a
certain number of suggestions. The cause was the following condition.

It turns out that popups, like autocomplete, close when the left mouse
button up event happens. The problem is that autocomplete itself is
launched by pressing the left mouse button. This would normally mean
that the popup would hide as soon as the button was released, but this
was actually prevented through a time condition, which ignored the
event if it was 'soon enough'.

Unfortunately, in cases where there were a lot of suggestions, the left
mouse button up event can come after the expected timespan. This
resulted in the workaround failing to prevent the popup from hiding
immediately.

The newly implemented solution replaces the previous time-based
workaround with a more robust solution, which is to ignore the next
event after the call to show the popup is made.

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 happened when the amount of suggestions for an input surpassed a
certain number of suggestions. The cause was the following condition.

It turns out that popups, like autocomplete, close when the left mouse
button up event happens. The problem is that autocomplete itself is
launched by pressing the left mouse button. This would normally mean
that the popup would hide as soon as the button was released, but this
was actually prevented through a time condition, which ignored the
event if it was 'soon enough'.

Unfortunately, in cases where there were a lot of suggestions, the left
mouse button up event can come after the expected timespan. This
resulted in the workaround failing to prevent the popup from hiding
immediately.

The newly implemented solution replaces the previous time-based
workaround with a more robust solution, which is to ignore the next
event after the call to show the popup is made.
@mmisol mmisol self-assigned this Feb 10, 2021
@@ -390,7 +394,6 @@ void OnWorkspaceViewDataContextChanged(object sender, DependencyPropertyChangedE
ViewModel.RequestAddViewToOuterCanvas += vm_RequestAddViewToOuterCanvas;
ViewModel.WorkspacePropertyEditRequested += VmOnWorkspacePropertyEditRequested;
ViewModel.RequestSelectionBoxUpdate += VmOnRequestSelectionBoxUpdate;
ViewModel.RequestNodeAutoCompleteSearch += ShowHideNodeAutoCompleteControl;
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember this was added so that Node AutoComplete works for custom node workspace. Any reason of removing this? If intended, would you test using node autocomplete in a new custom node?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see. This was causing the Show function to be called two times in a row. I can add it back to avoid affecting custom nodes, but it does seem suspicious.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Instead of removing this handler assignment I removed the other one. Tested in both standard and custom node workspace and it's working. This also prevents the existing behaviour of the handler being called twice on standard workspaces.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you @mmisol . Looks reasonable,

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.

@mmisol mmisol merged commit ffe69d5 into DynamoDS:master Feb 11, 2021
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