[FIX] web: graph rendering when in DOM

Backported fixes:

    1. [IMP] web: GraphView: remove hack to render in DOM

        - ref. 166319fa4a31c0105d786c1e8b8d0228ff45bd14
        - fully backported.

This commit backports the fix that removes the hack for rendering graphs
(delay in a setTimeout), so that the graph is rendered when it is in the DOM.

    2. [IMP] web: restore scroll position

        - ref. 4e204ed2233ccd36b2eb7d3e56bfa741b9cb3e14
        - partially backported: only keeps `on_attach_callback`
          and `on_detach_callback` in the abstract_controller
          and in the abstract_renderer.

This commit backports the use of the hooks methods `on_attach_callback` and
`on_detach_callback` in abstract controller and renderer, so that the
controller of any view warns the renderer when it has been attached in
(or detach from) the DOM.

Also, this commit adds `on_attach_callback` and `on_detach_callback` for the
dashboard app, so that the graph view is correctly rendered.

Fixes #24092

6 files changed, 139 insertions, 16 deletions

diff --git a/addons/board/static/src/js/dashboard.js b/addons/board/static/src/js/dashboard.js
index 558f7e53961..ee8f720a9df 100644
--- a/addons/board/static/src/js/dashboard.js
+++ b/addons/board/static/src/js/dashboard.js
@@ -139,6 +139,31 @@ FormRenderer.include({
         this._super.apply(this, arguments);
         this.noContentHelp = params.noContentHelp;
         this.actionsDescr = {};
+        this._boardSubcontrollers = []; // for board: controllers of subviews
+    },
+    /**
+     * Call `on_attach_callback` for each subview
+     *
+     * @override
+     */
+    on_attach_callback: function () {
+        _.each(this._boardSubcontrollers, function (controller) {
+            if ('on_attach_callback' in controller) {
+                controller.on_attach_callback();
+            }
+        });
+    },
+    /**
+     * Call `on_detach_callback` for each subview
+     *
+     * @override
+     */
+    on_detach_callback: function () {
+        _.each(this._boardSubcontrollers, function (controller) {
+            if ('on_detach_callback' in controller) {
+                controller.on_detach_callback();
+            }
+        });
     },
 
     //--------------------------------------------------------------------------
@@ -241,6 +266,7 @@ FormRenderer.include({
                         hasSelectors: false,
                     });
                     return view.getController(self).then(function (controller) {
+                        self._boardSubcontrollers.push(controller);
                         return controller.appendTo(params.$node);
                     });
                 });
diff --git a/addons/board/static/tests/dashboard_tests.js b/addons/board/static/tests/dashboard_tests.js
index ab0feaf2c96..4a6f25f5950 100644
--- a/addons/board/static/tests/dashboard_tests.js
+++ b/addons/board/static/tests/dashboard_tests.js
@@ -3,6 +3,7 @@ odoo.define('board.dashboard_tests', function (require) {
 
 var testUtils = require('web.test_utils');
 var FormView = require('web.FormView');
+var ListRenderer = require('web.ListRenderer');
 
 var createView = testUtils.createView;
 
@@ -454,5 +455,48 @@ QUnit.test('clicking on a kanban\'s button should trigger the action', function
     form.destroy();
 });
 
+QUnit.test('subviews are aware of attach in or detach from the DOM', function (assert) {
+    assert.expect(2);
+
+    // patch list renderer `on_attach_callback` for the test only
+    testUtils.patch(ListRenderer, {
+        on_attach_callback: function () {
+            assert.step('subview on_attach_callback');
+        }
+    });
+
+    var form = createView({
+        View: FormView,
+        model: 'board',
+        data: this.data,
+        arch: '<form string="My Dashboard">' +
+                '<board style="2-1">' +
+                    '<column>' +
+                        '<action context="{}" view_mode="list" string="ABC" name="51" domain="[]"></action>' +
+                    '</column>' +
+                '</board>' +
+            '</form>',
+        mockRPC: function (route) {
+            if (route === '/web/action/load') {
+                return $.when({
+                    res_model: 'partner',
+                    views: [[4, 'list']],
+                });
+            }
+            return this._super.apply(this, arguments);
+        },
+        archs: {
+            'partner,4,list':
+                '<list string="Partner"><field name="foo"/></list>',
+        },
+    });
+
+    assert.verifySteps(['subview on_attach_callback']);
+
+    // restore on_attach_callback of ListRenderer
+    testUtils.unpatch(ListRenderer);
+
+    form.destroy();
+});
 
 });
