[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.

6 files changed, 63 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),
@@ -799,6 +807,18 @@ var FieldX2Many = AbstractField.extend({
         });
     },
     /**
+     * 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',
@@ -457,6 +458,31 @@ var BasicController = AbstractController.extend(FieldManagerMixin, {
     //--------------------------------------------------------------------------
 
     /**
+     * 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
      * in readonly (e.g. Priority).
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',
     }),
@@ -377,32 +376,6 @@ var FormController = BasicController.extend({
         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
      *
      * @private
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;
         }
     },
     /**