From 9348dfd70d57077ebe56da587a4dcba0b0ef0485 Mon Sep 17 00:00:00 2001
From: std-odoo <std@odoo.com>
Date: Thu, 1 Jun 2023 06:44:55 +0000
Subject: [PATCH] [IMP] web: properties selection, insert after the focused
 option

Purpose
=======
Currently, when we want to add a properties selection option, we need
to focus another option and press enter. This will add the new option
at the end, but we want to add it just after the focused option.

Task-3338157

Part-of: odoo/odoo#123216
---
 .../property_definition_selection.js          | 30 +++++++++++++------
 .../property_definition_selection.xml         |  2 +-
 .../views/fields/properties_field_tests.js    | 25 ++++++++++++++--
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/addons/web/static/src/views/fields/properties/property_definition_selection.js b/addons/web/static/src/views/fields/properties/property_definition_selection.js
index ea4f0ce09f2..ad35eeebebb 100644
--- a/addons/web/static/src/views/fields/properties/property_definition_selection.js
+++ b/addons/web/static/src/views/fields/properties/property_definition_selection.js
@@ -12,18 +12,23 @@ export class PropertyDefinitionSelection extends Component {
         // when we create a new option, it's added in the state
         // when we have finished to edit it (blur / enter) we propagate
         // the new value in the props
-        this.state = useState({ newOption: null });
+        this.state = useState({
+            newOption: null,
+        });
 
         this.propertyDefinitionSelectionRef = useRef("propertyDefinitionSelection");
         this.addButtonRef = useRef("addButton");
 
         useEffect(() => {
             // automatically give the focus to the new option if it is empty
+            if (!this.state.newOption) {
+                return;
+            }
             const inputs = this.propertyDefinitionSelectionRef.el.querySelectorAll(
                 ".o_field_property_selection_option input"
             );
-            if (inputs && inputs.length && !inputs[inputs.length - 1].value) {
-                inputs[inputs.length - 1].focus();
+            if (inputs && inputs.length && !inputs[this.state.newOption.index].value) {
+                inputs[this.state.newOption.index].focus();
             }
         });
     }
@@ -51,7 +56,11 @@ export class PropertyDefinitionSelection extends Component {
      */
     get optionsVisible() {
         const options = this.options || [];
-        return this.state.newOption ? [...options, this.state.newOption] : options;
+        const newOption = this.state.newOption;
+        if (newOption) {
+            options.splice(newOption.index, 0, [newOption.name, ""]);
+        }
+        return options;
     }
 
     /* --------------------------------------------------------
@@ -61,8 +70,11 @@ export class PropertyDefinitionSelection extends Component {
     /**
      * Add a new empty selection option.
      */
-    onOptionCreate() {
-        this.state.newOption = [uuid(), ""];
+    onOptionCreate(index) {
+        this.state.newOption = {
+            index: index,
+            name: uuid(),
+        };
     }
 
     /**
@@ -93,7 +105,7 @@ export class PropertyDefinitionSelection extends Component {
         const nonEmptyOptions = options.filter((option) => option[1] && option[1].length);
         this.props.onOptionsChange(nonEmptyOptions);
 
-        if (this.state.newOption && this.state.newOption[1] && this.state.newOption[1].length) {
+        if (this.state.newOption) {
             // the new option has been propagated in the props
             this.state.newOption = null;
         }
@@ -117,7 +129,7 @@ export class PropertyDefinitionSelection extends Component {
             // if the value is empty, just ignore and cancel the event
             event.stopPropagation();
             event.preventDefault();
-        } else if (optionIndex >= this.options.length) {
+        } else if (optionIndex === this.state.newOption?.index) {
             // we remove the focus from the new empty option, remove it
             this.state.newOption = null;
         }
@@ -144,7 +156,7 @@ export class PropertyDefinitionSelection extends Component {
             }
 
             this.onOptionChange(event, optionIndex);
-            this.onOptionCreate();
+            this.onOptionCreate(optionIndex + 1);
         } else if (["ArrowUp", "ArrowDown"].includes(event.key)) {
             event.stopPropagation();
             event.preventDefault();
diff --git a/addons/web/static/src/views/fields/properties/property_definition_selection.xml b/addons/web/static/src/views/fields/properties/property_definition_selection.xml
index 21eb781625f..b05a9b06070 100644
--- a/addons/web/static/src/views/fields/properties/property_definition_selection.xml
+++ b/addons/web/static/src/views/fields/properties/property_definition_selection.xml
@@ -35,7 +35,7 @@
                 <button
                     t-if="!props.readonly"
                     class="btn btn-link btn-sm"
-                    t-on-click="onOptionCreate"
+                    t-on-click="() => this.onOptionCreate(this.options.length)"
                     t-ref="addButton">
                     <i class="fa fa-plus"/>
                     Add a Value
diff --git a/addons/web/static/tests/views/fields/properties_field_tests.js b/addons/web/static/tests/views/fields/properties_field_tests.js
index 6bd37b8f57d..f75657b2aa3 100644
--- a/addons/web/static/tests/views/fields/properties_field_tests.js
+++ b/addons/web/static/tests/views/fields/properties_field_tests.js
@@ -561,12 +561,33 @@ QUnit.module("Fields", (hooks) => {
         await click(target, ".o_field_property_selection_option:nth-child(2) .fa-trash-o");
         options = popover.querySelectorAll(".o_field_property_selection_option");
         assert.strictEqual(options.length, 3, "Should have removed the second option");
-        const optionValues = [...options].map((option) => option.querySelector("input").value);
         assert.deepEqual(
-            optionValues,
+            [...options].map((option) => option.querySelector("input").value),
             ["A", "C", "New option"],
             "Should have removed the second option"
         );
+
+        // focus should be in option C
+        assert.strictEqual(document.activeElement, options[1].querySelector("input"));
+        // test that pressing 'Enter' inserts a new option after the one currently focused (and not last).
+        await triggerEvent(
+            target,
+            ".o_field_property_selection_option:nth-child(2) input",
+            "keydown",
+            { key: "Enter" }
+        );
+        await editInput(
+            target,
+            ".o_field_property_selection_option:nth-child(3) input",
+            "New option 2"
+        );
+        options = popover.querySelectorAll(".o_field_property_selection_option");
+        assert.strictEqual(options.length, 4, "Should have added a new option");
+        assert.deepEqual(
+            [...options].map((option) => option.querySelector("input").value),
+            ["A", "C", "New option 2", "New option"],
+            "Should have added a new option at the correct spot"
+        );
     });
 
     /**