From 7af78e6aa5430fd7f19979673074716cd6ca89ab Mon Sep 17 00:00:00 2001 From: "Julien Segers (juse)" Date: Thu, 14 Dec 2023 15:39:44 +0100 Subject: [PATCH] [PERF] web: avoid unnecessary sorting in select menus Sorting is being computed on each mounted select menu during the useEffect() hook since this commit: 8a44857. This can lead to long wait times or browser crashes when there are many select menus with many options. Before this commit, all of the select menu were sorted when the user select a value in one of them. This is not necessary as the sorting is already handled in beforeOpen. After this commit, no sorting is being computed in useEffect() anymore to avoid unnecessary computation. A test has been added to verify the sorting happens when needed. opw-3616438 closes odoo/odoo#151318 X-original-commit: f58b88383ce6a7439c02dbe761dc9025ddfe3b0a Signed-off-by: Luca Vitali (luvi) Co-authored-by: Luca Vitali (luvi) --- .../src/core/select_menu/select_menu.js | 16 ++- .../src/core/select_menu/select_menu.xml | 2 +- .../static/tests/core/select_menu_tests.js | 101 ++++++++++++++++++ 3 files changed, 114 insertions(+), 5 deletions(-) diff --git a/addons/web/static/src/core/select_menu/select_menu.js b/addons/web/static/src/core/select_menu/select_menu.js index 63c439b6336..62402513cad 100644 --- a/addons/web/static/src/core/select_menu/select_menu.js +++ b/addons/web/static/src/core/select_menu/select_menu.js @@ -92,6 +92,7 @@ export class SelectMenu extends Component { () => this.onInput(this.inputRef.el ? this.inputRef.el.value.trim() : ""), 250 ); + this.isOpen = false; this.selectedChoice = this.getSelectedChoice(this.props); onWillUpdateProps((nextProps) => { @@ -101,8 +102,10 @@ export class SelectMenu extends Component { }); useEffect( () => { - const groups = [{ choices: this.props.choices }, ...this.props.groups]; - this.filterOptions(this.state.searchValue, groups); + if (this.isOpen) { + const groups = [{ choices: this.props.choices }, ...this.props.groups]; + this.filterOptions(this.state.searchValue, groups); + } }, () => [this.props.choices, this.props.groups] ); @@ -135,8 +138,13 @@ export class SelectMenu extends Component { }); } - onOpened() { - this.state.searchValue = ""; + onStateChanged({ open }) { + this.isOpen = open; + + if (!open) { + this.state.searchValue = ""; + return; + } const selectedElement = document.querySelector(".o_select_active"); if (selectedElement) { diff --git a/addons/web/static/src/core/select_menu/select_menu.xml b/addons/web/static/src/core/select_menu/select_menu.xml index 21088db6b41..ba61fc64880 100644 --- a/addons/web/static/src/core/select_menu/select_menu.xml +++ b/addons/web/static/src/core/select_menu/select_menu.xml @@ -8,8 +8,8 @@ menuClass="`o_select_menu_menu${props.searchable ? ' py-0' : ''}${props.multiSelect ? ' o_select_menu_multi_select' : ''} border`" position="'bottom-fit'" beforeOpen.bind="filterOptions" - onOpened.bind="onOpened" onScroll.bind="onScroll" + onStateChanged.bind="onStateChanged" > diff --git a/addons/web/static/tests/core/select_menu_tests.js b/addons/web/static/tests/core/select_menu_tests.js index 920f1868d05..16a0ebce5ea 100644 --- a/addons/web/static/tests/core/select_menu_tests.js +++ b/addons/web/static/tests/core/select_menu_tests.js @@ -1101,4 +1101,105 @@ QUnit.module("Web Components", (hooks) => { "SelectMenu has two updated choices available" ); }); + + QUnit.test("SelectMenu group items only after being opened", async (assert) => { + let count = 0; + + patchWithCleanup(SelectMenu.prototype, { + filterOptions(args) { + assert.step("filterOptions"); + super.filterOptions(args); + }, + }); + class Parent extends Component { + static components = { SelectMenu }; + static props = ["*"]; + static template = xml` + + `; + setup() { + this.state = useState({ + choices: [{ label: "Option A", value: "optionA" }], + groups: [ + { + label: "Group A", + choices: [ + { label: "Option C", value: "optionC" }, + { label: "Option B", value: "optionB" }, + ], + }, + ], + value: "hello", + }); + } + + onInput() { + count++; + assert.verifySteps( + ["filterOptions"], + "options have been filtered when typing on the search input" + ); + if (count === 1) { + this.state.choices = [{ label: "Option C", value: "optionC" }]; + this.state.groups = [ + { + label: "Group B", + choices: [{ label: "Option D", value: "optionD" }], + }, + ]; + } else { + this.state.choices = [{ label: "Option A", value: "optionA" }]; + this.state.groups = [ + { + label: "Group A", + choices: [ + { label: "Option C", value: "optionC" }, + { label: "Option B", value: "optionB" }, + ], + }, + ]; + } + } + } + + await mount(Parent, target, { env }); + assert.verifySteps([], "options have not yet been filtered"); + + await open(); + assert.strictEqual( + target.querySelector(".o_select_menu_menu").textContent, + "Option AGroup AOption BOption C" + ); + assert.verifySteps(["filterOptions"], "options have been filtered when the menu opens"); + + // edit the input, to trigger onInput and update the props + await editInput(target, "input.o_select_menu_sticky", "option d"); + await nextTick(); + assert.strictEqual( + target.querySelector(".o_select_menu_menu").textContent, + "Group BOption D", + "options and groups have been recomputed" + ); + assert.verifySteps( + ["filterOptions"], + "options have been filtered since the choices changed" + ); + + // edit the input, to trigger onInput and update the props + await editInput(target, "input.o_select_menu_sticky", ""); + await nextTick(); + assert.strictEqual( + target.querySelector(".o_select_menu_menu").textContent, + "Option AGroup AOption BOption C" + ); + assert.verifySteps( + ["filterOptions"], + "options have been filtered since the choices changed" + ); + }); });