From 42bdf5aab051e6ed4efc0ffd02859f33afc3f84a Mon Sep 17 00:00:00 2001
From: Bartek Fabiszewski <github@fabiszewski.net>
Date: Tue, 14 Apr 2020 19:54:28 +0200
Subject: [PATCH] Fix chart update on (auto)refresh

---
 js/src/chartviewmodel.js       | 34 ++++++++++++++++++++++++++--------
 js/test/chartviewmodel.test.js | 27 +++++++++++++++++++++++----
 2 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/js/src/chartviewmodel.js b/js/src/chartviewmodel.js
index 91bcbf6..0070dee 100644
--- a/js/src/chartviewmodel.js
+++ b/js/src/chartviewmodel.js
@@ -21,6 +21,7 @@ import { lang as $ } from './initializer.js';
 import Chartist from 'chartist'
 import ViewModel from './viewmodel.js';
 import ctAxisTitle from 'chartist-plugin-axistitle';
+import uObserve from './observe.js';
 import uUtils from './utils.js';
 
 /**
@@ -116,8 +117,12 @@ export default class ChartViewModel extends ViewModel {
 
   setObservers() {
     this.state.onChanged('currentTrack', (track) => {
-      this.render();
-      this.model.buttonVisible = !!track && track.hasPlotData;
+      if (track) {
+        uObserve.observe(track, 'positions', () => {
+          this.onTrackUpdate(track, true);
+        });
+      }
+      this.onTrackUpdate(track);
     });
     this.onChanged('buttonVisible', (visible) => this.renderButton(visible));
     this.onChanged('chartVisible', (visible) => this.renderContainer(visible));
@@ -129,13 +134,22 @@ export default class ChartViewModel extends ViewModel {
     };
   }
 
+  /**
+   * @param {?uTrack} track
+   * @param {boolean=} update
+   */
+  onTrackUpdate(track, update = false) {
+    this.render(track, update);
+    this.model.buttonVisible = !!track && track.hasPlotData;
+  }
+
   /**
    * @param {boolean} isVisible
    */
   renderContainer(isVisible) {
     if (isVisible) {
       this.chartContainer.style.display = 'block';
-      this.render();
+      this.render(this.state.currentTrack);
     } else {
       this.chartContainer.style.display = 'none';
     }
@@ -152,15 +166,19 @@ export default class ChartViewModel extends ViewModel {
     }
   }
 
-  render() {
+  /**
+   * @param {?uTrack} track
+   * @param {boolean=} update
+   */
+  render(track, update = false) {
     let data = [];
-    if (this.state.currentTrack && this.state.currentTrack.hasPlotData && this.model.chartVisible) {
-      data = this.state.currentTrack.plotData;
+    if (track && track.hasPlotData && this.model.chartVisible) {
+      data = track.plotData;
     } else {
       this.model.chartVisible = false;
     }
-    if (this.data !== data) {
-      console.log(`Chart update (${data.length})`);
+    if (update || this.data !== data) {
+      console.log(`Chart${update ? ' forced' : ''} update (${data.length})`);
       this.data = data;
       const options = {
         lineSmooth: (data.length <= LARGE_DATA)
diff --git a/js/test/chartviewmodel.test.js b/js/test/chartviewmodel.test.js
index 143cebd..7206880 100644
--- a/js/test/chartviewmodel.test.js
+++ b/js/test/chartviewmodel.test.js
@@ -256,7 +256,7 @@ describe('ChartViewModel tests', () => {
     vm.data = null;
     vm.chartSetup();
     // when
-    vm.render();
+    vm.render(state.currentTrack);
     // then
     expect(mockChart.update).toHaveBeenCalledTimes(1);
     expect(mockChart.update.calls.mostRecent().args[0].series[0]).toBe(track.plotData);
@@ -275,12 +275,31 @@ describe('ChartViewModel tests', () => {
     vm.data = track.plotData;
     vm.chartSetup();
     // when
-    vm.render();
+    vm.render(state.currentTrack);
     // then
     expect(mockChart.update).not.toHaveBeenCalled();
     expect(vm.data).toBe(track.plotData);
   });
 
+  it('should render chart on same track and chart visible with update requested', () => {
+    // given
+    const positions = [
+      TrackFactory.getPosition({ id: 1, latitude: 2, longitude: 3, altitude: 4 }),
+      TrackFactory.getPosition({ id: 2, latitude: 3, longitude: 4, altitude: 5 })
+    ];
+    const track = TrackFactory.getTrack(positions);
+    state.currentTrack = track;
+    vm.model.chartVisible = true;
+    vm.data = track.plotData;
+    vm.chartSetup();
+    // when
+    vm.render(state.currentTrack, true);
+    // then
+    expect(mockChart.update).toHaveBeenCalledTimes(1);
+    expect(mockChart.update.calls.mostRecent().args[0].series[0]).toBe(track.plotData);
+    expect(vm.data).toBe(track.plotData);
+  });
+
   it('should render empty chart on empty track and hide chart container', () => {
     // given
     const track = TrackFactory.getTrack(0);
@@ -289,7 +308,7 @@ describe('ChartViewModel tests', () => {
     vm.data = chartData;
     vm.chartSetup();
     // when
-    vm.render();
+    vm.render(state.currentTrack);
     // then
     expect(mockChart.update).toHaveBeenCalledTimes(1);
     expect(mockChart.update.calls.mostRecent().args[0].series[0]).toEqual(track.plotData);
@@ -304,7 +323,7 @@ describe('ChartViewModel tests', () => {
     vm.data = chartData;
     vm.chartSetup();
     // when
-    vm.render();
+    vm.render(state.currentTrack);
     // then
     expect(mockChart.update).toHaveBeenCalledTimes(1);
     expect(mockChart.update.calls.mostRecent().args[0].series[0]).toEqual([]);