[FIX] web: navigate in editable list with custom controls

Rev. f241441 recently added the possibiliy to define custom
controls (instead of the single  'Add a line') in editable list
views. However, it didn't take into account keyboard navigation.

Suppose that the focus is on the field preceeding the list, and
the user clicks on TAB, the code assumed that there was only one
control, and focused it. However, when there were several control,
the last one was focused (which is typically the less common one),
and there was no way to navigate to the others.

This rev. focuses the first control by default, and allows to
navigate through controls with RIGHT and LEFT. Pressing TAB still
moves out of the list, to the next field.

Note that i'm not happy with this fix, as the logic doesn't seem
to be in the right place. Indeed, the notion of controls is the
concern of the editable list, not the x2many field, so the list
should handle this logic by itself. Unfortenately, this is not that
easy to change, as for now the list doesn't even know that we are
navigating to/from its control(s), until we click on it/them.
Changing that will be for later, in master.

We also did a linting pass on the concerned code.

Task 1878254

4 files changed, 107 insertions, 13 deletions

diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js
index cf217cf70bc..4c92c0973b9 100644
--- a/addons/web/static/src/js/fields/relational_fields.js
+++ b/addons/web/static/src/js/fields/relational_fields.js
@@ -402,7 +402,7 @@ var FieldMany2One = AbstractField.extend({
         var domain = this.record.getDomain(this.recordParams);
 
         // Add the additionalContext
-        _.extend(context, this.additionalContext)
+        _.extend(context, this.additionalContext);
 
         var blacklisted_ids = this._getSearchBlacklist();
         if (blacklisted_ids.length > 0) {
@@ -848,9 +848,8 @@ var FieldX2Many = AbstractField.extend({
 
     /**
      * @override
-     * @param {Object} [options]
      */
-    activate: function (options) {
+    activate: function () {
         if (!this.activeActions.create || this.isReadonly || !this.$el.is(":visible")) {
             return false;
         }
@@ -858,7 +857,7 @@ var FieldX2Many = AbstractField.extend({
             this.$buttons.find(".o-kanban-button-new").focus();
         }
         if (this.view.arch.tag === 'tree') {
-            this.renderer.$('.o_field_x2many_list_row_add a').focus();
+            this.renderer.$('.o_field_x2many_list_row_add a:first').focus();
         }
         return true;
     },
@@ -1166,6 +1165,31 @@ var FieldX2Many = AbstractField.extend({
         }
     },
     /**
+     * Override to handle the navigation inside editable list controls
+     *
+     * @override
+     * @private
+     */
+    _onNavigationMove: function (ev) {
+        if (this.view.arch.tag === 'tree') {
+            var $curControl = this.renderer.$('.o_field_x2many_list_row_add a:focus');
+            if ($curControl.length) {
+                var $nextControl;
+                if (ev.data.direction === 'right') {
+                    $nextControl = $curControl.next('a');
+                } else if (ev.data.direction === 'left') {
+                    $nextControl = $curControl.prev('a');
+                }
+                if ($nextControl && $nextControl.length) {
+                    ev.stopPropagation();
+                    $nextControl.focus();
+                    return;
+                }
+            }
+        }
+        this._super.apply(this, arguments);
+    },
+    /**
      * Called when the user clicks on a relational record.
      *
      * @abstract
@@ -1660,7 +1684,7 @@ var FieldMany2ManyBinaryMultiFiles = AbstractField.extend({
         var attachment_ids = this.value.res_ids;
 
         // Don't create an attachment if the upload window is cancelled.
-        if(files.length == 0)
+        if(files.length === 0)
             return;
 
         _.each(files, function (file) {
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 771b5b047dc..0951b3f0cc6 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
@@ -15,7 +15,6 @@ var core = require('web.core');
 var dom = require('web.dom');
 var ListRenderer = require('web.ListRenderer');
 var utils = require('web.utils');
-var Context = require('web.Context');
 
 var _t = core._t;
 
@@ -38,7 +37,7 @@ ListRenderer.include({
      * @param {boolean} params.addTrashIcon
      */
     init: function (parent, state, params) {
-        var self = this
+        var self = this;
         this._super.apply(this, arguments);
 
         // if addCreateLine is true, the renderer will add a 'Add a line' link
@@ -67,8 +66,8 @@ ListRenderer.include({
                     }
 
                     self.creates.push({
-                        'context': child.attrs.context,
-                        'string': child.attrs.string,
+                        context: child.attrs.context,
+                        string: child.attrs.string,
                     });
                 });
             });
@@ -270,7 +269,6 @@ ListRenderer.include({
      * @param {string} recordID
      */
     removeLine: function (state, recordID) {
-        var self = this;
         var rowIndex = _.findIndex(this.state.data, {id: recordID});
         this.state = state;
         if (rowIndex === -1) {
diff --git a/addons/web/static/src/scss/list_view.scss b/addons/web/static/src/scss/list_view.scss
index 7332cec6d45..39e14c9f8c4 100644
--- a/addons/web/static/src/scss/list_view.scss
+++ b/addons/web/static/src/scss/list_view.scss
@@ -112,6 +112,12 @@
         border-top: none;  // Override bootstrap for grouped list views
     }
 
+    tbody {
+        tr:focus-within {
+            background-color: $o-form-lightsecondary;
+        }
+    }
+
     // Values styling
     .o_list_number {
         text-align: right;
diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js
index 3a6423f9cdf..2fff96e8ee3 100644
--- a/addons/web/static/tests/fields/relational_fields_tests.js
+++ b/addons/web/static/tests/fields/relational_fields_tests.js
@@ -4,7 +4,6 @@ odoo.define('web.relational_fields_tests', function (require) {
 var AbstractField = require('web.AbstractField');
 var BasicModel = require('web.BasicModel');
 var concurrency = require('web.concurrency');
-var config = require('web.config');
 var FormView = require('web.FormView');
 var ListRenderer = require('web.ListRenderer');
 var ListView = require('web.ListView');
@@ -13354,8 +13353,8 @@ QUnit.module('relational_fields', {
         form.destroy();
     });
 
-
     QUnit.module('TabNavigation');
+
     QUnit.test('when Navigating to a many to one with tabs, it receives the focus on the add new line link', function (assert) {
          assert.expect(3);
 
@@ -13442,7 +13441,7 @@ QUnit.module('relational_fields', {
                            "after tab, the focus should be on the many2one"));
 
        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) {
         assert.expect(3);
@@ -13567,6 +13566,73 @@ QUnit.module('relational_fields', {
         form.destroy();
     });
 
+    QUnit.test('navigating through an editable list with custom controls [REQUIRE FOCUS]', function (assert) {
+        assert.expect(5);
+
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch:
+                '<form>' +
+                    '<field name="display_name"/>' +
+                    '<field name="p">' +
+                        '<tree editable="bottom">' +
+                            '<control>' +
+                                '<create string="Custom 1" context="{\'default_foo\': \'1\'}"/>' +
+                                '<create string="Custom 2" context="{\'default_foo\': \'2\'}"/>' +
+                            '</control>' +
+                            '<field name="foo"/>' +
+                        '</tree>' +
+                    '</field>' +
+                    '<field name="int_field"/>' +
+                '</form>',
+            viewOptions: {
+                mode: 'edit',
+            },
+        });
+
+        assert.strictEqual(document.activeElement, form.$('.o_field_widget[name="display_name"]')[0],
+            "first input should be focused by default");
+
+        // press tab to navigate to the list
+        form.$('.o_field_widget[name="display_name"]').trigger($.Event('keydown', {
+            which: $.ui.keyCode.TAB,
+            keyCode: $.ui.keyCode.TAB,
+        }));
+
+        assert.strictEqual(document.activeElement, form.$('.o_field_x2many_list_row_add a:first')[0],
+            "first editable list control should now have the focus");
+
+        // press right to focus the second control
+        form.$('.o_field_x2many_list_row_add a:first').trigger($.Event('keydown', {
+            which: $.ui.keyCode.RIGHT,
+            keyCode: $.ui.keyCode.RIGHT,
+        }));
+
+        assert.strictEqual(document.activeElement, form.$('.o_field_x2many_list_row_add a:nth(1)')[0],
+            "second editable list control should now have the focus");
+
+        // press left to come back to first control
+        form.$('.o_field_x2many_list_row_add a:nth(1)').trigger($.Event('keydown', {
+            which: $.ui.keyCode.LEFT,
+            keyCode: $.ui.keyCode.LEFT,
+        }));
+
+        assert.strictEqual(document.activeElement, form.$('.o_field_x2many_list_row_add a:first')[0],
+            "first editable list control should now have the focus");
+
+        // press tab to leave the list
+        form.$('.o_field_x2many_list_row_add a:first').trigger($.Event('keydown', {
+            which: $.ui.keyCode.TAB,
+            keyCode: $.ui.keyCode.TAB,
+        }));
+
+        assert.strictEqual(document.activeElement, form.$('.o_field_widget[name="int_field"]')[0],
+            "last input should now be focused");
+
+        form.destroy();
+    });
 });
 });
 });