Skip to content

Conversation

@zeusongit
Copy link
Contributor

Purpose

ADP tracking for in-canvas search control was being recorded incorrectly as it was also triggered right after startup.
Therefore, it is now moved to the function that handles visibility change for both the controls to maintain uniformity and accurate hits.

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.

Reviewers

@DynamoDS/dynamo

@zeusongit zeusongit requested a review from QilongTang November 3, 2020 23:06
@zeusongit zeusongit changed the title Change analytics block position for both controls, Nodeautocomplete and Incanvas search Change analytics code block position for both controls, Nodeautocomplete and Incanvas search Nov 3, 2020
// When launching this control, always start with clear search term.
SearchTextBox.Clear();

Analytics.TrackEvent(
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this get called twice - once when the control becomes visible and once when it gets hidden?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if (!(bool)e.NewValue)
this takes care of those cases when control hides.

Copy link
Contributor

Choose a reason for hiding this comment

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

@aparajit-pratap See line 120

Copy link
Contributor

@QilongTang QilongTang left a comment

Choose a reason for hiding this comment

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

LGTM

@QilongTang
Copy link
Contributor

@zeusongit Let's merge this and cherry-pick to 2.9.0 branch

@zeusongit zeusongit merged commit b7d98b4 into DynamoDS:master Nov 5, 2020
QilongTang pushed a commit that referenced this pull request Nov 5, 2020
…Nodeautocomplete and Incanvas search (#11232) (#11235)

* Skip primitive type input ports from node-autocomplete (#11201)

* Add list of nodes to be skipped

* Add test and comments

* Add Analytics Coverage for Node AutoComplete (#11209)

* add analytics coverage for node autocomplete

Open auto complete window
Select suggestion

* Update NodeAutoCompleteSearchControl.xaml.cs

* add tracking to in-canvas search node selection

* add track points for in canvas search open/select

Co-authored-by: Ashish Aggarwal <ashish.zeus17@gmail.com>

* cherrypicking adp code change for node auto

Co-authored-by: Ashish Aggarwal <ashish.zeus17@gmail.com>
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.

4 participants