[FIX] web: field color: adapted for views

The FieldColor widget had been designed exclusively for the document
layout configurator. Because of this, the widget would not display/behave
correctly in other views.

Before this commit:
- in list views, the widget had a "white-space" attribute adapted for char fields,
which gave it much more space than it needed
- there was no handling of the widget in readonly state: it was always editable
- its size in list views was a bit too large

Now:
- all styling problems have been adjusted for list views (unchanged for forms)
- the widget is disabled when in readonly mode

Task 2072466

closes odoo/odoo#36949

Signed-off-by: VincentSchippefilt <VincentSchippefilt@users.noreply.github.com>

5 files changed, 96 insertions, 14 deletions

diff --git a/addons/web/static/src/js/fields/basic_fields.js b/addons/web/static/src/js/fields/basic_fields.js
index a34d024b8e6..69be202a33d 100644
--- a/addons/web/static/src/js/fields/basic_fields.js
+++ b/addons/web/static/src/js/fields/basic_fields.js
@@ -3271,11 +3271,22 @@ var AceEditor = DebouncedField.extend({
  */
 var FieldColor = AbstractField.extend({
     template: 'FieldColor',
-    events: {
+    events: _.extend({}, AbstractField.prototype.events, {
         'click .o_field_color': '_onColorClick',
-    },
-    custom_events: {
+    }),
+    custom_events: _.extend({}, AbstractField.prototype.custom_events, {
         'colorpicker:saved': '_onColorpickerSaved',
+    }),
+
+    //--------------------------------------------------------------------------
+    // Public
+    //--------------------------------------------------------------------------
+
+    /**
+     * @override
+     */
+    getFocusableElement: function () {
+        return this.$('.o_field_color');
     },
 
     //--------------------------------------------------------------------------
@@ -3287,10 +3298,10 @@ var FieldColor = AbstractField.extend({
     * @private
     */
     _render: function () {
-        this._super.apply(this, arguments);
         this.$('.o_field_color').data('value', this.value)
             .css('background-color', this.value)
             .attr('title', this.value);
+        return this._super.apply(this, arguments);
     },
 
     //--------------------------------------------------------------------------
@@ -3299,13 +3310,20 @@ var FieldColor = AbstractField.extend({
 
     /**
     * @private
-    * @param {MouseEvent} ev
     */
-    _onColorClick: function (ev) {
-        new ColorpickerDialog(this, {
-            defaultColor: this.value,
-            noTransparency: true,
-        }).open();
+    _onColorClick: function () {
+        if (this.mode === 'edit') {
+            const dialog = new ColorpickerDialog(this, {
+                defaultColor: this.value,
+                noTransparency: true,
+            }).open();
+            dialog.on('closed', this, () => {
+                // we need to wait for the modal to execute its whole close function.
+                Promise.resolve().then(() => {
+                    this.getFocusableElement().focus();
+                });
+            });
+        }
     },
 
     /**
@@ -3315,6 +3333,20 @@ var FieldColor = AbstractField.extend({
     _onColorpickerSaved: function (ev) {
         this._setValue(ev.data.hex);
     },
+
+    /**
+     * @override
+     * @private
+     */
+    _onKeydown: function (ev) {
+        if (ev.which === $.ui.keyCode.ENTER) {
+            ev.preventDefault();
+            ev.stopPropagation();
+            this._onColorClick(ev);
+        } else {
+            this._super.apply(this, arguments);
+        }
+    },
 });
 
 return {
diff --git a/addons/web/static/src/scss/list_view.scss b/addons/web/static/src/scss/list_view.scss
index 0e62f071d42..6c3d452fb47 100644
--- a/addons/web/static/src/scss/list_view.scss
+++ b/addons/web/static/src/scss/list_view.scss
@@ -34,7 +34,7 @@
             &.o_list_number {
                 text-align: right;
             }
-            &.o_list_char,
+            &.o_list_char:not(.o_color_cell),
             &.o_list_text,
             &.o_list_many2one {
                 white-space: pre-wrap;
@@ -42,6 +42,10 @@
             &.o_list_text {
                 word-break: break-word;
             }
+            &.o_list_char.o_color_cell .o_field_color {
+                width: 17px;
+                height: 17px;
+            }
         }
 
         tfoot {
diff --git a/addons/web/static/src/scss/list_view_extra.scss b/addons/web/static/src/scss/list_view_extra.scss
index 9a16fb69347..2cef58a57a7 100644
--- a/addons/web/static/src/scss/list_view_extra.scss
+++ b/addons/web/static/src/scss/list_view_extra.scss
@@ -12,7 +12,7 @@
         }
     }
 
-    .o_list_table.o_editable_list .o_data_row.o_selected_row > .o_data_cell:not(.o_readonly_modifier) {
+    .o_list_table.o_editable_list .o_data_row.o_selected_row > .o_data_cell:not(.o_readonly_modifier):not(.o_invisible_modifier) {
         padding: .15rem;
         .o_input {
             border: none;
@@ -32,5 +32,11 @@
         &.o_invalid_cell .o_input {
             box-shadow: 0 0 0 1px theme-color('danger');
         }
+        &.o_list_button {
+            padding: .25rem;
+        }
+        &.o_color_cell > .o_field_widget {
+            padding: .15rem;
+        }
     }
 }
diff --git a/addons/web/static/src/xml/base.xml b/addons/web/static/src/xml/base.xml
index f52818d5ed6..ff0d23a928f 100644
--- a/addons/web/static/src/xml/base.xml
+++ b/addons/web/static/src/xml/base.xml
@@ -1540,7 +1540,7 @@
 
 <t t-name="FieldColor">
     <div>
-        <button class="o_field_color"></button>
+        <button class="o_field_color" t-attf-tabindex="#{widget.mode === 'edit' ? 0 : -1}"></button>
     </div>
 </t>
 
diff --git a/addons/web/static/tests/fields/basic_fields_tests.js b/addons/web/static/tests/fields/basic_fields_tests.js
index 63ce02be614..1d91f581317 100644
--- a/addons/web/static/tests/fields/basic_fields_tests.js
+++ b/addons/web/static/tests/fields/basic_fields_tests.js
@@ -6442,7 +6442,7 @@ QUnit.module('basic_fields', {
     });
 
     QUnit.test('Field Color: default widget state', async function (assert) {
-        assert.expect(3);
+        assert.expect(4);
 
         var form = await createView({
             View: FormView,
@@ -6467,6 +6467,46 @@ QUnit.module('basic_fields', {
 
         await testUtils.dom.click($('.modal .btn:contains("Discard")'));
 
+        assert.strictEqual(document.activeElement, form.$('.o_field_color')[0],
+            "Focus should go back to the color field");
+
+        form.destroy();
+    });
+
+    QUnit.test('Field Color: behaviour in different views', async function (assert) {
+        assert.expect(2);
+
+        this.data.partner.records[0].p = [4, 2];
+        this.data.partner.records[1].hex_color = '#ff0080';
+
+        const form = await createView({
+            arch: '<form>' +
+                    '<field name="hex_color" widget="color"/>' +
+                    '<field name="p">' +
+                        '<tree editable="top">' +
+                            '<field name="display_name"/>' +
+                            '<field name="hex_color" widget="color"/>' +
+                        '</tree>' +
+                    '</field>' +
+                '</form>',
+            data: this.data,
+            model: 'partner',
+            res_id: 1,
+            View: FormView,
+        });
+
+        await testUtils.dom.click(form.$('.o_field_color:first()'));
+        assert.containsNone($(document.body), '.modal',
+            "Color field in readonly shouldn't be editable");
+
+        const rowInitialHeight = form.$('.o_data_row:first()').height();
+
+        await testUtils.form.clickEdit(form);
+        await testUtils.dom.click(form.$('.o_data_row:first() .o_data_cell:first()'));
+
+        assert.strictEqual(rowInitialHeight, form.$('.o_data_row:first()').height(),
+            "Color field shouldn't change the color height when edited");
+
         form.destroy();
     });