From 3615fb3c7f1bfeb9adeec251cf33cda50f1e7324 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Thu, 12 Sep 2019 06:30:11 +0000 Subject: [PATCH] [FIX] web: list: disable record selectors when editing Having those inputs enabled can lead to weird situations where a record has been modified but can't be saved anymore (because the user clicked on one of those record selectors to focusout the edited row). Also we don't want the user to start multi-editing a field, then change the selection, and then validate the changes. Task 2068280 --- .../js/views/list/list_editable_renderer.js | 30 +++++++++++++++++-- addons/web/static/tests/views/list_tests.js | 20 +++++++++++-- 2 files changed, 44 insertions(+), 6 deletions(-) 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 1aec18f6fb7..deb29554798 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 @@ -356,6 +356,7 @@ ListRenderer.include({ } if ($row.prop('rowIndex') - 1 === this.currentRow) { this.currentRow = null; + this._enableRecordSelectors(); } // destroy widgets first @@ -450,7 +451,11 @@ ListRenderer.include({ // Toggle selected class here so that style is applied at the end $row.toggleClass('o_selected_row', editMode); - $row.find('.o_list_record_selector input').prop('disabled', !record.res_id); + if (editMode) { + this._disableRecordSelectors(); + } else { + this._enableRecordSelectors(); + } return Promise.all(defs).then(function () { // necessary to trigger resize on fieldtexts @@ -492,6 +497,7 @@ ListRenderer.include({ onFailure: reject, }); }).then(changedFields => { + this._enableRecordSelectors(); // If any field has changed and if the list is in multiple edition, // we send a truthy boolean to _selectRow to tell it not to select // the following record. @@ -541,7 +547,14 @@ ListRenderer.include({ el.addEventListener(type, callback, options); this.eventListeners.push({ type, el, callback, options }); }, - + /** + * When editing a row, we want to disable all record selectors. + * + * @private + */ + _disableRecordSelectors: function () { + this.$('.o_list_record_selector input').attr('disabled', 'disabled'); + }, /** * Destroy all field widgets corresponding to a record. Useful when we are * removing a useless row. @@ -555,6 +568,12 @@ ListRenderer.include({ delete this.allFieldWidgets[recordID]; } }, + /** + * @private + */ + _enableRecordSelectors: function () { + this.$('.o_list_record_selector input').attr('disabled', false); + }, /** * This function freezes the column widths and forces a fixed table-layout, * once the browser has computed the optimal width of each column according @@ -637,6 +656,7 @@ ListRenderer.include({ return fixedWidths[type] || '1'; }, /** + * @private * * @returns {integer} */ @@ -1193,7 +1213,10 @@ ListRenderer.include({ return new Promise(function (resolve) { self.trigger_up('edit_line', { recordId: recordId, - onSuccess: resolve, + onSuccess: function () { + self._disableRecordSelectors(); + resolve(); + }, }); }); }); @@ -1384,6 +1407,7 @@ ListRenderer.include({ this.trigger_up('discard_changes', { recordID: ev.target.dataPointID, onSuccess: function () { + self._enableRecordSelectors(); var recordId = self._getRecordID(rowIndex); if (recordId) { var correspondingRow = self._getRow(recordId); diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index 16dfad681ac..0311fab3062 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -214,7 +214,7 @@ QUnit.module('Views', { }); QUnit.test('simple editable rendering', async function (assert) { - assert.expect(12); + assert.expect(15); var list = await createView({ View: ListView, @@ -225,6 +225,7 @@ QUnit.module('Views', { assert.containsN(list, 'th', 3, "should have 2 th"); assert.containsN(list, 'th', 3, "should have 3 th"); + assert.containsN(list, '.o_list_record_selector input:enabled', 5); assert.containsOnce(list, 'td:contains(yop)', "should contain yop"); assert.isVisible(list.$buttons.find('.o_list_button_add'), @@ -242,6 +243,7 @@ QUnit.module('Views', { "should have a visible save button"); assert.isVisible(list.$buttons.find('.o_list_button_discard'), "should have a visible discard button"); + assert.containsNone(list, '.o_list_record_selector input:enabled'); await testUtils.dom.click(list.$buttons.find('.o_list_button_save')); @@ -251,6 +253,8 @@ QUnit.module('Views', { "should not have a visible save button"); assert.isNotVisible(list.$buttons.find('.o_list_button_discard'), "should not have a visible discard button"); + assert.containsN(list, '.o_list_record_selector input:enabled', 5); + list.destroy(); }); @@ -3662,7 +3666,7 @@ QUnit.module('Views', { }); QUnit.test('edition: create new line, then discard', async function (assert) { - assert.expect(8); + assert.expect(11); var list = await createView({ View: ListView, @@ -3677,11 +3681,13 @@ QUnit.module('Views', { "create button should be visible"); assert.strictEqual(list.$buttons.find('.o_list_button_discard:visible').length, 0, "discard button should be hidden"); + assert.containsN(list, '.o_list_record_selector input:enabled', 5); await testUtils.dom.click(list.$buttons.find('.o_list_button_add')); assert.strictEqual(list.$buttons.find('.o_list_button_add:visible').length, 0, "create button should be hidden"); assert.strictEqual(list.$buttons.find('.o_list_button_discard:visible').length, 1, "discard button should be visible"); + assert.containsNone(list, '.o_list_record_selector input:enabled'); await testUtils.dom.click(list.$buttons.find('.o_list_button_discard')); assert.containsN(list, 'tr.o_data_row', 4, "should still have 4 records"); @@ -3689,6 +3695,8 @@ QUnit.module('Views', { "create button should be visible again"); assert.strictEqual(list.$buttons.find('.o_list_button_discard:visible').length, 0, "discard button should be hidden again"); + assert.containsN(list, '.o_list_record_selector input:enabled', 5); + list.destroy(); }); @@ -5373,7 +5381,7 @@ QUnit.module('Views', { }); QUnit.test('editable list view: multi edition error and cancellation handling', async function (assert) { - assert.expect(3); + assert.expect(8); var list = await createView({ View: ListView, @@ -5384,22 +5392,28 @@ QUnit.module('Views', { '', }); + assert.containsN(list, '.o_list_record_selector input:enabled', 5); + // select two records await testUtils.dom.click(list.$('.o_data_row:eq(0) .o_list_record_selector input')); await testUtils.dom.click(list.$('.o_data_row:eq(1) .o_list_record_selector input')); // edit a line and cancel await testUtils.dom.click(list.$('.o_data_row:eq(0) .o_data_cell:eq(0)')); + assert.containsNone(list, '.o_list_record_selector input:enabled'); await testUtils.fields.editInput(list.$('.o_selected_row .o_field_widget[name=foo]'), "abc"); await testUtils.dom.click($('.modal .btn:contains("Cancel")')); assert.strictEqual(list.$('.o_data_row:eq(0) .o_data_cell').text(), 'yop', "first cell should have discarded any change"); + assert.containsN(list, '.o_list_record_selector input:enabled', 5); // edit a line with an invalid value await testUtils.dom.click(list.$('.o_data_row:eq(0) .o_data_cell:eq(0)')); + assert.containsNone(list, '.o_list_record_selector input:enabled'); await testUtils.fields.editInput(list.$('.o_selected_row .o_field_widget[name=foo]'), ""); assert.containsOnce(document.body, '.modal', "there should be an opened modal"); await testUtils.dom.click($('.modal .btn-primary')); assert.strictEqual(list.$('.o_data_row:eq(0) .o_data_cell').text(), 'yop', "changes should be discarded"); + assert.containsN(list, '.o_list_record_selector input:enabled', 5); list.destroy(); });