diff --git a/addons/web/static/src/js/views/abstract_controller.js b/addons/web/static/src/js/views/abstract_controller.js
index 3bdf8ab08ca..6a3f9a3bbaa 100644
--- a/addons/web/static/src/js/views/abstract_controller.js
+++ b/addons/web/static/src/js/views/abstract_controller.js
@@ -62,6 +62,18 @@ var AbstractController = Widget.extend({
         }
         return this._super.apply(this, arguments);
     },
+    /**
+     * Called each time the controller is attached into the DOM.
+     */
+    on_attach_callback: function () {
+        this.renderer.on_attach_callback();
+    },
+    /**
+     * Called each time the controller is detached from the DOM.
+     */
+    on_detach_callback: function () {
+        this.renderer.on_detach_callback();
+    },
 
     //--------------------------------------------------------------------------
     // Public
diff --git a/addons/web/static/src/js/views/abstract_renderer.js b/addons/web/static/src/js/views/abstract_renderer.js
index 935fcc6643e..2394fb31a64 100644
--- a/addons/web/static/src/js/views/abstract_renderer.js
+++ b/addons/web/static/src/js/views/abstract_renderer.js
@@ -36,6 +36,14 @@ return Widget.extend({
         this.$el.addClass(this.arch.attrs.class);
         return $.when(this._render(), this._super());
     },
+    /**
+     * Called each time the renderer is attached into the DOM.
+     */
+    on_attach_callback: function () {},
+    /**
+     * Called each time the renderer is detached from the DOM.
+     */
+    on_detach_callback: function () {},
 
     //--------------------------------------------------------------------------
     // Public
diff --git a/addons/web/static/src/js/views/form/form_controller.js b/addons/web/static/src/js/views/form/form_controller.js
index 2f037a7a005..b21f9d435f8 100644
--- a/addons/web/static/src/js/views/form/form_controller.js
+++ b/addons/web/static/src/js/views/form/form_controller.js
@@ -99,6 +99,7 @@ var FormController = BasicController.extend({
      * @todo convert to new style
      */
     on_attach_callback: function () {
+        this._super.apply(this, arguments);
         this.autofocus();
     },
     /**
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 74443843405..58ce4b2fc39 100644
--- a/addons/web/static/src/js/views/graph/graph_renderer.js
+++ b/addons/web/static/src/js/views/graph/graph_renderer.js
@@ -43,6 +43,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
     //--------------------------------------------------------------------------
@@ -51,10 +75,9 @@ return AbstractRenderer.extend({
      * Render the chart.
      *
      * Note that This method is synchronous, but the actual rendering is done
-     * asynchronously (in a setTimeout).  The reason for that is that nvd3/d3
-     * needs to be in the DOM to correctly render itself.  So, we trick Odoo by
-     * returning immediately, then wait a tiny interval before actually
-     * displaying the data.
+     * asynchronously.  The reason for that is that nvd3/d3 needs to be in the
+     * DOM to correctly render itself.  So, we trick Odoo by returning
+     * immediately, then we render the chart when the widget is in the DOM.
      *
      * @returns {Deferred} The _super deferred is actually resolved immediately
      */
@@ -76,21 +99,15 @@ return AbstractRenderer.extend({
                     "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 +344,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);
+        }
+    },
 });
 
 });