From a36fdc43cd2c4eb51b1c5809c03b63dc105cd84f Mon Sep 17 00:00:00 2001 From: "Simon Genin (ges)" Date: Thu, 22 Sep 2022 11:13:23 +0000 Subject: [PATCH] [IMP] web: toggling an optional field in list view do not unselect row Toggling an optional field was unselecting the row, but didn't correctly copy the same bahavior than legacy when it comes to the saving and discarding of the data being edited. However, by simply removing the line of code unselecting the row, we get a better ux for no cost. The user can continue the record edition after adding or removing columns. Note that the focus will always be reset to the first column though. closes odoo/odoo#102142 X-original-commit: 63c87eac6173af13ec84f90a7f25f07a074197f3 Signed-off-by: Aaron Bohy (aab) Signed-off-by: Simon Genin (ges@odoo) --- .../static/src/views/list/list_renderer.js | 1 - .../views/fields/one2many_field_tests.js | 4 +- .../web/static/tests/views/list_view_tests.js | 81 ++++++++++++++----- 3 files changed, 64 insertions(+), 22 deletions(-) diff --git a/addons/web/static/src/views/list/list_renderer.js b/addons/web/static/src/views/list/list_renderer.js index b450d7d2fe3..98c270a16c7 100644 --- a/addons/web/static/src/views/list/list_renderer.js +++ b/addons/web/static/src/views/list/list_renderer.js @@ -1514,7 +1514,6 @@ export class ListRenderer extends Component { } async toggleOptionalField(fieldName) { - await this.props.list.unselectRecord(true); this.optionalActiveFields[fieldName] = !this.optionalActiveFields[fieldName]; this.state.columns = this.getActiveColumns(this.props.list); this.saveOptionalActiveFields( diff --git a/addons/web/static/tests/views/fields/one2many_field_tests.js b/addons/web/static/tests/views/fields/one2many_field_tests.js index 7f49dd07950..437e2c8d767 100644 --- a/addons/web/static/tests/views/fields/one2many_field_tests.js +++ b/addons/web/static/tests/views/fields/one2many_field_tests.js @@ -11470,10 +11470,10 @@ QUnit.module("Fields", (hooks) => { await click(target.querySelector(".o_optional_columns_dropdown .dropdown-toggle")); await click(target.querySelector(".o_optional_columns_dropdown .dropdown-item")); - assert.containsNone( + assert.containsOnce( target.querySelector(".o_field_one2many"), "tr.o_selected_row", - "current edition mode discarded when selecting advanced field" + "current edition mode kept when selecting advanced field" ); assert.containsN( target.querySelector(".o_field_one2many"), diff --git a/addons/web/static/tests/views/list_view_tests.js b/addons/web/static/tests/views/list_view_tests.js index 407c647168b..3198da57c75 100644 --- a/addons/web/static/tests/views/list_view_tests.js +++ b/addons/web/static/tests/views/list_view_tests.js @@ -101,7 +101,11 @@ QUnit.module("Views", (hooks) => { o2m: { string: "O2M field", type: "one2many", relation: "bar" }, m2m: { string: "M2M field", type: "many2many", relation: "bar" }, amount: { string: "Monetary field", type: "monetary" }, - amount_currency: { string: "Monetary field (currency)", type: "monetary", currency_field: "company_currency_id" }, + amount_currency: { + string: "Monetary field (currency)", + type: "monetary", + currency_field: "company_currency_id", + }, currency_id: { string: "Currency", type: "many2one", @@ -2983,31 +2987,34 @@ QUnit.module("Views", (hooks) => { ); }); - QUnit.test("currency_field is taken into account when formatting monetary values", async (assert) => { - await makeView({ - type: "list", - resModel: "foo", - serverData, - arch: ` + QUnit.test( + "currency_field is taken into account when formatting monetary values", + async (assert) => { + await makeView({ + type: "list", + resModel: "foo", + serverData, + arch: ` `, - }); + }); - assert.strictEqual( - target.querySelectorAll('.o_data_row td[name="amount"]')[0].textContent, - "1200.00\u00a0€", - "field should be formatted based on currency_id" - ); - assert.strictEqual( - target.querySelectorAll('.o_data_row td[name="amount_currency"]')[0].textContent, - "$\u00a01100.00", - "field should be formatted based on company_currency_id" - ); - }); + assert.strictEqual( + target.querySelectorAll('.o_data_row td[name="amount"]')[0].textContent, + "1200.00\u00a0€", + "field should be formatted based on currency_id" + ); + assert.strictEqual( + target.querySelectorAll('.o_data_row td[name="amount_currency"]')[0].textContent, + "$\u00a01100.00", + "field should be formatted based on company_currency_id" + ); + } + ); QUnit.test( "groups can not be sorted on a different field than the first field of the groupBy - 1", @@ -15337,4 +15344,40 @@ QUnit.module("Views", (hooks) => { ); } ); + + QUnit.test("optional field selection do not unselect current row", async function (assert) { + await makeView({ + type: "list", + resModel: "foo", + serverData, + arch: ` + + + + `, + }); + + await click(target.querySelector(".o_list_button_add")); + + assert.containsOnce(target, ".o_selected_row"); + assert.containsOnce(target, "div[name=foo] input:focus"); + + await click(target, "table .o_optional_columns_dropdown .dropdown-toggle"); + + assert.containsOnce(target, ".o_selected_row"); + assert.containsOnce(target, "div[name=foo] input:focus"); + + await click(target, ".o_optional_columns_dropdown span.dropdown-item:nth-child(3) label"); + + assert.containsOnce(target, ".o_selected_row"); + assert.containsOnce(target, "div[name=foo] input:focus"); + assert.containsOnce(target, ".o_selected_row div[name=bar]"); + + await click(target, ".o_optional_columns_dropdown span.dropdown-item:nth-child(1) label"); + + assert.containsOnce(target, ".o_selected_row"); + // This below would be better if it still focused foo, but it is an acceptable tradeoff. + assert.containsOnce(target, "div[name=text] textarea:focus"); + assert.containsOnce(target, ".o_selected_row div[name=text]"); + }); });