Add overlay chart if selecting two saved results

[Tahler]

Resolves #180.

When two items are selected from the saved results, the chart displays 4 datasets, with one saved result's 2 datasets overlayed on top of the other result's 2 datasets.

This introduces a weird bug in which the chart sometimes temporarily stops displaying one of the two saved results. It is rather inconsistent to reproduce, but clicking to hide and show datasets often triggers the bug. I'm not sure if this bug lies in my code or in Chart.js

[Tahler]

This line unfortunately removes the cute transition animation when switching between single results. To keep them, there needs to be some better management of the 2 "overlayed" datasets in the global chart variable.

[ldemailly]

thanks for pointing this out ! let's try to keep the "using up/down arrow" to move from result to result working nicely:

add an if ? there is already logic to switch from multi to single - we just now have 3 modes and as long as we stay in either one of the mode the only change is the data ?

[Tahler]

This line resolves the bug that happens when moving from an overlay chart back to a single chart. Without this line, the data isn't reset and the single chart shows 4 datasets.

It only needs to be called when going from an overlay chart back to a single chart, however, so I added an if check. It has those nice animations when switching between single charts again.

[codecov]

Codecov Report

Merging #235 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master    #235   +/-   ##
======================================
  Coverage    90.2%   90.2%           
======================================
  Files          10      10           
  Lines        1906    1906           
======================================
  Hits         1720    1720           
  Misses        119     119           
  Partials       67      67

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 898414a...c085492. Read the comment docs.

[ldemailly]

I checked out the branch and it's super cool - thanks for this !

What can we do about the title though ?
(can we do the html table or that would take longer / for another PR)

for the 2 can you change the title to show what is A and what is B - then we can merge this
(and do the table additionally later)

[ldemailly]

found a small bug (that I had to fix when I first added multi chart) : resize doesn't work correctly, it looses the 2-chart or switches to multi-chart when you resize the window

[Tahler]

Both the original weird bug and the resizing bug are solved by separating an overlayChart and destroying the other charts as you switch 'modes'.

The title is pretty long but does label and show data for A and B. It can be adjusted / removed later like you said.

[ldemailly]

This is sweet ! thanks! 2 issues though, 1 for title space, let's apply:

ldemailly-macbookpro:fortio ldemailly$ git show 57ba5c9431e81c9c77888964b0e1d72efea91523
commit 57ba5c9431e81c9c77888964b0e1d72efea91523 (HEAD -> Tahler-overlay-180)
Author: Laurent Demailly <ldemailly@google.com>
Date:   Thu Apr 26 19:06:24 2018 -0700

    use fewer lines (skip 3rd one and collapse first 2)

diff --git a/ui/static/js/fortio_chart.js b/ui/static/js/fortio_chart.js
index da1668b..ef6ce99 100644
--- a/ui/static/js/fortio_chart.js
+++ b/ui/static/js/fortio_chart.js
@@ -202,11 +202,9 @@ function toggleVisibility () {
 function makeOverlayChartTitle (titleA, titleB) {
   // Each string in the array is a separate line
   return [
-    'A',
-    ...titleA,
+    'A: ' + titleA[0], titleA[1], // skip 3rd line for now
     '',
-    'B',
-    ...titleB
+    'B: ' + titleB[0], titleB[1]
   ]
 }
 

(I tried to put that in your fork but don't know how, for fortio feel free to make branches in this repo directly (unlike istio where it's not recommended))

2, I get errors when typing a max (like 400) and hitting return (it used to set the max to 400 right away)

    at setChartOptions (fortio_chart.js:370)
    at updateChart (fortio_chart.js:404)
    at <anonymous>:1:1

[ldemailly]

for title I tried

'A: ' + titleA[0], ...titleA.splice(1)

which works but takes too much space

[Tahler]

Done. I applied your title change and fixed the update form bug.

browse.html's updtForm calls updateChart() (with no arguments) so it did not know which chart to apply the form's options to. I applied a quick fix but it would be better in the future to have all these functions operate on a single, current chart rather than maintaining a chart for each mode.

[ldemailly]

awesomesauce! thanks!

ps: when we have time I want to explore the histogram with 4 slots where only 2 are used in single mode