[IMP] web: x2many adds a new line by tabbing on a not dirty line

Before this change, when the focus was on the first field on a x2many
field, and the user pressed TAB, the line would be discarded and the
focus moved to the next field (at the level of the x2many) in the form.

This was an issue because if the first field was required, the focus
could not be moved out of the field with TAB so the user had to discard
the line manually (with ESC) before moving to the next field

This change works with 2 changes:
1. detecting if a line is dirty (has been modified by the user) and at
the end of the line, decide to either
    - discard the line and move the next field, if the line is not dirty
    - commit the line and add a new line, if the line is dirty

2. allow users to use TAB through required field inside x2many so they
can easilly go to the end of the line.

Task id : 1872248

addons/web/static/src/js/fields/basic_fields.js

@@ -334,7 +334,8 @@ var InputField = DebouncedField.extend({
             }
             if (ev.data.direction ==='next' &&
                 this.attrs.modifiersValue &&
-                this.attrs.modifiersValue.required) {
+                this.attrs.modifiersValue.required &&
+                this.viewType !== 'list') {
                 if (!this.$input.val()){
                     this.setInvalidClass();
                     ev.stopPropagation();
@@ -388,7 +389,7 @@ var NumericField = InputField.extend({
      *
      * Note: We have to overwrite this method to set the input's type to number if
      * option setted into the field.
-     * 
+     *
      * @override
      * @private
      */

addons/web/static/src/js/views/basic/basic_model.js

@@ -594,6 +594,7 @@ var BasicModel = AbstractModel.extend({
                 getDomain: element.getDomain,
                 getFieldNames: element.getFieldNames,
                 id: element.id,
+                isDirty: element.isDirty,
                 limit: element.limit,
                 model: element.model,
                 offset: element.offset,
@@ -648,6 +649,7 @@ var BasicModel = AbstractModel.extend({
             getFieldNames: element.getFieldNames,
             groupedBy: element.groupedBy,
             id: element.id,
+            isDirty: element.isDirty,
             isOpen: element.isOpen,
             limit: element.limit,
             model: element.model,
@@ -3490,6 +3492,7 @@ var BasicModel = AbstractModel.extend({
         dataPoint.getContext = this._getContext.bind(this, dataPoint);
         dataPoint.getDomain = this._getDomain.bind(this, dataPoint);
         dataPoint.getFieldNames = this._getFieldNames.bind(this, dataPoint);
+        dataPoint.isDirty = this.isDirty.bind(this, dataPoint.id);
 
         this.localData[dataPoint.id] = dataPoint;
 

addons/web/static/src/js/views/list/list_controller.js

@@ -27,6 +27,7 @@ var ListController = BasicController.extend({
         selection_changed: '_onSelectionChanged',
         toggle_column_order: '_onToggleColumnOrder',
         toggle_group: '_onToggleGroup',
+        navigation_move: '_onNavigationMove',
     }),
     /**
      * @constructor

addons/web/static/src/js/views/list/list_editable_renderer.js

@@ -813,6 +813,10 @@ ListRenderer.include({
     _onFooterClick: function () {
         this.unselectRow();
     },
+   /**
+    * @param {KeyDownEvent} e
+    * @private
+    */
     _onKeyDownAddRecord: function(e) {
         switch(e.keyCode) {
             case $.ui.keyCode.ENTER:
@@ -823,25 +827,25 @@ ListRenderer.include({
         }
     },
     /**
-     * It will returns the first visible widget that is editable
+     * It will returns the last visible widget that is editable
      *
      * @private
-     * @returns {Class} Widget returns first widget
+     * @returns {Class} Widget returns last widget
      */
-    _getFirstWidget: function () {
+    _getLastWidget: function () {
         var record = this.state.data[this.currentRow];
         var recordWidgets = this.allFieldWidgets[record.id];
-        var firstWidget = _.find(recordWidgets, function (widget) {
-            var isFirst =
+        var lastWidget = _.chain(recordWidgets).filter(function (widget) {
+            var isLast =
                 widget.$el.is(':visible') &&
                 (
                     widget.$('input').length > 0 || widget.tagName === 'input' ||
                     widget.$('textarea').length > 0 || widget.tagName === 'textarea'
                 ) &&
                 !widget.$el.hasClass('o_readonly_modifier');
-            return isFirst;
-        });
-        return firstWidget;
+            return isLast;
+        }).last().value();
+        return lastWidget;
     },
 
     /**
@@ -872,11 +876,22 @@ ListRenderer.include({
                 break;
             case 'next':
                 // When navigating with the keyboard, we want to get out of the list editable if the
-                // first field is left empty.
+                // entire line is left unmodified and we are on the next line.
                 var column = this.columns[this.currentFieldIndex];
-                var firstWidget = this._getFirstWidget();
-                if (column.attrs.name === firstWidget.name && !firstWidget.$input.val()) {
-                    this.trigger_up('activate_next_widget');
+                var lastWidget = this._getLastWidget();
+                if (column.attrs.name === lastWidget.name) {
+                    if (this.currentRow + 1 < this.state.data.length) {
+                        this._selectCell(this.currentRow+1, 0, {wrap:false})
+                            .fail(this._moveToNextLine.bind(this));
+                    } else {
+                        var currentRowData = this.state.data[this.currentRow];
+                        if (currentRowData.isDirty(currentRowData.id)) {
+                            this._moveToNextLine();
+                        }
+                        else {
+                            this.trigger_up('activate_next_widget');
+                        }
+                    }
                 } else {
                     if (this.currentFieldIndex + 1 < this.columns.length) {
                         this._selectCell(this.currentRow, this.currentFieldIndex + 1, {wrap: false})

addons/web/static/tests/fields/relational_fields_tests.js

@@ -13493,7 +13493,8 @@ QUnit.module('relational_fields', {
        form.destroy();
     });
 
-    QUnit.test('when Navigating to a many to one with tabs, not filling in the first field and hitting tab, we should not add a first line but navigate to the next control', function (assert) {
+    QUnit.test('when Navigating to a many2one with tabs, not filling any field and hitting tab,' +
+            ' we should not add a first line but navigate to the next control', function (assert) {
         assert.expect(3);
 
         this.data.partner.records[0].turtles = [];
@@ -13536,6 +13537,10 @@ QUnit.module('relational_fields', {
             keyCode: $.ui.keyCode.TAB,
         }));
 
+        // skips the first field of the one2many
+        $(document.activeElement).trigger($.Event('keydown', {which: $.ui.keyCode.TAB}));
+
+        // skips the second (and last) field of the one2many
         $(document.activeElement).trigger($.Event('keydown', {which: $.ui.keyCode.TAB}));
 
         assert.strictEqual(assert.strictEqual(form.$el.find('input[name="foo"]')[0],

addons/web/static/tests/views/list_tests.js

@@ -2352,7 +2352,7 @@ QUnit.module('Views', {
     });
 
     QUnit.test('pressing tab on last cell of editable list view', function (assert) {
-        assert.expect(7);
+        assert.expect(9);
 
         var list = createView({
             View: ListView,
@@ -2369,6 +2369,10 @@ QUnit.module('Views', {
         assert.strictEqual(document.activeElement.name, "foo",
             "focus should be on an input with name = foo");
 
+        //it will not create a new line unless a modification is made
+        document.activeElement.value = "blip-changed";
+        $(document.activeElement).trigger({type: 'change'});
+
         list.$('tr.o_selected_row input[name="foo"]').trigger({type: 'keydown', which: 9}); // tab
         assert.strictEqual(document.activeElement.name, "int_field",
             "focus should be on an input with name = int_field");
@@ -2380,7 +2384,10 @@ QUnit.module('Views', {
         assert.strictEqual(document.activeElement.name, "foo",
             "focus should be on an input with name = foo");
 
-        assert.verifySteps(['/web/dataset/search_read', '/web/dataset/call_kw/foo/default_get']);
+        assert.verifySteps(['/web/dataset/search_read',
+            '/web/dataset/call_kw/foo/write',
+            '/web/dataset/call_kw/foo/read',
+            '/web/dataset/call_kw/foo/default_get']);
         list.destroy();
     });
 
@@ -2499,7 +2506,7 @@ QUnit.module('Views', {
         list.destroy();
     });
 
-    QUnit.test('navigation with tab and readonly field', function (assert) {
+    QUnit.test('navigation with tab and readonly field (no modification)', function (assert) {
         // This test makes sure that if we have 2 cells in a row, the first in
         // edit mode, and the second one readonly, then if we press TAB when the
         // focus is on the first, then the focus skip the readonly cells and
@@ -2515,6 +2522,43 @@ QUnit.module('Views', {
 
         // click on first td and press TAB
         list.$('td:contains(yop)').last().click();
+
+        list.$('tr.o_selected_row input[name="foo"]').trigger({type: 'keydown', which: $.ui.keyCode.TAB});
+
+        assert.ok(list.$('tr.o_data_row:eq(1)').hasClass('o_selected_row'),
+            "2nd row should be selected");
+
+        // we do it again. This was broken because the this.currentRow variable
+        // was not properly set, and the second TAB could cause a crash.
+        list.$('tr.o_selected_row input[name="foo"]').trigger({type: 'keydown', which: $.ui.keyCode.TAB});
+        assert.ok(list.$('tr.o_data_row:eq(2)').hasClass('o_selected_row'),
+            "3rd row should be selected");
+
+        list.destroy();
+    });
+
+
+    QUnit.test('navigation with tab and readonly field (with modification)', function (assert) {
+        // This test makes sure that if we have 2 cells in a row, the first in
+        // edit mode, and the second one readonly, then if we press TAB when the
+        // focus is on the first, then the focus skips the readonly cells and
+        // directly goes to the next line instead.
+        assert.expect(2);
+
+        var list = createView({
+            View: ListView,
+            model: 'foo',
+            data: this.data,
+            arch: '<tree editable="bottom"><field name="foo"/><field name="int_field" readonly="1"/></tree>',
+        });
+
+        // click on first td and press TAB
+        list.$('td:contains(yop)').last().click();
+
+        //modity the cell content
+        document.activeElement.value = "blip-changed";
+        $(document.activeElement).trigger({type: 'change'});
+
         list.$('tr.o_selected_row input[name="foo"]').trigger({type: 'keydown', which: $.ui.keyCode.TAB});
 
         assert.ok(list.$('tr.o_data_row:eq(1)').hasClass('o_selected_row'),
@@ -2578,6 +2622,7 @@ QUnit.module('Views', {
                             '<field name="display_name"/>' +
                         '</tree>' +
                     '</field>' +
+                    '<field name="foo"/>' +
                 '</sheet></form>',
             res_id: 1,
             viewOptions: {
@@ -2597,10 +2642,10 @@ QUnit.module('Views', {
         assert.ok(form.$('.o_field_widget[name=o2m] .o_data_row:nth(1)').hasClass('o_selected_row'),
             "second row should be in edition");
 
-        // Press 'Tab' -> should go back to first line as the create action isn't available
+        // Press 'Tab' -> should get out of the one to many and go to the next field of the form
         form.$('.o_field_widget[name=o2m] .o_selected_row input').trigger({type: 'keydown', which: 9});
-        assert.ok(form.$('.o_field_widget[name=o2m] .o_data_row:first').hasClass('o_selected_row'),
-            "first row should be in edition");
+        assert.strictEqual(document.activeElement, form.$('input[name="foo"]')[0],
+            "the next field should be selected");
 
         form.destroy();
     });
@@ -2623,12 +2668,12 @@ QUnit.module('Views', {
                 }
                 return this._super.apply(this, arguments);
             },
-            fieldDebounce: 1
+            fieldDebounce: 1,
         });
 
         // click on first td and press TAB
         list.$('td:contains(yop)').click();
-        list.$('tr.o_selected_row input[name="foo"]').val('new value').trigger('input');
+        list.$('tr.o_selected_row input[name="foo"]').val('new value').trigger('change');
         list.$('tr.o_selected_row input[name="foo"]').trigger({type: 'keydown', which: $.ui.keyCode.TAB});
 
         assert.strictEqual(list.$('tbody tr:first td:contains(new value)').length, 1,