From 166319fa4a31c0105d786c1e8b8d0228ff45bd14 Mon Sep 17 00:00:00 2001
From: Mathieu Duckerts-Antoine <dam@odoo.com>
Date: Wed, 21 Mar 2018 14:20:01 +0100
Subject: [PATCH] [IMP] web: GraphView: remove hack to render in DOM

The graph view uses the nv(d3) lib to render the graph. This lib
requires that the rendering is done directly into the DOM (so that
it can correctly compute positions). However, the views are always
rendered in fragments, and appended to the DOM once ready (to
prevent them from flickering).

Before this rev., the graph view circumvented this by performing
the rendering in a setTimeout(0), letting the framework append the
widget to the DOM first.

This rev. removes this hack and uses the on_attach_callback hook
instead, which is called when the widget is attached to the DOM.
This ensures that the rendering is always done in the DOM, and we
keep the rendering part synchronous.
---
 .../src/js/views/graph/graph_renderer.js      | 57 +++++++++++++++----
 1 file changed, 45 insertions(+), 12 deletions(-)

diff --git a/addons/web/static/src/js/views/graph/graph_renderer.js b/addons/web/static/src/js/views/graph/graph_renderer.js
index 8ce6ec14aba..bc11bb675df 100644
--- a/addons/web/static/src/js/views/graph/graph_renderer.js
+++ b/addons/web/static/src/js/views/graph/graph_renderer.js
@@ -42,6 +42,30 @@ return AbstractRenderer.extend({
         nv.utils.offWindowResize(this.to_remove);
         this._super();
     },
+    /**
+     * The graph view uses the nv(d3) lib to render the graph. This lib requires
+     * that the rendering is done directly into the DOM (so that it can correctly
+     * compute positions). However, the views are always rendered in fragments,
+     * and appended to the DOM once ready (to prevent them from flickering). We
+     * here use the on_attach_callback hook, called when the widget is attached
+     * to the DOM, to perform the rendering. This ensures that the rendering is
+     * always done in the DOM.
+     *
+     * @override
+     */
+    on_attach_callback: function () {
+        this._super.apply(this, arguments);
+        this.isInDOM = true;
+        this._renderGraph();
+    },
+    /**
+     * @override
+     */
+    on_detach_callback: function () {
+        this._super.apply(this, arguments);
+        this.isInDOM = false;
+    },
+
     //--------------------------------------------------------------------------
     // Private
     //--------------------------------------------------------------------------
@@ -76,21 +100,15 @@ return AbstractRenderer.extend({
                 description: _t("Try to add some records, or make sure that " +
                     "there is no active filter in the search bar."),
             }));
-        } else {
-            var self = this;
-            setTimeout(function () {
-                self.$el.empty();
-                var chart = self['_render' + _.str.capitalize(self.state.mode) + 'Chart']();
-                if (chart && chart.tooltip.chartContainer) {
-                    self.to_remove = chart.update;
-                    nv.utils.onWindowResize(chart.update);
-                    chart.tooltip.chartContainer(self.el);
-                }
-            }, 0);
+        } else if (this.isInDOM) {
+            // only render the graph if the widget is already in the DOM (this
+            // happens typically after an update), otherwise, it will be
+            // rendered when the widget will be attached to the DOM (see
+            // 'on_attach_callback')
+            this._renderGraph();
         }
         return this._super.apply(this, arguments);
     },
-
     /**
      * Helper function to set up data properly for the multiBarChart model in
      * nvd3.
@@ -327,6 +345,21 @@ return AbstractRenderer.extend({
         chart(svg);
         return chart;
     },
+    /**
+     * Renders the graph according to its type. This function must be called
+     * when the renderer is in the DOM (for nvd3 to render the graph correctly).
+     *
+     * @private
+     */
+    _renderGraph: function () {
+        this.$el.empty();
+        var chart = this['_render' + _.str.capitalize(this.state.mode) + 'Chart']();
+        if (chart && chart.tooltip.chartContainer) {
+            this.to_remove = chart.update;
+            nv.utils.onWindowResize(chart.update);
+            chart.tooltip.chartContainer(this.el);
+        }
+    },
 });
 
 });