Skip to content

Commit

Permalink
[Live] Reverting ignoreActiveValue: true in Idiomorph
Browse files Browse the repository at this point in the history
This setting was recently reverted in Turbo. By setting this to true,
it makes it impossible to update the active input's value with a new
value from the server.
  • Loading branch information
weaverryan committed Feb 27, 2024
1 parent 368e533 commit a15740e
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 10 deletions.
2 changes: 2 additions & 0 deletions src/LiveComponent/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion src/LiveComponent/assets/dist/live_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
25 changes: 18 additions & 7 deletions src/LiveComponent/assets/src/morphdom.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
25 changes: 24 additions & 1 deletion src/LiveComponent/assets/test/controller/render.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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)}>
Expand Down

0 comments on commit a15740e

Please sign in to comment.