1133519 - Improve the interaction with the result-set endpoint

Status: RESOLVED FIXED

(Tree Management :: Perfherder, defect, P3)

Description

[Mauro Doglio [:mdoglio]]

A few things I noticed playing with the interface, filing this bug to not forget it:
- the requests to the resultset endpoint should be delayed by a few ms to avoid firing tens of requests to the api when moving the mouse pointer around.
- the endpoint uri has a trailing slash, at the moment every request  gets redirected increasing the time it takes to retrieve the data
- if a request to the result-set endpoint hasn't completed when the user moves the mouse away, it should be aborted.
- all the requests to the result-set endpoint end up with 404 if the series doesn't belong to mozilla-central

Comment 1

[Mauro Doglio [:mdoglio]]

:wlach let me know if you want separate bugs/more info

Comment 2

[William Lachance (:wlach)]

(In reply to Mauro Doglio [:mdoglio] from comment #0)
> A few things I noticed playing with the interface, filing this bug to not
> forget it:
> - the requests to the resultset endpoint should be delayed by a few ms to
> avoid firing tens of requests to the api when moving the mouse pointer
> around.
> - the endpoint uri has a trailing slash, at the moment every request  gets
> redirected increasing the time it takes to retrieve the data
> - if a request to the result-set endpoint hasn't completed when the user
> moves the mouse away, it should be aborted.
> - all the requests to the result-set endpoint end up with 404 if the series
> doesn't belong to mozilla-central

The last point has since been fixed, but all of the rest of these points are still valid. We should fix them at some point.

Comment 3

[Jonathan French (:jfrench)]

Akhilesh was working on this, and was on IRC this afternoon, so assigning as requested. Thank you Akhilesh! I'll let Mauro and Will sort out who will set themselves as mentor.

Comment 4

[Akhilesh Pillai]

https://github.com/mozilla/treeherder/blob/master/ui/js/graphs.js#L52

Unsuccessful Attempt:
Tried modifying the showToolTip function as follows 

function showTooltip(dataPoint) {
     $scope.showToolPromise = $timeout(function() {
        console.log($scope.showToolPromise)
        $timeout.cancel($scope.showToolPromise)
         ...............
         .... 
        },2000);
//2000 was just to exaggerrate the delay for testing purpose. 

Now Reading up on debounce : http://benalman.com/projects/jquery-throttle-debounce-plugin/

Comment 5

[William Lachance (:wlach)]

We have used setTimeout elsewhere for a similar problem with decent results:

https://github.com/mozilla/treeherder/blob/master/ui/js/directives/clonejobs.js#L194

Debounce sounds interesting, though it seems to be intended for a slightly different use case than the one we have here.

Comment 6

[Akhilesh Pillai]

Attachment: Fix for bug 1133519

Attaching the associated pull request.

Comment 7

[Akhilesh Pillai]

Attachment: graphs.js

Thanks for reviewing previous code. 

Updated the graph.js:

1) changed the showToolTipTimeout to 250
2) added space between , and 250.
3) indented the function block in showTooltip function. 
4) To not pop up the tooltip after the timeout if the cursor has moved away from the datapoint in the interim, I updated the hideTooltip function.

Comment 8

[William Lachance (:wlach)]

Comment on attachment 8610699 [details]
graphs.js

A whole file is difficult to review. Can you ask for review on your pull request? If this is the right PR:

https://github.com/mozilla/treeherder/pull/561

Then just attach it as you did the previous one and re-request r?

Thanks :)

Comment 9

[Akhilesh Pillai]

Attachment: Fix for bug 1133519

Comment 10

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/cda43afedd6c866e7eeedd95c881c57e9a3745f0
Bug 1133519: set timeout to 400 ms for showToolTip and cancel timeout if the user moves to another point before the timeout elapses. Further updated getRevisions in resultset.js to include the trailing /

https://github.com/mozilla/treeherder/commit/a4d75bce94390e674a6af2ecdd86aad5e37fa32e
Bug 1133519 - Improve interaction with result set endpoint

* Updated the timeout for showToolTipTimeout to 250
* Indented the function and added cancel timeout for showToolTipTimeout to
  hideTooltip function.

Comment 11

[William Lachance (:wlach)]

Comment on attachment 8610706 [details] [review]
Fix for bug 1133519

Updated patch was good, pushed.

(unfortunately didn't notice there were two commits that needed to be rebased, we'll fix that next time)