diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js index 135d21e5adf..51986224c78 100644 --- a/addons/web/static/src/js/fields/relational_fields.js +++ b/addons/web/static/src/js/fields/relational_fields.js @@ -638,7 +638,7 @@ var FieldX2Many = AbstractField.extend({ return this._super(); } if (this.renderer) { - this.renderer.updateState(this.value); + this.renderer.updateState(this.value, {}); this.pager.updateState({ size: this.value.count }); return $.when(); } @@ -746,8 +746,8 @@ var FieldX2Many = AbstractField.extend({ onFailure: def.reject.bind(def), }); } else { - self.renderer.setRowMode(recordID, 'readonly'); - def.resolve(); + self.renderer.setRowMode(recordID, 'readonly') + .done(def.resolve.bind(def)); } }); return def; @@ -791,7 +791,8 @@ var FieldX2Many = AbstractField.extend({ */ _onEditLine: function (ev) { ev.stopPropagation(); - this.renderer.setRowMode(ev.data.recordID, 'edit'); + this.renderer.setRowMode(ev.data.recordID, 'edit') + .done(ev.data.onSuccess); }, /** * Updates the given record with the changes. diff --git a/addons/web/static/src/js/views/abstract_renderer.js b/addons/web/static/src/js/views/abstract_renderer.js index 70ad5350670..7d10f05eab8 100644 --- a/addons/web/static/src/js/views/abstract_renderer.js +++ b/addons/web/static/src/js/views/abstract_renderer.js @@ -65,15 +65,18 @@ return Widget.extend({ setLocalState: function (localState) { }, /** - * update the state of the view. It always retrigger a full rerender. + * Updates the state of the view. It retriggers a full rerender, unless told + * otherwise (for optimization for example). * * @param {any} state * @param {Object} params + * @param {boolean} [params.noRender=false] + * if true, the method only updates the state without rerendering * @returns {Deferred} */ updateState: function (state, params) { this.state = state; - return this._render(); + return params.noRender ? $.when() : this._render(); }, //-------------------------------------------------------------------------- diff --git a/addons/web/static/src/js/views/basic/basic_controller.js b/addons/web/static/src/js/views/basic/basic_controller.js index b7cd40d9e7c..f1e71bcfd5a 100644 --- a/addons/web/static/src/js/views/basic/basic_controller.js +++ b/addons/web/static/src/js/views/basic/basic_controller.js @@ -118,13 +118,18 @@ var BasicController = AbstractController.extend(FieldManagerMixin, { var self = this; recordID = recordID || this.handle; return this.canBeDiscarded(recordID).then(function (needDiscard) { - if (needDiscard) { // Just some optimization - self.model.discardChanges(recordID); - } if (options && options.readonlyIfRealDiscard && !needDiscard) { return; } - self._setMode('readonly', recordID); + + if (needDiscard) { // Just some optimization + self.model.discardChanges(recordID); + } + if (self.model.isNew(recordID)) { + self._abandonRecord(recordID); + return; + } + return self._confirmSave(recordID); }); }, /** @@ -386,7 +391,7 @@ var BasicController = AbstractController.extend(FieldManagerMixin, { if (options.stayInEdit) { return def; } else { - return def.then(this._setMode.bind(this, 'readonly', recordID)); + return def.then(this._confirmSave.bind(this, recordID)); } }, /** @@ -397,17 +402,13 @@ var BasicController = AbstractController.extend(FieldManagerMixin, { * @private * @param {string} mode - 'readonly' or 'edit' * @param {string} [recordID] + * @returns {Deferred} */ _setMode: function (mode, recordID) { - recordID = recordID || this.handle; - // If trying to make a temporary record readonly, discard the record - if (mode === 'readonly' && this.model.isNew(recordID)) { - this._abandonRecord(recordID); - return; - } - if (recordID === this.handle) { - this.update({mode: mode}, {reload: false}); + if ((recordID || this.handle) === this.handle) { + return this.update({mode: mode}, {reload: false}); } + return $.when(); }, /** * Helper method, to get the current environment variables from the model diff --git a/addons/web/static/src/js/views/form/form_controller.js b/addons/web/static/src/js/views/form/form_controller.js index f15bdfa5382..51357ce967e 100644 --- a/addons/web/static/src/js/views/form/form_controller.js +++ b/addons/web/static/src/js/views/form/form_controller.js @@ -67,7 +67,7 @@ var FormController = BasicController.extend({ }).then(function (handle) { self.handle = handle; self._updateEnv(); - self._setMode('edit'); + return self._setMode('edit'); }); }, /** @@ -183,7 +183,11 @@ var FormController = BasicController.extend({ */ _confirmSave: function (id) { if (id === this.handle) { - return this.reload(); + if (this.mode === 'readonly') { + return this.reload(); + } else { + return this._setMode('readonly'); + } } else { // a subrecord changed, so update the corresponding relational field // i.e. the one whose value is a record with the given id or a list diff --git a/addons/web/static/src/js/views/list/list_controller.js b/addons/web/static/src/js/views/list/list_controller.js index 241aeb8a104..b08291854d3 100644 --- a/addons/web/static/src/js/views/list/list_controller.js +++ b/addons/web/static/src/js/views/list/list_controller.js @@ -200,7 +200,7 @@ var ListController = BasicController.extend({ this.model.addDefaultRecord(this.handle, {position: this.editable}).then(function (recordID) { self._toggleNoContentHelper(false); var state = self.model.get(self.handle); - self.renderer.updateState(state); + self.renderer.updateState(state, {}); self.renderer.editRecord(recordID); }); }, @@ -231,13 +231,14 @@ var ListController = BasicController.extend({ */ _confirmSave: function (id) { var state = this.model.get(this.handle); - return this.renderer.confirmSave(state, id); + return this.renderer.updateState(state, {noRender: true}) + .then(this._setMode.bind(this, 'readonly', id)); }, /** * @override * @private */ - _getSidebarEnv: function() { + _getSidebarEnv: function () { var env = this._super.apply(this, arguments); var record = this.model.get(this.handle); return _.extend(env, {domain: record.getDomain()}); @@ -249,12 +250,14 @@ var ListController = BasicController.extend({ * @private * @param {string} mode * @param {string} [recordID] - default to main recordID + * @returns {Deferred} */ _setMode: function (mode, recordID) { - this._super.apply(this, arguments); if ((recordID || this.handle) !== this.handle) { - this.renderer.setRowMode(recordID, mode); this._updateButtons(mode); + return this.renderer.setRowMode(recordID, mode); + } else { + return this._super.apply(this, arguments); } }, /** @@ -356,7 +359,8 @@ var ListController = BasicController.extend({ */ _onEditLine: function (ev) { ev.stopPropagation(); - this._setMode('edit', ev.data.recordID); + this._setMode('edit', ev.data.recordID) + .done(ev.data.onSuccess); }, /** * Opens the Export Dialog 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 901e178eba6..65614e04983 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 @@ -85,20 +85,6 @@ ListRenderer.include({ } return this._super(recordID); }, - /** - * This method is called by the controller when the user has clicked 'save', - * and the model as actually saved the current changes. We then need to - * set the correct values for each cell. The difference with confirmChange - * is that the edited line is now in readonly mode, so we can just set its - * new value. - * - * @param {Object} state the new full state for the list - * @param {string} savedRecordID the id for the saved record - */ - confirmSave: function (state, savedRecordID) { - this.state = state; - this.setRowMode(savedRecordID, 'readonly'); - }, /** * Edit a given record in the list * @@ -143,6 +129,7 @@ ListRenderer.include({ * * @param {string} recordID * @param {string} mode + * @returns {Deferred} */ setRowMode: function (recordID, mode) { var self = this; @@ -155,40 +142,67 @@ ListRenderer.include({ this.currentRow = editMode ? rowIndex : null; var $row = this.$('.o_data_row:nth(' + rowIndex + ')'); + var $tds = $row.children('.o_data_cell'); + var oldWidgets = _.clone(this.allFieldWidgets[record.id]); + // When switching to edit mode, force the dimensions of all cells to + // their current value so that they won't change if their content + // changes, to prevent the view from flickering. if (editMode) { - // Instantiate column widgets and destroy potential readonly ones - var oldWidgets = _.clone(this.allFieldWidgets[record.id]); - var $tds = $row.children('.o_data_cell'); - // Force the size of each cell to its current value so that it - // won't change if its content changes, to prevent the view from - // flickering $tds.each(function () { var $td = $(this); $td.css({width: $td.outerWidth()}); }); - _.each(this.columns, function (node, colIndex) { - var $td = $tds.eq(colIndex); - var $newTd = self._renderBodyCell(record, node, colIndex, { - renderInvisible: true, - renderWidgets: true, - }); - - // TODO this is ugly... - if ($td.hasClass('o_list_button')) { - self._unregisterModifiersElement(node, record, $td.children()); - } - - $td.empty().append($newTd.contents()); - }); - _.each(oldWidgets, this._destroyFieldWidget.bind(this, record)); - } else { - for (var colIndex = 0; colIndex < this.columns.length; colIndex++) { - this._setCellValue(rowIndex, colIndex); - } } - $row.toggleClass('o_selected_row', editMode); // Toggle here so that style is applied at the end + // Prepare options for cell rendering (this depends on the mode) + var options = { + renderInvisible: editMode, + renderWidgets: editMode, + }; + if (!editMode) { + // Force 'readonly' mode for widgets in readonly rows as + // otherwise they default to the view mode which is 'edit' for + // an editable list view + options.mode = 'readonly'; + } + + // Switch each cell to the new mode; note: the '_renderBodyCell' + // function might fill the 'this.defs' variables with multiple deferred + // so we create the array and delete it after the rendering. + var defs = []; + this.defs = defs; + _.each(this.columns, function (node, colIndex) { + var $td = $tds.eq(colIndex); + var $newTd = self._renderBodyCell(record, node, colIndex, options); + + // Widgets are unregistered of modifiers data when they are + // destroyed. This is not the case for simple buttons so we have to + // do it here. + if ($td.hasClass('o_list_button')) { + self._unregisterModifiersElement(node, record, $td.children()); + } + + // For edit mode we only replace the content of the cell with its + // new content (invisible fields, editable fields, ...). + // For readonly mode, we replace the whole cell so that the + // dimensions of the cell are not forced anymore. + if (editMode) { + $td.empty().append($newTd.contents()); + } else { + self._unregisterModifiersElement(node, record, $td); + $td.replaceWith($newTd); + } + }); + delete this.defs; + + // Destroy old field widgets + _.each(oldWidgets, this._destroyFieldWidget.bind(this, record)); + + // Toggle selected class here so that style is applied at the end + $row.toggleClass('o_selected_row', editMode); + + return $.when.apply($, defs); }, //-------------------------------------------------------------------------- @@ -360,6 +374,7 @@ ListRenderer.include({ * Activates the row at the given row index. * * @param {integer} rowIndex + * @returns {Deferred} */ _selectRow: function (rowIndex) { // Do nothing if already selected @@ -372,40 +387,14 @@ ListRenderer.include({ return this._unselectRow().then(function () { // Notify the controller we want to make a record editable var record = self.state.data[rowIndex]; + var def = $.Deferred(); self.trigger_up('edit_line', { recordID: record.id, + onSuccess: def.resolve.bind(def), }); + return def; }); }, - /** - * Set the value of a cell. This method can be called when the value of a - * cell was modified, for example after an onchange. - * - * @param {integer} rowIndex - * @param {integer} colIndex - */ - _setCellValue: function (rowIndex, colIndex) { - var record = this.state.data[rowIndex]; - var node = this.columns[colIndex]; - - var $oldTd = this.$('.o_data_row:nth(' + rowIndex + ') > .o_data_cell:nth(' + colIndex + ')'); - var $newTd = this._renderBodyCell(record, node, colIndex, {mode: 'readonly'}); - this._unregisterModifiersElement(node, record, $oldTd); - $oldTd.replaceWith($newTd); - - // Destroy old cell field widget if any - // TODO this is very inefficient O(n^3) instead of O(n) on row update - // because this method is called for each cell (O(n)), each time we have - // to find the associated record widget (O(n)) and once found, the - // search is performed again by the _destroyFieldWidget function - if (node.tag === 'field') { - var recordWidgets = this.allFieldWidgets[record.id]; - var w = _.findWhere(recordWidgets, {name: node.attrs.name}); // - if (w) { - this._destroyFieldWidget(record, w); - } - } - }, /** * This method is called whenever we click/move outside of a row that was * in edit mode. This is the moment we save all accumulated changes on that diff --git a/addons/web/static/src/js/views/list/list_renderer.js b/addons/web/static/src/js/views/list/list_renderer.js index 3cda7e9f809..8437881a771 100644 --- a/addons/web/static/src/js/views/list/list_renderer.js +++ b/addons/web/static/src/js/views/list/list_renderer.js @@ -268,9 +268,7 @@ var ListRenderer = BasicRenderer.extend({ return $td.append(this._renderButton(record, node)); } if (node.attrs.widget || (options && options.renderWidgets)) { - this.defs = []; // TODO maybe wait for those somewhere ? var widget = this._renderFieldWidget(node, record, _.pick(options, 'mode')); - delete this.defs; return $td.append(widget.$el); } var name = node.attrs.name; @@ -534,9 +532,11 @@ var ListRenderer = BasicRenderer.extend({ */ _renderRow: function (record) { var self = this; + this.defs = []; // TODO maybe wait for those somewhere ? var $cells = _.map(this.columns, function (node, index) { return self._renderBodyCell(record, node, index, {mode: 'readonly'}); }); + delete this.defs; var className = 'o_data_row'; var decorations = this._computeDecorationClassNames(record); if (decorations.length) { diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index 764bd2017b4..79666c949c3 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -2036,6 +2036,35 @@ QUnit.module('Views', { list.destroy(); }); + + QUnit.test('discarding changes in a row properly updates the rendering', function (assert) { + assert.expect(3); + + var list = createView({ + View: ListView, + model: 'foo', + data: this.data, + arch: + '' + + '' + + '', + }); + + assert.strictEqual(list.$('.o_data_cell:first').text(), "yop", + "first cell should contain 'yop'"); + + list.$('.o_data_cell:first').click(); + list.$('input[name="foo"]').val("hello").trigger('input'); + list.$buttons.find('.o_list_button_discard').click(); + assert.strictEqual($('.modal:visible').length, 1, + "a modal to ask for discard should be visible"); + + $('.modal:visible .btn-primary').click(); + assert.strictEqual(list.$('.o_data_cell:first').text(), "yop", + "first cell should still contain 'yop'"); + + list.destroy(); + }); }); });