diff --git a/src/core/drive/visit.ts b/src/core/drive/visit.ts index 9c6566658..260739e87 100644 --- a/src/core/drive/visit.ts +++ b/src/core/drive/visit.ts @@ -6,7 +6,7 @@ import { getAnchor } from "../url" import { Snapshot } from "../snapshot" import { PageSnapshot } from "./page_snapshot" import { Action } from "../types" -import { uuid } from "../../util" +import { getHistoryMethodForAction, uuid } from "../../util" import { PageView } from "./page_view" export interface VisitDelegate { @@ -45,6 +45,8 @@ export type VisitOptions = { response?: VisitResponse visitCachedSnapshot(snapshot: Snapshot): void willRender: boolean + updateHistory: boolean + restorationIdentifier?: string shouldCacheSnapshot: boolean } @@ -53,6 +55,7 @@ const defaultOptions: VisitOptions = { historyChanged: false, visitCachedSnapshot: () => {}, willRender: true, + updateHistory: true, shouldCacheSnapshot: true, } @@ -77,6 +80,7 @@ export class Visit implements FetchRequestDelegate { readonly timingMetrics: TimingMetrics = {} readonly visitCachedSnapshot: (snapshot: Snapshot) => void readonly willRender: boolean + readonly updateHistory: boolean followedRedirect = false frame?: number @@ -110,6 +114,7 @@ export class Visit implements FetchRequestDelegate { response, visitCachedSnapshot, willRender, + updateHistory, shouldCacheSnapshot, } = { ...defaultOptions, @@ -123,6 +128,7 @@ export class Visit implements FetchRequestDelegate { this.isSamePage = this.delegate.locationWithActionIsSamePage(this.location, this.action) this.visitCachedSnapshot = visitCachedSnapshot this.willRender = willRender + this.updateHistory = updateHistory this.scrolled = !willRender this.shouldCacheSnapshot = shouldCacheSnapshot } @@ -187,9 +193,9 @@ export class Visit implements FetchRequestDelegate { } changeHistory() { - if (!this.historyChanged) { + if (!this.historyChanged && this.updateHistory) { const actionForHistory = this.location.href === this.referrer?.href ? "replace" : this.action - const method = this.getHistoryMethodForAction(actionForHistory) + const method = getHistoryMethodForAction(actionForHistory) this.history.update(method, this.location, this.restorationIdentifier) this.historyChanged = true } diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts index f09d7b86d..5a29907c3 100644 --- a/src/core/frames/frame_controller.ts +++ b/src/core/frames/frame_controller.ts @@ -7,7 +7,16 @@ import { import { FetchMethod, FetchRequest, FetchRequestDelegate, FetchRequestHeaders } from "../../http/fetch_request" import { FetchResponse } from "../../http/fetch_response" import { AppearanceObserver, AppearanceObserverDelegate } from "../../observers/appearance_observer" -import { clearBusyState, dispatch, getAttribute, parseHTMLDocument, markAsBusy } from "../../util" +import { + clearBusyState, + dispatch, + getAttribute, + parseHTMLDocument, + markAsBusy, + uuid, + getHistoryMethodForAction, + getVisitAction, +} from "../../util" import { FormSubmission, FormSubmissionDelegate } from "../drive/form_submission" import { Snapshot } from "../snapshot" import { ViewDelegate, ViewRenderOptions } from "../view" @@ -18,7 +27,8 @@ import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor" import { FormLinkInterceptor, FormLinkInterceptorDelegate } from "../../observers/form_link_interceptor" import { FrameRenderer } from "./frame_renderer" import { session } from "../index" -import { isAction } from "../types" +import { isAction, Action } from "../types" +import { VisitOptions } from "../drive/visit" import { TurboBeforeFrameRenderEvent } from "../session" export class FrameController @@ -45,6 +55,9 @@ export class FrameController private connected = false private hasBeenLoaded = false private ignoredAttributes: Set = new Set() + private action: Action | null = null + private frame?: FrameElement + readonly restorationIdentifier: string private previousFrameElement?: FrameElement constructor(element: FrameElement) { @@ -53,6 +66,7 @@ export class FrameController this.appearanceObserver = new AppearanceObserver(this, this.element) this.formLinkInterceptor = new FormLinkInterceptor(this, this.element) this.linkInterceptor = new LinkInterceptor(this, this.element) + this.restorationIdentifier = uuid() this.formSubmitObserver = new FormSubmitObserver(this, this.element) } @@ -141,6 +155,8 @@ export class FrameController false ) if (this.view.renderPromise) await this.view.renderPromise + this.changeHistory() + await this.view.render(renderer) this.complete = true session.frameRendered(fetchResponse, this.element) @@ -328,9 +344,10 @@ export class FrameController } private proposeVisitIfNavigatedWithAction(frame: FrameElement, element: Element, submitter?: HTMLElement) { - const action = getAttribute("data-turbo-action", submitter, element, frame) + this.action = getVisitAction(submitter, element, frame) + this.frame = frame - if (isAction(action)) { + if (isAction(this.action)) { const { visitCachedSnapshot } = frame.delegate frame.delegate.fetchResponseLoaded = (fetchResponse: FetchResponse) => { @@ -338,18 +355,29 @@ export class FrameController const { statusCode, redirected } = fetchResponse const responseHTML = frame.ownerDocument.documentElement.outerHTML const response = { statusCode, redirected, responseHTML } - - session.visit(frame.src, { - action, + const options: Partial = { response, visitCachedSnapshot, willRender: false, - }) + updateHistory: false, + restorationIdentifier: this.restorationIdentifier, + } + + if (this.action) options.action = this.action + + session.visit(frame.src, options) } } } } + changeHistory() { + if (this.action && this.frame) { + const method = getHistoryMethodForAction(this.action) + session.history.update(method, expandURL(this.frame.src || ""), this.restorationIdentifier) + } + } + private findFrameElement(element: Element, submitter?: HTMLElement) { const id = getAttribute("data-turbo-frame", submitter, element) || this.element.getAttribute("target") return getFrameElementById(id) ?? this.element diff --git a/src/core/native/browser_adapter.ts b/src/core/native/browser_adapter.ts index 20792025e..9b5988e3c 100644 --- a/src/core/native/browser_adapter.ts +++ b/src/core/native/browser_adapter.ts @@ -24,7 +24,7 @@ export class BrowserAdapter implements Adapter { } visitProposedToLocation(location: URL, options?: Partial) { - this.navigator.startVisit(location, uuid(), options) + this.navigator.startVisit(location, options?.restorationIdentifier || uuid(), options) } visitStarted(visit: Visit) { diff --git a/src/tests/fixtures/tabs.html b/src/tests/fixtures/tabs.html new file mode 100644 index 000000000..cc9666c29 --- /dev/null +++ b/src/tests/fixtures/tabs.html @@ -0,0 +1,22 @@ + + + + + Tabs + + + + +

Tabs

+ + +
+ Tab 1 + Tab 2 + Tab 3 +
+ +
One
+
+ + diff --git a/src/tests/fixtures/tabs/three.html b/src/tests/fixtures/tabs/three.html new file mode 100644 index 000000000..cc7804ef8 --- /dev/null +++ b/src/tests/fixtures/tabs/three.html @@ -0,0 +1,9 @@ + +
+ Tab 1 + Tab 2 + Tab 3 +
+ +
Three
+
diff --git a/src/tests/fixtures/tabs/two.html b/src/tests/fixtures/tabs/two.html new file mode 100644 index 000000000..80d46f66c --- /dev/null +++ b/src/tests/fixtures/tabs/two.html @@ -0,0 +1,9 @@ + +
+ Tab 1 + Tab 2 + Tab 3 +
+ +
Two
+
diff --git a/src/tests/fixtures/test.js b/src/tests/fixtures/test.js index fa98eb3d2..acb543061 100644 --- a/src/tests/fixtures/test.js +++ b/src/tests/fixtures/test.js @@ -47,6 +47,7 @@ "turbo:before-fetch-request", "turbo:before-fetch-response", "turbo:visit", + "turbo:before-frame-render", "turbo:frame-load", "turbo:frame-render", "turbo:reload" diff --git a/src/tests/functional/frame_navigation_tests.ts b/src/tests/functional/frame_navigation_tests.ts index 212b709c5..3c4533576 100644 --- a/src/tests/functional/frame_navigation_tests.ts +++ b/src/tests/functional/frame_navigation_tests.ts @@ -1,5 +1,6 @@ import { test } from "@playwright/test" -import { nextEventOnTarget } from "../helpers/page" +import { getFromLocalStorage, nextEventNamed, nextEventOnTarget, pathname } from "../helpers/page" +import { assert } from "chai" test.beforeEach(async ({ page }) => { await page.goto("/src/tests/fixtures/frame_navigation.html") @@ -22,3 +23,25 @@ test("test frame navigation with exterior link", async ({ page }) => { await nextEventOnTarget(page, "frame", "turbo:frame-load") }) + +test("test promoted frame navigation updates the URL before rendering", async ({ page }) => { + await page.goto("/src/tests/fixtures/tabs.html") + + page.evaluate(() => { + addEventListener("turbo:before-frame-render", () => { + localStorage.setItem("beforeRenderUrl", window.location.pathname) + localStorage.setItem("beforeRenderContent", document.querySelector("#tab-content")?.textContent || "") + }) + }) + + await page.click("#tab-2") + await nextEventNamed(page, "turbo:before-frame-render") + + assert.equal(await getFromLocalStorage(page, "beforeRenderUrl"), "/src/tests/fixtures/tabs/two.html") + assert.equal(await getFromLocalStorage(page, "beforeRenderContent"), "One") + + await nextEventNamed(page, "turbo:frame-render") + + assert.equal(await pathname(page.url()), "/src/tests/fixtures/tabs/two.html") + assert.equal(await page.textContent("#tab-content"), "Two") +}) diff --git a/src/util.ts b/src/util.ts index 33ff1424a..d76222601 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1,3 +1,5 @@ +import { Action, isAction } from "./core/types" + export type DispatchOptions = { target: EventTarget cancelable: boolean @@ -96,6 +98,22 @@ export function clearBusyState(...elements: Element[]) { } } +export function getHistoryMethodForAction(action: Action) { + switch (action) { + case "replace": + return history.replaceState + case "advance": + case "restore": + return history.pushState + } +} + +export function getVisitAction(...elements: (Element | undefined)[]): Action | null { + const action = getAttribute("data-turbo-action", ...elements) + + return isAction(action) ? action : null +} + export function getMetaElement(name: string): HTMLMetaElement | null { return document.querySelector(`meta[name="${name}"]`) }