From 3aad756d32ece364ccbe0953d6acebdbe7581f1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Sun, 21 May 2017 09:23:27 +0200 Subject: [PATCH] [FIX] web: properly handles ESC in editable lists It is not really easy to handle discard operations. We need to handle the discard in editable lists, and in x2manys. This is not exactly the same code, but most of this was done by moving the discard method from the formcontroller to the basic controller. With this commit, the user can now discard the changes for the current record by pressing ESC. If the record is new, it will remove it, if not, it will ask for confirmation before. --- .../static/src/js/fields/abstract_field.js | 3 + .../static/src/js/fields/relational_fields.js | 22 +++++- .../src/js/views/basic/basic_controller.js | 26 +++++++ .../src/js/views/form/form_controller.js | 27 -------- .../src/js/views/list/list_controller.js | 5 +- .../js/views/list/list_editable_renderer.js | 11 ++- .../tests/fields/relational_fields_tests.js | 68 +++++++++++++++++++ addons/web/static/tests/views/list_tests.js | 22 ++++++ 8 files changed, 153 insertions(+), 31 deletions(-) diff --git a/addons/web/static/src/js/fields/abstract_field.js b/addons/web/static/src/js/fields/abstract_field.js index d6d4c27fdb3..c455710a697 100644 --- a/addons/web/static/src/js/fields/abstract_field.js +++ b/addons/web/static/src/js/fields/abstract_field.js @@ -408,6 +408,9 @@ var AbstractField = Widget.extend({ ev.stopPropagation(); this.trigger_up('navigation_move', {direction: 'next_line'}); break; + case $.ui.keyCode.ESCAPE: + this.trigger_up('navigation_move', {direction: 'cancel'}); + break; case $.ui.keyCode.UP: ev.stopPropagation(); this.trigger_up('navigation_move', {direction: 'up'}); diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js index 5dc74facf5e..acc33394676 100644 --- a/addons/web/static/src/js/fields/relational_fields.js +++ b/addons/web/static/src/js/fields/relational_fields.js @@ -165,6 +165,13 @@ var FieldMany2One = AbstractField.extend({ focus: function (event) { event.preventDefault(); // don't automatically select values on focus }, + close: function (event) { + // it is necessary to prevent ESC key from propagating to field + // root, to prevent unwanted discard operations. + if (event.which === $.ui.keyCode.ESCAPE) { + event.stopPropagation(); + } + }, autoFocus: true, html: true, minLength: 0, @@ -547,6 +554,7 @@ var FieldX2Many = AbstractField.extend({ tagName: 'div', custom_events: _.extend({}, AbstractField.prototype.custom_events, { add_record: '_onAddRecord', + discard_changes: '_onDiscardChanges', edit_line: '_onEditLine', field_changed: '_onFieldChanged', kanban_record_delete: '_onDeleteRecord', @@ -756,7 +764,7 @@ var FieldX2Many = AbstractField.extend({ this.renderer.commitChanges(recordID).then(function () { // TODO wrong as no mutex protection var fieldNames = self.renderer.canBeSaved(recordID); if (fieldNames.length) { - self.trigger_up('discard_x2m_changes', { + self.trigger_up('discard_changes', { recordID: recordID, onSuccess: def.resolve.bind(def), onFailure: def.reject.bind(def), @@ -798,6 +806,18 @@ var FieldX2Many = AbstractField.extend({ ids: [ev.data.id], }); }, + /** + * When the discard_change event go through this field, we can just decorate + * the data with the name of the field. The origin field ignore this + * information (it is a subfield in a o2m), and the controller will need to + * know which field needs to be handled. + * + * @private + * @param {OdooEvent} ev + */ + _onDiscardChanges: function (ev) { + ev.data.fieldName = this.name; + }, /** * Called when the renderer asks to edit a line, in that case simply tells * him back to toggle the mode of this row. 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 4c8363a81b1..3b24df19e3e 100644 --- a/addons/web/static/src/js/views/basic/basic_controller.js +++ b/addons/web/static/src/js/views/basic/basic_controller.js @@ -18,6 +18,7 @@ var _t = core._t; var BasicController = AbstractController.extend(FieldManagerMixin, { custom_events: _.extend({}, AbstractController.prototype.custom_events, FieldManagerMixin.custom_events, { + discard_changes: '_onDiscardChanges', reload: '_onReload', sidebar_data_asked: '_onSidebarDataAsked', translate: '_onTranslate', @@ -456,6 +457,31 @@ var BasicController = AbstractController.extend(FieldManagerMixin, { // Handlers //-------------------------------------------------------------------------- + /** + * Called when a list element asks to discard the changes made to one of + * its rows. It can happen with a x2many (if we are in a form view) or with + * a list view. + * + * @private + * @param {OdooEvent} ev + */ + _onDiscardChanges: function (ev) { + var self = this; + ev.stopPropagation(); + var recordID = ev.data.recordID; + this.discardChanges(recordID) + .done(function () { + if (self.model.isNew(recordID)) { + self._abandonRecord(recordID); + } + // TODO this will tell the renderer to rerender the widget that + // asked for the discard but will unfortunately lose the click + // made on another row if any + self._confirmChange(self.handle, [ev.data.fieldName], ev) + .always(ev.data.onSuccess); + }) + .fail(ev.data.onFailure); + }, /** * Forces to save directly the changes if the controller is in readonly, * because in that case the changes come from widgets that are editable even 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 99d21552be6..0ec959233e6 100644 --- a/addons/web/static/src/js/views/form/form_controller.js +++ b/addons/web/static/src/js/views/form/form_controller.js @@ -15,7 +15,6 @@ var FormController = BasicController.extend({ open_one2many_record: '_onOpenOne2ManyRecord', bounce_edit: '_onBounceEdit', button_clicked: '_onButtonClicked', - discard_x2m_changes: '_onDiscardX2MChanges', open_record: '_onOpenRecord', toggle_column_order: '_onToggleColumnOrder', }), @@ -376,32 +375,6 @@ var FormController = BasicController.extend({ _onDiscard: function () { this.discardChanges(); }, - /** - * Called when a x2m asks to discard the changes made to one of its row. - * - * @todo find a better way to handle this... this could also be used outside - * of form views - * - * @private - * @param {OdooEvent} ev - */ - _onDiscardX2MChanges: function (ev) { - var self = this; - ev.stopPropagation(); - var recordID = ev.data.recordID; - this.discardChanges(recordID) - .done(function () { - if (self.model.isNew(recordID)) { - self._abandonRecord(recordID); - } - // TODO this will tell the renderer to rerender the widget that - // asked for the discard but will unfortunately lose the click - // made on another row if any - self._confirmChange(self.handle, [ev.target.name], ev) - .always(ev.data.onSuccess); - }) - .fail(ev.data.onFailure); - }, /** * Called when the user clicks on 'Duplicate Record' in the sidebar * 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 21e29780d09..815ac0497a2 100644 --- a/addons/web/static/src/js/views/list/list_controller.js +++ b/addons/web/static/src/js/views/list/list_controller.js @@ -64,7 +64,10 @@ var ListController = BasicController.extend({ return $.when(); } } - return this._super(recordID); + var self = this; + return this._super(recordID).then(function () { + self._updateButtons('readonly'); + }); }, /** * Calculate the active domain of the list view. This should be done only 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 e65b8332e7a..5b4f24d04e7 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 @@ -113,13 +113,15 @@ ListRenderer.include({ */ removeLine: function (state, recordID) { var rowIndex = _.findIndex(this.state.data, {id: recordID}); + this.state = state; + if (rowIndex === -1) { + return; + } if (rowIndex === this.currentRow) { this.currentRow = null; } var $row = this.$('.o_data_row:nth(' + rowIndex + ')'); $row.remove(); - - this.state = state; }, /** * Updates the already rendered row associated to the given recordID so that @@ -574,6 +576,11 @@ ListRenderer.include({ case 'next_line': this._moveToNextLine(); break; + case 'cancel': + this.trigger_up('discard_changes', { + recordID: ev.target.dataPointID, + }); + break; } }, /** diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js index 15248b950ce..9d021d331fd 100644 --- a/addons/web/static/tests/fields/relational_fields_tests.js +++ b/addons/web/static/tests/fields/relational_fields_tests.js @@ -1814,6 +1814,74 @@ QUnit.module('relational_fields', { form.destroy(); }); + QUnit.test('editable o2m, pressing ESC discard current changes', function (assert) { + assert.expect(5); + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '
' + + '' + + '' + + '' + + '' + + '' + + '
', + res_id: 2, + mockRPC: function (route, args) { + assert.step(args.method); + return this._super.apply(this, arguments); + }, + }); + + form.$buttons.find('.o_form_button_edit').click(); + form.$('.o_field_x2many_list_row_add a').click(); + assert.strictEqual(form.$('tr.o_data_row').length, 1, + "there should be one data row"); + + form.$('input[name="turtle_foo"]').trigger({type: 'keydown', which: $.ui.keyCode.ESCAPE}); + assert.strictEqual(form.$('tr.o_data_row').length, 0, + "data row should have been discarded"); + assert.verifySteps(['read', 'default_get']); + form.destroy(); + }); + + QUnit.test('editable o2m with required field, pressing ESC discard current changes', function (assert) { + assert.expect(5); + + this.data.turtle.fields.turtle_foo.required = true; + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '
' + + '' + + '' + + '' + + '' + + '' + + '
', + res_id: 2, + mockRPC: function (route, args) { + assert.step(args.method); + return this._super.apply(this, arguments); + }, + }); + + form.$buttons.find('.o_form_button_edit').click(); + form.$('.o_field_x2many_list_row_add a').click(); + assert.strictEqual(form.$('tr.o_data_row').length, 1, + "there should be one data row"); + + form.$('input[name="turtle_foo"]').trigger({type: 'keydown', which: $.ui.keyCode.ESCAPE}); + assert.strictEqual(form.$('tr.o_data_row').length, 0, + "data row should have been discarded"); + assert.verifySteps(['read', 'default_get']); + form.destroy(); + }); + QUnit.test('onchange in a one2many', function (assert) { assert.expect(1); diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index 986f4649b64..4346e3c2b75 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -2185,6 +2185,28 @@ QUnit.module('Views', { list.destroy(); }); + + QUnit.test('pressing ESC discard the current line changes', function (assert) { + assert.expect(3); + + var list = createView({ + View: ListView, + model: 'foo', + data: this.data, + arch: '', + }); + + list.$buttons.find('.o_list_button_add').click(); + + list.$('input[name="foo"]').trigger({type: 'keydown', which: $.ui.keyCode.ESCAPE}); + assert.strictEqual(list.$('tr.o_data_row').length, 4, + "should have 4 data row in list"); + assert.strictEqual(list.$('tr.o_data_row.o_selected_row').length, 0, + "no rows should be selected"); + assert.ok(!list.$buttons.find('.o_list_button_save').is(':visible'), + "should not have a visible save button"); + list.destroy(); + }); }); });