From 36da1799b5158d2eefe6aefb4847650f00147607 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Tue, 15 Sep 2020 12:35:48 +0000 Subject: [PATCH] [FIX] web: list: call mounted on subcomponents at reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue was spotted in list views containing the "badge" widget with decorations (e.g. in the Purchase Order list view). At first, the decorations were correctly applied, but they weren't anymore after a reload (e.g. after toggling a filter). They weren't because the list renderer didn't called 'on_attach_callback' on its subwidgets at reload (thus, 'mounted' wasn't called on its subcomponents, and the decorations are applied in the mounted hook). This commit moves the logic from the FormRenderer to the BasicRenderer, so that it applies on List, Kanban, and Form views. This commit also removes the transition scss rule on the badge field widget as it caused a flickering at reload. Task 2336440 closes odoo/odoo#57766 X-original-commit: 713bce22e8f412f2c26e663b9e30fdd64933ff0c Signed-off-by: Michaël Mattiello Signed-off-by: Aaron Bohy (aab) --- .../src/js/views/basic/basic_renderer.js | 28 +++++++++++++++++++ .../static/src/js/views/form/form_renderer.js | 21 -------------- .../src/js/views/kanban/kanban_renderer.js | 13 +-------- .../js/views/list/list_editable_renderer.js | 11 -------- addons/web/static/src/scss/fields.scss | 3 +- .../static/tests/fields/basic_fields_tests.js | 8 +++++- addons/web/static/tests/views/list_tests.js | 12 ++++++-- 7 files changed, 47 insertions(+), 49 deletions(-) diff --git a/addons/web/static/src/js/views/basic/basic_renderer.js b/addons/web/static/src/js/views/basic/basic_renderer.js index b324d2ea772..16ecc473915 100644 --- a/addons/web/static/src/js/views/basic/basic_renderer.js +++ b/addons/web/static/src/js/views/basic/basic_renderer.js @@ -52,6 +52,7 @@ var BasicRenderer = AbstractRenderer.extend(WidgetAdapterMixin, { * Called each time the renderer is attached into the DOM. */ on_attach_callback: function () { + this._isInDom = true; // call on_attach_callback on field widgets for (const handle in this.allFieldWidgets) { this.allFieldWidgets[handle].forEach(widget => { @@ -60,6 +61,12 @@ var BasicRenderer = AbstractRenderer.extend(WidgetAdapterMixin, { } }); } + // call on_attach_callback on widgets + this.widgets.forEach(widget => { + if (widget.on_attach_callback) { + widget.on_attach_callback(); + } + }); // call on_attach_callback on child components (including field components) WidgetAdapterMixin.on_attach_callback.call(this); }, @@ -67,6 +74,7 @@ var BasicRenderer = AbstractRenderer.extend(WidgetAdapterMixin, { * Called each time the renderer is detached from the DOM. */ on_detach_callback: function () { + this._isInDom = false; // call on_detach_callback on field widgets for (const handle in this.allFieldWidgets) { this.allFieldWidgets[handle].forEach(widget => { @@ -75,6 +83,12 @@ var BasicRenderer = AbstractRenderer.extend(WidgetAdapterMixin, { } }); } + // call on_detach_callback on widgets + this.widgets.forEach(widget => { + if (widget.on_detach_callback) { + widget.on_detach_callback(); + } + }); // call on_detach_callback on child components (including field components) WidgetAdapterMixin.on_detach_callback.call(this); }, @@ -667,6 +681,20 @@ var BasicRenderer = AbstractRenderer.extend(WidgetAdapterMixin, { for (const widget of oldWidgets) { widget.destroy(); } + if (this._isInDom) { + for (const handle in this.allFieldWidgets) { + this.allFieldWidgets[handle].forEach(widget => { + if (widget.on_attach_callback) { + widget.on_attach_callback(); + } + }); + } + this.widgets.forEach(widget => { + if (widget.on_attach_callback) { + widget.on_attach_callback(); + } + }); + } }, /** * Instantiates the appropriate AbstractField specialization for the given diff --git a/addons/web/static/src/js/views/form/form_renderer.js b/addons/web/static/src/js/views/form/form_renderer.js index 97e3cbd8cb0..b6e1593b215 100644 --- a/addons/web/static/src/js/views/form/form_renderer.js +++ b/addons/web/static/src/js/views/form/form_renderer.js @@ -52,21 +52,6 @@ var FormRenderer = BasicRenderer.extend({ this._applyFormSizeClass(); return this._super.apply(this, arguments); }, - /** - * Called each time the form view is attached into the DOM - */ - on_attach_callback: function () { - this._isInDom = true; - _.invoke(this.widgets, 'on_attach_callback'); - this._super.apply(this, arguments); - }, - /** - * Called each time the renderer is detached from the DOM. - */ - on_detach_callback: function () { - this._isInDom = false; - this._super.apply(this, arguments); - }, //-------------------------------------------------------------------------- // Public @@ -1053,12 +1038,6 @@ var FormRenderer = BasicRenderer.extend({ if (self.lastActivatedFieldIndex >= 0) { self._activateNextFieldWidget(self.state, self.lastActivatedFieldIndex); } - if (self._isInDom) { - _.forEach(self.allFieldWidgets, function (widgets){ - _.invoke(widgets, 'on_attach_callback'); - }); - _.invoke(self.widgets, 'on_attach_callback'); - } }).guardedCatch(function () { $form.remove(); }); diff --git a/addons/web/static/src/js/views/kanban/kanban_renderer.js b/addons/web/static/src/js/views/kanban/kanban_renderer.js index 555bbd665fa..dfaba0a1a4f 100644 --- a/addons/web/static/src/js/views/kanban/kanban_renderer.js +++ b/addons/web/static/src/js/views/kanban/kanban_renderer.js @@ -141,19 +141,11 @@ var KanbanRenderer = BasicRenderer.extend({ * Called each time the renderer is attached into the DOM. */ on_attach_callback: function () { - this._isInDom = true; - _.invoke(this.widgets, 'on_attach_callback'); + this._super(...arguments); if (this.quickCreate) { this.quickCreate.on_attach_callback(); } }, - /** - * Called each time the renderer is detached from the DOM. - */ - on_detach_callback: function () { - this._isInDom = false; - _.invoke(this.widgets, 'on_detach_callback'); - }, //-------------------------------------------------------------------------- // Public @@ -469,9 +461,6 @@ var KanbanRenderer = BasicRenderer.extend({ self.$el.toggleClass('o_kanban_ungrouped', !isGrouped); self.$el.append(fragment); self._toggleNoContentHelper(); - if (self._isInDom) { - _.invoke(self.widgets, 'on_attach_callback'); - } }); }); }, diff --git a/addons/web/static/src/js/views/list/list_editable_renderer.js b/addons/web/static/src/js/views/list/list_editable_renderer.js index 99bbcfbb814..41bc2ecf1bd 100644 --- a/addons/web/static/src/js/views/list/list_editable_renderer.js +++ b/addons/web/static/src/js/views/list/list_editable_renderer.js @@ -127,23 +127,12 @@ ListRenderer.include({ * @override */ on_attach_callback: function () { - this.isInDOM = true; this._super(); // _freezeColumnWidths requests style information, which produces a // repaint, so we call it after _super to prevent flickering (in case // other code would also modify the DOM post rendering/before repaint) this._freezeColumnWidths(); }, - /** - * The list renderer needs to know if it is in the DOM to properly compute - * column widths. - * - * @override - */ - on_detach_callback: function () { - this.isInDOM = false; - this._super(); - }, //-------------------------------------------------------------------------- // Public diff --git a/addons/web/static/src/scss/fields.scss b/addons/web/static/src/scss/fields.scss index 0e948c5921e..70436149d23 100644 --- a/addons/web/static/src/scss/fields.scss +++ b/addons/web/static/src/scss/fields.scss @@ -499,9 +499,10 @@ border: 0; font-size: 12px; user-select: none; - background-color: rgba(lightgray, 0.5);; + background-color: rgba(lightgray, 0.5); font-weight: 500; @include o-text-overflow; + transition: none; // remove transition to prevent badges from flickering at reload } } diff --git a/addons/web/static/tests/fields/basic_fields_tests.js b/addons/web/static/tests/fields/basic_fields_tests.js index 8debec81b3a..65df3a5af19 100644 --- a/addons/web/static/tests/fields/basic_fields_tests.js +++ b/addons/web/static/tests/fields/basic_fields_tests.js @@ -7606,7 +7606,7 @@ QUnit.module('basic_fields', { }); QUnit.test('FieldBadge component with decoration-xxx attributes', async function (assert) { - assert.expect(3); + assert.expect(6); const list = await createView({ View: ListView, @@ -7623,6 +7623,12 @@ QUnit.module('basic_fields', { assert.containsOnce(list, '.o_field_badge[name="foo"].bg-danger-light'); assert.containsOnce(list, '.o_field_badge[name="foo"].bg-warning-light'); + await list.reload(); + + assert.containsN(list, '.o_field_badge[name="foo"]', 5); + assert.containsOnce(list, '.o_field_badge[name="foo"].bg-danger-light'); + assert.containsOnce(list, '.o_field_badge[name="foo"].bg-warning-light'); + list.destroy(); }); }); diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index 2ea2e0396a9..900375d0b59 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -10618,7 +10618,7 @@ QUnit.module('Views', { QUnit.test('list view with field component: mounted and willUnmount calls', async function (assert) { // this test could be removed as soon as the list view will be written in Owl - assert.expect(3); + assert.expect(7); let mountedCalls = 0; let willUnmountCalls = 0; @@ -10641,10 +10641,16 @@ QUnit.module('Views', { }); assert.containsN(list, '.o_data_row', 4); - list.destroy(); - assert.strictEqual(mountedCalls, 4); + assert.strictEqual(willUnmountCalls, 0); + + await list.reload(); + assert.strictEqual(mountedCalls, 8); assert.strictEqual(willUnmountCalls, 4); + + list.destroy(); + assert.strictEqual(mountedCalls, 8); + assert.strictEqual(willUnmountCalls, 8); }); QUnit.test('editable list view: multi edition of owl field component', async function (assert) {