refactor(autosave): improuve events handling

app/javascript/controllers/autosave_controller.ts

@@ -1,4 +1,13 @@
-import { httpRequest, ResponseError, getConfig } from '@utils';
+import {
+  httpRequest,
+  ResponseError,
+  isSelectElement,
+  isCheckboxOrRadioInputElement,
+  isTextInputElement,
+  show,
+  hide,
+  getConfig
+} from '@utils';
 
 import { ApplicationController } from './application_controller';
 import { AutoUpload } from '../shared/activestorage/auto-upload';
@@ -34,8 +43,8 @@ export class AutosaveController extends ApplicationController {
   connect() {
     this.#latestPromise = Promise.resolve();
     this.onGlobal('autosave:retry', () => this.didRequestRetry());
-    this.on('change', (event) => this.onInputChange(event));
-    this.on('input', (event) => this.onInputChange(event));
+    this.on('change', (event) => this.onChange(event));
+    this.on('input', (event) => this.onInput(event));
   }
 
   disconnect() {
@@ -60,18 +69,35 @@ export class AutosaveController extends ApplicationController {
     }
   }
 
-  private onInputChange(event: Event) {
+  private onChange(event: Event) {
     const target = event.target as HTMLInputElement;
-    if (target.disabled) {
-      return;
-    }
-    if (
-      target.type == 'file' &&
-      target.dataset.autoAttachUrl &&
-      target.files?.length
-    ) {
+    if (!target.disabled) {
+      if (target.type == 'file') {
+        if (target.dataset.autoAttachUrl && target.files?.length) {
           this.enqueueAutouploadRequest(target, target.files[0]);
-    } else if (target.type != 'file') {
+        }
+      } else if (target.type == 'hidden') {
+        // In React comboboxes we dispatch a "change" event on hidden inputs to trigger autosave.
+        // We want to debounce them.
+        this.debounce(this.enqueueAutosaveRequest, AUTOSAVE_DEBOUNCE_DELAY);
+      } else if (
+        isSelectElement(target) ||
+        isCheckboxOrRadioInputElement(target)
+      ) {
+        this.toggleOtherInput(target);
+        this.enqueueAutosaveRequest();
+      }
+    }
+  }
+
+  private onInput(event: Event) {
+    const target = event.target as HTMLInputElement;
+    if (
+      !target.disabled &&
+      // Ignore input from React comboboxes. We trigger "change" events on them when selection is changed.
+      target.getAttribute('role') != 'combobox' &&
+      isTextInputElement(target)
+    ) {
       this.debounce(this.enqueueAutosaveRequest, AUTOSAVE_DEBOUNCE_DELAY);
     }
   }
@@ -176,4 +202,21 @@ export class AutosaveController extends ApplicationController {
     }
     return inputs.filter((element) => !element.disabled);
   }
+
+  private toggleOtherInput(target: HTMLSelectElement | HTMLInputElement) {
+    const parent = target.closest('.editable-champ-drop_down_list');
+    const inputGroup = parent?.querySelector<HTMLElement>('.drop_down_other');
+    if (inputGroup) {
+      const input = inputGroup.querySelector('input');
+      if (input) {
+        if (target.value == '__other__') {
+          show(inputGroup);
+          input.disabled = false;
+        } else {
+          hide(inputGroup);
+          input.disabled = true;
+        }
+      }
+    }
+  }
 }

app/javascript/controllers/type_de_champ_editor_controller.ts

@@ -1,6 +1,11 @@
 /* eslint-disable react-hooks/rules-of-hooks */
 import { ActionEvent } from '@hotwired/stimulus';
-import { httpRequest } from '@utils';
+import {
+  httpRequest,
+  isSelectElement,
+  isCheckboxOrRadioInputElement,
+  isTextInputElement
+} from '@utils';
 import { useIntersection } from 'stimulus-use';
 
 import { ApplicationController } from './application_controller';
@@ -170,26 +175,3 @@ function createHiddenInput(
   input.value = String(value);
   form.appendChild(input);
 }
-
-function isSelectElement(element: HTMLElement): element is HTMLSelectElement {
-  return element.tagName == 'SELECT';
-}
-
-function isCheckboxOrRadioInputElement(
-  element: HTMLElement & { type?: string }
-): element is HTMLInputElement {
-  return (
-    element.tagName == 'INPUT' &&
-    (element.type == 'checkbox' || element.type == 'radio')
-  );
-}
-
-function isTextInputElement(
-  element: HTMLElement & { type?: string }
-): element is HTMLInputElement {
-  return (
-    ['INPUT', 'TEXTAREA'].includes(element.tagName) &&
-    element.type != 'checkbox' &&
-    element.type != 'radio'
-  );
-}

app/javascript/entrypoints/application.js

@@ -19,7 +19,6 @@ import '../new_design/spinner';
 import '../new_design/support';
 
 import '../new_design/champs/linked-drop-down-list';
-import '../new_design/champs/drop-down-list';
 
 import {
   toggleCondidentielExplanation,

app/javascript/new_design/champs/drop-down-list.js

@@ -1,20 +0,0 @@
-import { delegate, show, hide } from '@utils';
-
-delegate(
-  'change',
-  '.editable-champ-drop_down_list select, .editable-champ-drop_down_list input[type="radio"]',
-  (event) => {
-    const parent = event.target.closest('.editable-champ-drop_down_list');
-    const inputGroup = parent?.querySelector('.drop_down_other');
-    if (inputGroup) {
-      const input = inputGroup.querySelector('input');
-      if (event.target.value === '__other__') {
-        show(inputGroup);
-        input.disabled = false;
-      } else {
-        hide(inputGroup);
-        input.disabled = true;
-      }
-    }
-  }
-);

app/javascript/shared/utils.ts

@@ -272,3 +272,28 @@ export function isNumeric(s: string) {
   const n = parseFloat(s);
   return !isNaN(n) && isFinite(n);
 }
+
+export function isSelectElement(
+  element: HTMLElement
+): element is HTMLSelectElement {
+  return element.tagName == 'SELECT';
+}
+
+export function isCheckboxOrRadioInputElement(
+  element: HTMLElement & { type?: string }
+): element is HTMLInputElement {
+  return (
+    element.tagName == 'INPUT' &&
+    (element.type == 'checkbox' || element.type == 'radio')
+  );
+}
+
+export function isTextInputElement(
+  element: HTMLElement & { type?: string }
+): element is HTMLInputElement {
+  return (
+    ['INPUT', 'TEXTAREA'].includes(element.tagName) &&
+    element.type != 'checkbox' &&
+    element.type != 'radio'
+  );
+}

spec/system/users/dropdown_spec.rb

@@ -54,6 +54,8 @@ describe 'dropdown list with other option activated', js: true do
 
       select("Secondary 1.2")
       expect(page).to have_selector(".autosave-status.succeeded", visible: true)
+
+      wait_until { user_dossier.champs.first.value == "Secondary 1.2" }
       expect(user_dossier.champs.first.value).to eq("Secondary 1.2")
     end
   end