Skip to content

[Live] Fixing bug where the active input would maintain its value, but lose its cursor position #1501

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 16, 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
3 changes: 2 additions & 1 deletion src/LiveComponent/assets/dist/live_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ var Idiomorph = (function () {
* @returns {boolean}
*/
function ignoreValueOfActiveElement(possibleActiveElement, ctx) {
return ctx.ignoreActiveValue && possibleActiveElement === document.activeElement;
return ctx.ignoreActiveValue && possibleActiveElement === document.activeElement && possibleActiveElement !== document.body;
}

/**
Expand Down Expand Up @@ -1378,6 +1378,7 @@ 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 Down
2 changes: 1 addition & 1 deletion src/LiveComponent/assets/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
}
},
"dependencies": {
"idiomorph": "^0.3.0"
"idiomorph": "https://github.com/bigskysoftware/idiomorph.git"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure about this ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, but good question. Copied from Turbo - it allows us to get the absolute latest version. Will change after they tag next. We embed this library (for now), so this isn't exposed to the user.

},
"peerDependencies": {
"@hotwired/stimulus": "^3.0.0"
Expand Down
6 changes: 6 additions & 0 deletions src/LiveComponent/assets/src/morphdom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ 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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\o/ As we suspected 🕵️ 🕵️

callbacks: {
beforeNodeMorphed: (fromEl: Element, toEl: Element) => {
// Idiomorph loop also over Text node
Expand Down
24 changes: 11 additions & 13 deletions src/LiveComponent/assets/test/controller/model.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -810,9 +810,9 @@ describe('LiveController data-model Tests', () => {
<div ${initComponent(data)}>
<form data-model>
<textarea name="comment" data-testid="comment">${data.comment}</textarea>
</form>
</form>

<button data-action="live#$render">Reload</button>
<input data-testid="other-input">
</div>
`);

Expand All @@ -823,17 +823,18 @@ describe('LiveController data-model Tests', () => {
// delay slightly so we can type in the textarea
.delayResponse(10);

getByText(test.element, 'Reload').click();
const renderPromise = test.component.render();
// mimic changing the field, but without (yet) triggering the change event
const commentField = getByTestId(test.element, 'comment');
if (!(commentField instanceof HTMLTextAreaElement)) {
throw new Error('wrong type');
}
const inputField = getByTestId(test.element, 'other-input');
userEvent.type(commentField, ' ftw!');
// To make the test more robust, make the textarea NOT the active element.
// The active element's value is ignored during morphing. Here, we want
// to test that even if the morph system *does* want to update the value
// of the textarea, the value will be kept.
userEvent.type(inputField, 'making this element active');

// wait for loading start and end
await waitFor(() => expect(test.element).toHaveAttribute('busy'));
await waitFor(() => expect(test.element).not.toHaveAttribute('busy'));
await renderPromise;

expect(commentField).toHaveValue('Live components ftw!');

Expand All @@ -845,10 +846,7 @@ describe('LiveController data-model Tests', () => {
data.comment = 'server changed comment';
});

getByText(test.element, 'Reload').click();
// wait for loading start and end
await waitFor(() => expect(test.element).toHaveAttribute('busy'));
await waitFor(() => expect(test.element).not.toHaveAttribute('busy'));
await test.component.render();

expect(commentField).toHaveValue('server changed comment');
});
Expand Down
22 changes: 22 additions & 0 deletions src/LiveComponent/assets/test/controller/render.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,28 @@ describe('LiveController rendering Tests', () => {
expect((test.element.querySelector('textarea') as HTMLTextAreaElement).value).toEqual('typing after the request starts');
});

it('conserves cursor position of active model element', async () => {
const test = await createTest({ name: '' }, (data) => `
<div ${initComponent(data)}>
<input data-model="name" class="anything">
</div>
`);

test.expectsAjaxCall()
.expectUpdatedData({ name: 'Hello' })

const input = test.queryByDataModel('name') as HTMLInputElement;
userEvent.type(input, 'Hello');
userEvent.keyboard('{ArrowLeft}{ArrowLeft}');

await test.component.render();

// the cursor position should be preserved
expect(input.selectionStart).toBe(3);
userEvent.type(input, '!');
expect(input.value).toBe('Hel!lo');
});

it('does not render over elements with data-live-ignore', async () => {
const test = await createTest({ firstName: 'Ryan' }, (data: any) => `
<div ${initComponent(data)}>
Expand Down