Skip to content
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.

Sign up for GitHub

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

Preserve input values in cache #666

Merged
merged 1 commit into from
Aug 5, 2022
Merged

Preserve input values in cache #666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 16 additions & 1 deletion src/core/drive/page_snapshot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,22 @@ export class PageSnapshot extends Snapshot<HTMLBodyElement> {
}

clone() {
return new PageSnapshot(this.element.cloneNode(true), this.headSnapshot)
const clonedElement = this.element.cloneNode(true)

const selectElements = this.element.querySelectorAll("select")
const clonedSelectElements = clonedElement.querySelectorAll("select")

for (const [index, source] of selectElements.entries()) {
for (const [optionIndex, option] of Array.from(source.options).entries()) {
clonedSelectElements[index].options[optionIndex].selected = option.selected
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, the following approach is around 3x to 4x faster. Not an issue for small selects, but if the page contains large selects (e.g. Country Selector) it adds up.

for (const [index, source] of selectElements.entries()) {
  [...clonedSelectElements[index].selectedOptions].forEach(option => { option.selected = false });
  [...source.selectedOptions].forEach(option => clonedSelectElements[index].options[option.index].selected = true)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would it be even faster if the [...] syntax were replaced with a for ... of loop? I can open a follow-up PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've opened #669. Thank you!


for (const clonedPasswordInput of clonedElement.querySelectorAll<HTMLInputElement>('input[type="password"]')) {
clonedPasswordInput.value = ""
}

return new PageSnapshot(clonedElement, this.headSnapshot)
}

get headElement() {
Expand Down
18 changes: 18 additions & 0 deletions src/tests/fixtures/rendering.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,24 @@ <h1>Rendering</h1>
<p><a id="permanent-in-frame-element-link" href="/src/tests/fixtures/permanent_element.html" data-turbo-frame="frame">Permanent element in frame</a></p>
<p><a id="permanent-in-frame-without-layout-element-link" href="/src/tests/fixtures/frames/without_layout.html" data-turbo-frame="frame">Permanent element in frame without layout</a></p>
<p><a id="delayed-link" href="/__turbo/delayed_response">Delayed link</a></p>
<form>
<input type="text" id="text-input">
<input type="radio" id="radio-input">
<input type="checkbox" id="checkbox-input">
<textarea id="textarea"></textarea>
<select id="select">
<option value="1">1</option>
<option value="2">2</option>
</select>
<select id="select-multiple" multiple>
<option value="1">1</option>
<option value="2">2</option>
</select>

<input type="password" id="password-input">

<input type="reset" id="reset-input">
</form>
</section>
<div id="permanent" data-turbo-permanent>Rendering</div>

Expand Down
58 changes: 58 additions & 0 deletions src/tests/functional/rendering_tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
nextBody,
nextEventNamed,
pathname,
propertyForSelector,
readEventLogs,
scrollToSelector,
selectorHasFocus,
sleep,
Expand All @@ -19,6 +21,7 @@ import {
test.beforeEach(async ({ page }) => {
await page.goto("/src/tests/fixtures/rendering.html")
await clearLocalStorage(page)
await readEventLogs(page)
})

test("test triggers before-render and render events", async ({ page }) => {
Expand Down Expand Up @@ -392,6 +395,61 @@ test("test preserves permanent element video playback", async ({ page }) => {
assert.equal(timeAfterRender, timeBeforeRender, "element state is preserved")
})

test("test preserves input values", async ({ page }) => {
await page.fill("#text-input", "test")
await page.click("#checkbox-input")
await page.click("#radio-input")
await page.fill("#textarea", "test")
await page.selectOption("#select", "2")
await page.selectOption("#select-multiple", "2")

await page.click("#same-origin-link")
await nextEventNamed(page, "turbo:load")
await page.goBack()
await nextEventNamed(page, "turbo:load")

assert.equal(await propertyForSelector(page, "#text-input", "value"), "test")
assert.equal(await propertyForSelector(page, "#checkbox-input", "checked"), true)
assert.equal(await propertyForSelector(page, "#radio-input", "checked"), true)
assert.equal(await propertyForSelector(page, "#textarea", "value"), "test")
assert.equal(await propertyForSelector(page, "#select", "value"), "2")
assert.equal(await propertyForSelector(page, "#select-multiple", "value"), "2")
})

test("test does not preserve password values", async ({ page }) => {
await page.fill("#password-input", "test")

await page.click("#same-origin-link")
await nextEventNamed(page, "turbo:load")
await page.goBack()
await nextEventNamed(page, "turbo:load")

assert.equal(await propertyForSelector(page, "#password-input", "value"), "")
})

test("test <input type='reset'> clears values when restored from cache", async ({ page }) => {
await page.fill("#text-input", "test")
await page.click("#checkbox-input")
await page.click("#radio-input")
await page.fill("#textarea", "test")
await page.selectOption("#select", "2")
await page.selectOption("#select-multiple", "2")

await page.click("#same-origin-link")
await nextEventNamed(page, "turbo:load")
await page.goBack()
await nextEventNamed(page, "turbo:load")

await page.click("#reset-input")

assert.equal(await propertyForSelector(page, "#text-input", "value"), "")
assert.equal(await propertyForSelector(page, "#checkbox-input", "checked"), false)
assert.equal(await propertyForSelector(page, "#radio-input", "checked"), false)
assert.equal(await propertyForSelector(page, "#textarea", "value"), "")
assert.equal(await propertyForSelector(page, "#select", "value"), "1")
assert.equal(await propertyForSelector(page, "#select-multiple", "value"), "")
})

test("test before-cache event", async ({ page }) => {
await page.evaluate(() => {
addEventListener("turbo:before-cache", () => (document.body.innerHTML = "Modified"), { once: true })
Expand Down