From eb4a7d18b9a0efe972360d6a2d7f767cfa1d4a82 Mon Sep 17 00:00:00 2001 From: "Bastien Fafchamps (bafa)" Date: Wed, 22 Mar 2023 13:07:59 +0000 Subject: [PATCH] [FIX] web: SelectMenu value with no corresponding choice should not be displayed This commit fixes an issue where when a value without a corresponding choice was given to the SelectMenu, we would display it as a string. It was missleading as it looked like an option was selected. Now giving a value with no corresponding choice will result in the same behaviour as giving no value. Part-of: odoo/odoo#115799 --- .../src/core/select_menu/select_menu.js | 63 ++++++++++------ .../src/core/select_menu/select_menu.xml | 2 +- .../static/tests/core/select_menu_tests.js | 74 +++++++++++++++++-- 3 files changed, 106 insertions(+), 33 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 804519fbaed..4f27a5b6c99 100644 --- a/addons/web/static/src/core/select_menu/select_menu.js +++ b/addons/web/static/src/core/select_menu/select_menu.js @@ -1,6 +1,6 @@ /** @odoo-module **/ -import { Component, useState, useRef } from "@odoo/owl"; +import { Component, useState, useRef, onWillUpdateProps } from "@odoo/owl"; import { Dropdown } from "@web/core/dropdown/dropdown"; import { DropdownItem } from "@web/core/dropdown/dropdown_item"; import { _lt } from "@web/core/l10n/translation"; @@ -88,20 +88,25 @@ export class SelectMenu extends Component { () => this.onInput(this.inputRef.el ? this.inputRef.el.value.trim() : ""), 250 ); + + this.selectedChoice = this.getSelectedChoice(this.props); + onWillUpdateProps((nextProps) => { + if (this.props.value !== nextProps.value) { + this.selectedChoice = this.getSelectedChoice(nextProps); + } + }); } get displayValue() { - if (this.props.value !== undefined) { - const choices = [...this.props.choices, ...this.props.groups.flatMap((g) => g.choices)]; - const value = choices.find((c) => { - if (typeof c.value === "object" && typeof this.props.value === "object") { - return shallowEqual(c.value, this.props.value); - } - return c.value === this.props.value; - }); - return value ? value.label : this.props.value; - } - return ""; + return this.selectedChoice ? this.selectedChoice.label : ""; + } + + get canDeselect() { + return ( + !this.props.required && + this.selectedChoice !== undefined && + this.selectedChoice !== null + ); } get multiSelectChoices() { @@ -150,10 +155,6 @@ export class SelectMenu extends Component { } } - canClear() { - return !this.props.required && this.props.value != null; - } - onInput(searchString) { this.filterOptions(searchString); @@ -164,6 +165,28 @@ export class SelectMenu extends Component { } } + getSelectedChoice(props) { + if (props.value) { + const choices = [...props.choices, ...props.groups.flatMap((g) => g.choices)]; + return choices.find((c) => { + if (typeof c.value === "object" && typeof props.value === "object") { + return shallowEqual(c.value, props.value); + } + return c.value === props.value; + }); + } else { + return undefined; + } + } + + onItemSelected(value) { + if (this.props.multiSelect) { + this.props.onSelect([...this.props.value, value]); + } else { + this.props.onSelect(value); + } + } + // ========================================================================================== // # Search # // ========================================================================================== @@ -238,14 +261,6 @@ export class SelectMenu extends Component { return choices.flatMap((choice, index) => (index === 0 ? 0 : choice.isGroup ? index : [])); } - onItemSelected(value) { - if (this.props.multiSelect) { - this.props.onSelect([...this.props.value, value]); - } else { - this.props.onSelect(value); - } - } - // ========================================================================================== // # Scroll # // ========================================================================================== 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 1709c6525e8..a28021ea390 100644 --- a/addons/web/static/src/core/select_menu/select_menu.xml +++ b/addons/web/static/src/core/select_menu/select_menu.xml @@ -22,7 +22,7 @@ - + diff --git a/addons/web/static/tests/core/select_menu_tests.js b/addons/web/static/tests/core/select_menu_tests.js index 3ef884d0579..ec18c1589f2 100644 --- a/addons/web/static/tests/core/select_menu_tests.js +++ b/addons/web/static/tests/core/select_menu_tests.js @@ -100,16 +100,12 @@ QUnit.module("Web Components", (hooks) => { class Parent extends Component { setup() { this.state = useState({ value: "world" }); - this.choices = [ - { label: "Hello", value: "hello" }, - ]; + this.choices = [{ label: "Hello", value: "hello" }]; this.groups = [ { label: "Group A", - choices: [ - { label: "World", value: "world" }, - ] - } + choices: [{ label: "World", value: "world" }], + }, ]; } @@ -230,12 +226,74 @@ QUnit.module("Web Components", (hooks) => { ); }); + QUnit.test("Value with no corresponding choices displays as if no choice was selected", async (assert) => { + class Parent extends Component { + static components = { SelectMenu }; + static template = xml` + + `; + setup() { + this.choices = [ + { label: "World", value: "world" }, + { label: "Hello", value: "hello" }, + ]; + this.state = useState({ value: "coucou"}); + } + setValue(newValue) { + this.state.value = newValue; + } + } + + await mount(Parent, target, { env }); + assert.equal(getValue(), "", `The toggler should be empty`); + }); + + QUnit.test("Changing value props properly updates the selected choice", async (assert) => { + class Parent extends Component { + static components = { SelectMenu }; + static template = xml` + + `; + setup() { + this.choices = [ + { label: "Z", value: { hello: "world" } }, + { label: "A", value: { paper: "company" } }, + ]; + this.state = useState({ value: { paper: "company" }}); + } + setValue(newValue) { + this.state.value = newValue; + } + } + + const comp = await mount(Parent, target, { env }); + assert.equal( + getValue(), + "A", + `The select value shoud be "A"` + ); + + comp.setValue({ hello: "world" }); + await nextTick(); + assert.equal( + getValue(), + "Z", + `After changing the value props, the select value shoud be "Z"` + ); + }); + QUnit.test( "Clear button calls 'onSelect' with null value and appears only when value is not null", async (assert) => { class Parent extends Component { setup() { - this.state = useState({ value: "Hello" }); + this.state = useState({ value: "hello" }); this.choices = [ { label: "Hello", value: "hello" }, { label: "World", value: "world" },