Skip to content

Conversation

@ramramps
Copy link
Collaborator

Purpose

Task : https://jira.autodesk.com/browse/QNTM-3406

This PR fixes the scroll issue in Incanvas Search.

Declarations

Check these if you believe they are true

  • The code base 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.

Reviewers

@alfarok

//if original sender was scroll bar(i.e Thumb) don't close the popup.
if(!(e.OriginalSource is Thumb))
{
HidePopupWhenWindowDeactivated();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

b0700d5
This is the original commit where this code was added. I think, this fixed a bug where the incanvas search was open when you open another view (customnode) or another workspace.
this check should fix that inappropriate close when mouse is released in the scroll.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @ramramps I just pulled the branch and tested locally. The in-canvas search looks good and no longer collapses when scrolling. I am not sure how critical it is but the search window remains open now when switching windows and appears in front of any other content you have open which was I think what you were describing above. Such as
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @alfarok I will check this

Copy link
Collaborator Author

@ramramps ramramps Mar 20, 2018

Choose a reason for hiding this comment

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

@alfarok I was able to reproduce this on the master branch. This fix doesn't necessarily change that.

@alfarok alfarok added the LGTM label Mar 21, 2018
@ramramps ramramps merged commit e11430c into DynamoDS:master Mar 21, 2018
@ramramps ramramps mentioned this pull request Mar 21, 2018
7 tasks
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.

2 participants