From 5696b5686e5b76cb5eab6e634455ee0ef46de000 Mon Sep 17 00:00:00 2001 From: FrancoisGe Date: Thu, 26 Oct 2023 11:52:34 +0200 Subject: [PATCH] [FIX] web: select all records should trigger only one rendered Purpose: -------- From the new RelationalModel (odoo/odoo#114024), in the list view, when all the records are selected, the ListController is rendered once per record. This causes performance problems and can cause a flicker where each record is selected one by one. Why: ---- When selecting all records, we will call record.togggleSelection() on each record but this function uses a mutex. So each record will be selected asynchronously. As the state is not modified by the same tick, a rendering will be triggered for each call to record.togggleSelection. Solution: --------- Add a togggleSelection function to the DynamicList which will select all the records in the same mutex. The state will therefore be modified in the same tick and there will be a single rendering. closes odoo/odoo#139859 Signed-off-by: Francois Georis (fge) --- .../model/relational_model/dynamic_list.js | 15 ++++++++++++++ .../src/model/relational_model/record.js | 14 ++++++++----- .../static/src/views/list/list_renderer.js | 11 +--------- .../web/static/tests/views/list_view_tests.js | 20 ++++++++++++++++--- 4 files changed, 42 insertions(+), 18 deletions(-) diff --git a/addons/web/static/src/model/relational_model/dynamic_list.js b/addons/web/static/src/model/relational_model/dynamic_list.js index c345cf95db7..16ecbb5ddd2 100644 --- a/addons/web/static/src/model/relational_model/dynamic_list.js +++ b/addons/web/static/src/model/relational_model/dynamic_list.js @@ -176,6 +176,21 @@ export class DynamicList extends DataPoint { }); } + toggleSelection() { + return this.model.mutex.exec(() => { + if (this.selection.length === this.records.length) { + this.records.forEach((record) => { + record._toggleSelection(false); + }); + this.isDomainSelected = false; + } else { + this.records.forEach((record) => { + record._toggleSelection(true); + }); + } + }); + } + unarchive(isSelected) { return this.model.mutex.exec(() => this._toggleArchive(isSelected, false)); } diff --git a/addons/web/static/src/model/relational_model/record.js b/addons/web/static/src/model/relational_model/record.js index ccb314fade6..d3669d44263 100644 --- a/addons/web/static/src/model/relational_model/record.js +++ b/addons/web/static/src/model/relational_model/record.js @@ -239,11 +239,7 @@ export class Record extends DataPoint { toggleSelection(selected) { return this.model.mutex.exec(() => { - if (typeof selected === "boolean") { - this.selected = selected; - } else { - this.selected = !this.selected; - } + this._toggleSelection(selected); }); } @@ -1102,6 +1098,14 @@ export class Record extends DataPoint { } } + _toggleSelection(selected) { + if (typeof selected === "boolean") { + this.selected = selected; + } else { + this.selected = !this.selected; + } + } + async _getOnchangeValues(changes) { const onChangeFields = Object.keys(changes).filter( (fieldName) => this.activeFields[fieldName] && this.activeFields[fieldName].onChange diff --git a/addons/web/static/src/views/list/list_renderer.js b/addons/web/static/src/views/list/list_renderer.js index 2daaf276709..29bc25129b8 100644 --- a/addons/web/static/src/views/list/list_renderer.js +++ b/addons/web/static/src/views/list/list_renderer.js @@ -1801,16 +1801,7 @@ export class ListRenderer extends Component { if (!this.canSelectRecord) { return; } - if (list.selection.length === list.records.length) { - list.records.forEach((record) => { - record.toggleSelection(false); - list.selectDomain(false); - }); - } else { - list.records.forEach((record) => { - record.toggleSelection(true); - }); - } + return list.toggleSelection(); } toggleRecordSelection(record, ev) { diff --git a/addons/web/static/tests/views/list_view_tests.js b/addons/web/static/tests/views/list_view_tests.js index ce9ad27dd85..6a26f820c33 100644 --- a/addons/web/static/tests/views/list_view_tests.js +++ b/addons/web/static/tests/views/list_view_tests.js @@ -1,6 +1,6 @@ /** @odoo-module **/ -import { Component, markup, onWillStart, xml } from "@odoo/owl"; +import { Component, markup, onRendered, onWillStart, xml } from "@odoo/owl"; import { browser } from "@web/core/browser/browser"; import { Domain } from "@web/core/domain"; import { currencies } from "@web/core/currency"; @@ -2546,7 +2546,7 @@ QUnit.module("Views", (hooks) => { }); QUnit.test("many2one field rendering with many2one widget", async function (assert) { - serverData.models.bar.records[0].display_name = false + serverData.models.bar.records[0].display_name = false; await makeView({ type: "list", resModel: "foo", @@ -2561,7 +2561,7 @@ QUnit.module("Views", (hooks) => { }); QUnit.test("many2one field rendering when display_name is falsy", async function (assert) { - serverData.models.bar.records[0].display_name = false + serverData.models.bar.records[0].display_name = false; await makeView({ type: "list", resModel: "foo", @@ -3270,6 +3270,15 @@ QUnit.module("Views", (hooks) => { }); QUnit.test("selection changes are triggered correctly", async function (assert) { + patchWithCleanup(ListController.prototype, { + setup() { + super.setup(...arguments); + onRendered(() => { + assert.step("onRendered ListController"); + }); + }, + }); + const list = await makeView({ type: "list", resModel: "foo", @@ -3281,6 +3290,7 @@ QUnit.module("Views", (hooks) => { assert.strictEqual(list.model.root.selection.length, 0, "no record should be selected"); assert.notOk(tbody_selector.checked, "selection checkbox should be checked"); + assert.verifySteps(["onRendered ListController"]); // tbody checkbox click await click(tbody_selector); @@ -3294,10 +3304,12 @@ QUnit.module("Views", (hooks) => { "the correct record should be selected" ); assert.ok(tbody_selector.checked, "selection checkbox should be checked"); + assert.verifySteps(["onRendered ListController"]); await click(tbody_selector); assert.strictEqual(list.model.root.selection.length, 0, "no record should be selected"); assert.notOk(tbody_selector.checked, "selection checkbox should be checked"); + assert.verifySteps(["onRendered ListController"]); // head checkbox click await click(thead_selector); @@ -3308,6 +3320,7 @@ QUnit.module("Views", (hooks) => { target.querySelectorAll("tbody tr").length, "all selection checkboxes should be checked" ); + assert.verifySteps(["onRendered ListController"]); await click(thead_selector); assert.strictEqual(list.model.root.selection.length, 0, "no records should be selected"); @@ -3316,6 +3329,7 @@ QUnit.module("Views", (hooks) => { "tbody .o_list_record_selector input:checked", "no selection checkbox should be checked" ); + assert.verifySteps(["onRendered ListController"]); }); QUnit.test(