diff --git a/src/core/drive/page_renderer.ts b/src/core/drive/page_renderer.ts index af6adcb7f..177a5a8ec 100644 --- a/src/core/drive/page_renderer.ts +++ b/src/core/drive/page_renderer.ts @@ -1,10 +1,6 @@ -import { Renderer } from "../renderer" +import { Renderer, replaceElementWithElement } from "../renderer" import { PageSnapshot } from "./page_snapshot" -export type PermanentElement = Element & { id: string } - -export type Placeholder = { element: Element, permanentElement: PermanentElement } - export class PageRenderer extends Renderer { get shouldRender() { return this.newSnapshot.isVisitable && this.trackedElementsAreIdentical @@ -21,7 +17,7 @@ export class PageRenderer extends Renderer { finishRendering() { super.finishRendering() if (this.isPreview) { - this.focusFirstAutofocusableElement() + this.focusFirstAutofocusableElement(this.newSnapshot) } } @@ -45,10 +41,10 @@ export class PageRenderer extends Renderer { } replaceBody() { - const placeholders = this.relocateCurrentBodyPermanentElements() - this.activateNewBody() - this.assignNewBody() - this.replacePlaceholderElementsWithClonedPermanentElements(placeholders) + this.renderSnapshotWithPermanentElements(() => { + this.activateNewBody() + this.assignNewBody() + }) } get trackedElementsAreIdentical() { @@ -79,27 +75,6 @@ export class PageRenderer extends Renderer { } } - relocateCurrentBodyPermanentElements() { - return this.currentBodyPermanentElements.reduce((placeholders, permanentElement) => { - const newElement = this.newSnapshot.getPermanentElementById(permanentElement.id) - if (newElement) { - const placeholder = createPlaceholderForPermanentElement(permanentElement) - replaceElementWithElement(permanentElement, placeholder.element) - replaceElementWithElement(newElement, permanentElement) - return [ ...placeholders, placeholder ] - } else { - return placeholders - } - }, [] as Placeholder[]) - } - - replacePlaceholderElementsWithClonedPermanentElements(placeholders: Placeholder[]) { - for (const { element, permanentElement } of placeholders) { - const clonedElement = permanentElement.cloneNode(true) - replaceElementWithElement(element, clonedElement) - } - } - activateNewBody() { document.adoptNode(this.newElement) this.activateNewBodyScriptElements() @@ -120,13 +95,6 @@ export class PageRenderer extends Renderer { } } - focusFirstAutofocusableElement() { - const element = this.newSnapshot.firstAutofocusableElement - if (elementIsFocusable(element)) { - element.focus() - } - } - get newHeadStylesheetElements() { return this.newHeadSnapshot.getStylesheetElementsNotInSnapshot(this.currentHeadSnapshot) } @@ -143,29 +111,7 @@ export class PageRenderer extends Renderer { return this.newHeadSnapshot.provisionalElements } - get currentBodyPermanentElements(): PermanentElement[] { - return this.currentSnapshot.getPermanentElementsPresentInSnapshot(this.newSnapshot) - } - get newBodyScriptElements() { return [ ...this.newElement.querySelectorAll("script") ] } } - -function createPlaceholderForPermanentElement(permanentElement: PermanentElement) { - const element = document.createElement("meta") - element.setAttribute("name", "turbo-permanent-placeholder") - element.setAttribute("content", permanentElement.id) - return { element, permanentElement } -} - -function replaceElementWithElement(fromElement: Element, toElement: Element) { - const parentElement = fromElement.parentElement - if (parentElement) { - return parentElement.replaceChild(toElement, fromElement) - } -} - -function elementIsFocusable(element: any): element is { focus: () => void } { - return element && typeof element.focus == "function" -} diff --git a/src/core/frames/frame_renderer.ts b/src/core/frames/frame_renderer.ts index 8375f68ab..aa5ba2164 100644 --- a/src/core/frames/frame_renderer.ts +++ b/src/core/frames/frame_renderer.ts @@ -9,10 +9,12 @@ export class FrameRenderer extends Renderer { async render() { await nextAnimationFrame() - this.loadFrameElement() + this.renderSnapshotWithPermanentElements(() => { + this.loadFrameElement() + }) this.scrollFrameIntoView() await nextAnimationFrame() - this.focusFirstAutofocusableElement() + this.focusFirstAutofocusableElement(this.newSnapshot) } loadFrameElement() { @@ -40,24 +42,6 @@ export class FrameRenderer extends Renderer { } return false } - - focusFirstAutofocusableElement() { - const element = this.firstAutofocusableElement - if (element) { - element.focus() - return true - } - return false - } - - get id() { - return this.currentElement.id - } - - get firstAutofocusableElement() { - const element = this.currentElement.querySelector("[autofocus]") - return element instanceof HTMLElement ? element : null - } } function readScrollLogicalPosition(value: string | null, defaultValue: ScrollLogicalPosition): ScrollLogicalPosition { diff --git a/src/core/renderer.ts b/src/core/renderer.ts index a150af42e..c329f2a10 100644 --- a/src/core/renderer.ts +++ b/src/core/renderer.ts @@ -5,6 +5,10 @@ type ResolvingFunctions = { reject(reason?: any): void } +export type PermanentElement = Element & { id: string } + +export type Placeholder = { element: Element, permanentElement: PermanentElement } + export abstract class Renderer = Snapshot> { readonly currentSnapshot: S readonly newSnapshot: S @@ -48,6 +52,19 @@ export abstract class Renderer = Snapsh } } + renderSnapshotWithPermanentElements(render: () => void) { + const placeholders = relocatePermanentElements(this.currentSnapshot, this.newSnapshot) + render() + replacePlaceholderElementsWithClonedPermanentElements(placeholders) + } + + focusFirstAutofocusableElement(snapshot: Snapshot) { + const element = snapshot.firstAutofocusableElement + if (elementIsFocusable(element)) { + element.focus() + } + } + get currentElement() { return this.currentSnapshot.element } @@ -57,8 +74,47 @@ export abstract class Renderer = Snapsh } } +export function replaceElementWithElement(fromElement: Element, toElement: Element) { + const parentElement = fromElement.parentElement + if (parentElement) { + return parentElement.replaceChild(toElement, fromElement) + } +} + function copyElementAttributes(destinationElement: Element, sourceElement: Element) { for (const { name, value } of [ ...sourceElement.attributes ]) { destinationElement.setAttribute(name, value) } } + +function createPlaceholderForPermanentElement(permanentElement: PermanentElement) { + const element = document.createElement("meta") + element.setAttribute("name", "turbo-permanent-placeholder") + element.setAttribute("content", permanentElement.id) + return { element, permanentElement } +} + +function replacePlaceholderElementsWithClonedPermanentElements(placeholders: Placeholder[]) { + for (const { element, permanentElement } of placeholders) { + const clonedElement = permanentElement.cloneNode(true) + replaceElementWithElement(element, clonedElement) + } +} + +function relocatePermanentElements(currentSnapshot: Snapshot, newSnapshot: Snapshot) { + return currentSnapshot.getPermanentElementsPresentInSnapshot(newSnapshot).reduce((placeholders, permanentElement) => { + const newElement = newSnapshot.getPermanentElementById(permanentElement.id) + if (newElement) { + const placeholder = createPlaceholderForPermanentElement(permanentElement) + replaceElementWithElement(permanentElement, placeholder.element) + replaceElementWithElement(newElement, permanentElement) + return [ ...placeholders, placeholder ] + } else { + return placeholders + } + }, [] as Placeholder[]) +} + +function elementIsFocusable(element: any): element is { focus: () => void } { + return element && typeof element.focus == "function" +} diff --git a/src/tests/fixtures/frames/without_layout.html b/src/tests/fixtures/frames/without_layout.html new file mode 100644 index 000000000..41c6ebf62 --- /dev/null +++ b/src/tests/fixtures/frames/without_layout.html @@ -0,0 +1,5 @@ + +

