bug #1548 [Live] Reverting ignoreActiveValue: true in Idiomorph (weav…

…erryan)

This PR was merged into the 2.x branch.

Discussion
----------

[Live] Reverting ignoreActiveValue: true in Idiomorph

| Q             | A
| ------------- | ---
| Bug fix?      | yes/no
| New feature?  | yes/no <!-- please update src/**/CHANGELOG.md files -->
| Issues        | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT

This setting was recently reverted in Turbo (hotwired/turbo#1195). By setting this to true, it makes it impossible to update the active input's value with a new value from the server (see hotwired/turbo#1194).

I had used this to fix a cursor position bug. We now fix it in a different way.

Cheers!

Commits
-------

a15740e [Live] Reverting ignoreActiveValue: true in Idiomorph

src/LiveComponent/CHANGELOG.md

@@ -46,6 +46,8 @@
     if you were passing arguments to the event name, use action parameter attributes
     for those as well - e.g. `data-live-foo-param="bar"`.
 
+-   Reverted setting `ignoreActiveValue: true` in Idiomorph
+
 ## 2.15.0
 
 -   [BC BREAK] The `data-live-id` attribute was changed to `id` #1484

src/LiveComponent/assets/dist/live_controller.js

@@ -1349,7 +1349,6 @@ function executeMorphdom(rootFromElement, rootToElement, modifiedFieldElements,
         syncAttributes(newElement, oldElement);
     });
     Idiomorph.morph(rootFromElement, rootToElement, {
-        ignoreActiveValue: true,
         callbacks: {
             beforeNodeMorphed: (fromEl, toEl) => {
                 if (!(fromEl instanceof Element) || !(toEl instanceof Element)) {
@@ -1375,6 +1374,11 @@ function executeMorphdom(rootFromElement, rootToElement, modifiedFieldElements,
                     if (modifiedFieldElements.includes(fromEl)) {
                         setValueOnElement(toEl, getElementValue(fromEl));
                     }
+                    if (fromEl === document.activeElement
+                        && fromEl !== document.body
+                        && null !== getModelDirectiveFromElement(fromEl, false)) {
+                        setValueOnElement(toEl, getElementValue(fromEl));
+                    }
                     const elementChanges = externalMutationTracker.getChangedElement(fromEl);
                     if (elementChanges) {
                         elementChanges.applyToElement(toEl);

src/LiveComponent/assets/src/Component/UnsyncedInputsTracker.ts

@@ -73,7 +73,7 @@ export default class {
 /**
  * Tracks field & models whose values are "unsynced".
  *
- * For a model, unsynced means that the value has been updated inside of
+ * For a model, unsynced means that the value has been updated inside
  * a field (e.g. an input), but that this new value hasn't
  * yet been set onto the actual model data. It is "unsynced"
  * from the underlying model data.

src/LiveComponent/assets/src/morphdom.ts

@@ -1,4 +1,4 @@
-import { cloneHTMLElement, setValueOnElement } from './dom_utils';
+import { cloneHTMLElement, getModelDirectiveFromElement, setValueOnElement } from './dom_utils';
 // @ts-ignore
 import { Idiomorph } from 'idiomorph/dist/idiomorph.esm.js';
 import { normalizeAttributesForComparison } from './normalize_attributes_for_comparison';
@@ -53,12 +53,6 @@ export function executeMorphdom(
     });
 
     Idiomorph.morph(rootFromElement, rootToElement, {
-        // We handle updating the value of fields that have been changed
-        // since the HTML was requested. However, the active element is
-        // a special case: replacing the value isn't enough. We need to
-        // prevent the value from being changed in the first place so the
-        // user's cursor position is maintained.
-        ignoreActiveValue: true,
         callbacks: {
             beforeNodeMorphed: (fromEl: Element, toEl: Element) => {
                 // Idiomorph loop also over Text node
@@ -106,6 +100,23 @@ export function executeMorphdom(
                         setValueOnElement(toEl, getElementValue(fromEl));
                     }
 
+                    // Special handling for the active element of a model field.
+                    // Make the "to" element match the "from" element's value
+                    // to avoid any value change during the morphing. After morphing,
+                    // the SetValuesOntoModelFieldsPlugin handles setting the value
+                    // to whatever is in the data store.
+                    // Avoiding changing the value during morphing is important
+                    // to maintain the cursor position.
+                    // We skip this for non-model elements and allow this to either
+                    // maintain the value if changed (see code above) or for the
+                    // morphing process to update it to the value from the server.
+                    if (fromEl === document.activeElement
+                        && fromEl !== document.body
+                        && null !== getModelDirectiveFromElement(fromEl, false)
+                    ) {
+                        setValueOnElement(toEl, getElementValue(fromEl));
+                    }
+
                     // handle any external changes to this element
                     const elementChanges = externalMutationTracker.getChangedElement(fromEl);
                     if (elementChanges) {

src/LiveComponent/assets/test/controller/render.test.ts

@@ -157,7 +157,7 @@ describe('LiveController rendering Tests', () => {
         `);
 
         test.expectsAjaxCall()
-            .expectUpdatedData({ name: 'Hello' })
+            .expectUpdatedData({ name: 'Hello' });
 
         const input = test.queryByDataModel('name') as HTMLInputElement;
         userEvent.type(input, 'Hello');
@@ -171,6 +171,29 @@ describe('LiveController rendering Tests', () => {
         expect(input.value).toBe('Hel!lo');
     });
 
+    it('uses the new value of an unmapped field that was NOT modified even if active', async () => {
+        const test = await createTest({ title: 'greetings' }, (data: any) => `
+            <div ${initComponent(data)}>
+                <!-- An unmapped field -->
+                <input value="${data.title}">
+
+                Title: "${data.title}"
+            </div>
+        `);
+
+        test.expectsAjaxCall()
+            .serverWillChangeProps((data: any) => {
+                // change the data on the server so the template renders differently
+                data.title = 'Hello';
+            });
+
+        const input = test.element.querySelector('input') as HTMLInputElement;
+        // focus the input, but don't change it
+        userEvent.type(input, '');
+        await test.component.render();
+        expect(input.value).toEqual('Hello');
+    });
+
     it('does not render over elements with data-live-ignore', async () => {
         const test = await createTest({ firstName: 'Ryan' }, (data: any) => `
             <div ${initComponent(data)}>