Skip to content

[Live] Reverting ignoreActiveValue: true in Idiomorph #1548

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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