Frames: Without Layout

+ +
Permanent element
+
diff --git a/src/tests/fixtures/permanent_element.html b/src/tests/fixtures/permanent_element.html index 84250e7ca..31048dd72 100644 --- a/src/tests/fixtures/permanent_element.html +++ b/src/tests/fixtures/permanent_element.html @@ -11,5 +11,9 @@

Permanent element

Permanent element
+ + +
Permanent element
+
diff --git a/src/tests/fixtures/rendering.html b/src/tests/fixtures/rendering.html index d71a2da72..da2688364 100644 --- a/src/tests/fixtures/rendering.html +++ b/src/tests/fixtures/rendering.html @@ -18,7 +18,13 @@

Rendering

Nonexistent link

Visit control: reload

Permanent element

+

Permanent element in frame

+

Permanent element in frame without layout

Rendering
+ + +
Rendering
+
diff --git a/src/tests/functional/rendering_tests.ts b/src/tests/functional/rendering_tests.ts index 06400b5ff..bc14521b0 100644 --- a/src/tests/functional/rendering_tests.ts +++ b/src/tests/functional/rendering_tests.ts @@ -124,6 +124,26 @@ export class RenderingTests extends TurboDriveTestCase { this.assert(await permanentElement.equals(await this.permanentElement)) } + async "test preserves permanent elements within turbo-frames"() { + let permanentElement = await this.querySelector("#permanent-in-frame") + this.assert.equal(await permanentElement.getVisibleText(), "Rendering") + + await this.clickSelector("#permanent-in-frame-element-link") + await this.nextBeat + permanentElement = await this.querySelector("#permanent-in-frame") + this.assert.equal(await permanentElement.getVisibleText(), "Rendering") + } + + async "test preserves permanent elements within turbo-frames rendered without layouts"() { + let permanentElement = await this.querySelector("#permanent-in-frame") + this.assert.equal(await permanentElement.getVisibleText(), "Rendering") + + await this.clickSelector("#permanent-in-frame-without-layout-element-link") + await this.nextBeat + permanentElement = await this.querySelector("#permanent-in-frame") + this.assert.equal(await permanentElement.getVisibleText(), "Rendering") + } + async "test before-cache event"() { this.beforeCache(body => body.innerHTML = "Modified") this.clickSelector("#same-origin-link") diff --git a/src/tests/functional/visit_tests.ts b/src/tests/functional/visit_tests.ts index ac874cf78..c93992ecc 100644 --- a/src/tests/functional/visit_tests.ts +++ b/src/tests/functional/visit_tests.ts @@ -38,6 +38,7 @@ export class VisitTests extends TurboDriveTestCase { async "test visiting a location served with a non-HTML content type"() { const urlBeforeVisit = await this.location await this.visitLocation("/src/tests/fixtures/svg") + await this.nextBeat const url = await this.remote.getCurrentUrl() const contentType = await contentTypeOfURL(url)