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.

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

Add support for data-turbo-replace-method="morph" to force a morph #1145

Closed
Closed
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
10 changes: 9 additions & 1 deletion src/core/drive/navigator.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getVisitAction } from "../../util"
import { getVisitAction, getVisitReplaceMethod } from "../../util"
import { FormSubmission } from "./form_submission"
import { expandURL, getAnchor, getRequestURL } from "../url"
import { Visit } from "./visit"
Expand Down Expand Up @@ -79,8 +79,10 @@ export class Navigator {

const { statusCode, redirected } = fetchResponse
const action = this.#getActionForFormSubmission(formSubmission, fetchResponse)
const replaceMethod = this.#getReplaceMethodForFormSubmission(formSubmission, fetchResponse)
const visitOptions = {
action,
replaceMethod,
shouldCacheSnapshot,
response: { statusCode, responseHTML, redirected }
}
Expand Down Expand Up @@ -162,4 +164,10 @@ export class Navigator {
const sameLocationRedirect = fetchResponse.redirected && fetchResponse.location.href === this.location?.href
return sameLocationRedirect ? "replace" : "advance"
}

#getReplaceMethodForFormSubmission(formSubmission, fetchResponse) {
if (this.#getActionForFormSubmission(formSubmission, fetchResponse) !== "replace") return
const { submitter, formElement } = formSubmission
return getVisitReplaceMethod(submitter, formElement) || "body"
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if there's a better value than body here. It does not have the same clear action as morph does. The corresponding renderer is called PageRenderer and the rendering is not isolated to changing the body; it also does some merging of head elements etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's true that the contents of <head> also gets merged, but the body is what gets replaced and that's the thing in question; that's what differs between the morph & non-morph case. I was keying off this description from the docs:

During rendering, Turbo Drive replaces the current element outright and merges the contents of the element. The JavaScript window and document objects, and the element, persist from one rendering to the next.

But I initially had this as full. Do you think that's better? I don't feel strongly so I'm happy to go with full or something else.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have no strong opinions of this; my initial thought was replace, replaceBody or default?

}
}
12 changes: 10 additions & 2 deletions src/core/drive/page_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export class PageView extends View {
}

renderPage(snapshot, isPreview = false, willRender = true, visit) {
const shouldMorphPage = this.isPageRefresh(visit) && this.snapshot.shouldMorphPage
const shouldMorphPage = snapshot.shouldMorphPage && this.isMorphableVisit(visit)
const rendererClass = shouldMorphPage ? MorphRenderer : PageRenderer

const renderer = new rendererClass(this.snapshot, snapshot, PageRenderer.renderElement, isPreview, willRender)
Expand Down Expand Up @@ -55,8 +55,16 @@ export class PageView extends View {
return this.snapshotCache.get(location)
}

isMorphableVisit(visit) {
return this.isPageRefresh(visit) || this.isReplaceMethodMorph(visit)
}

isPageRefresh(visit) {
return !visit || (this.lastRenderedLocation.pathname === visit.location.pathname && visit.action === "replace")
return !visit || (this.lastRenderedLocation.href === visit.location.href && visit.action === "replace")
}

isReplaceMethodMorph(visit) {
return visit.action === "replace" && visit.replaceMethod === "morph"
}

shouldPreserveScrollPosition(visit) {
Expand Down
2 changes: 2 additions & 0 deletions src/core/drive/visit.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export class Visit {

const {
action,
replaceMethod,
historyChanged,
referrer,
snapshot,
Expand All @@ -78,6 +79,7 @@ export class Visit {
...options
}
this.action = action
this.replaceMethod = replaceMethod
this.historyChanged = historyChanged
this.referrer = referrer
this.snapshot = snapshot
Expand Down
1 change: 1 addition & 0 deletions src/core/frames/frame_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ export class FrameController {
// Appearance observer delegate

elementAppearedInViewport(element) {
// TODO: Determine if I need to check replaceMethod() here — I need to find the corresponding tests
this.proposeVisitIfNavigatedWithAction(element, getVisitAction(element))
this.#loadSourceURL()
}
Expand Down
4 changes: 4 additions & 0 deletions src/core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ export function registerAdapter(adapter) {
* @param options Options to apply
* @param options.action Type of history navigation to apply ("restore",
* "replace" or "advance")
* @param options.frame If specified, finds a turbo-frame element with an [id]
* attribute that matches this.
* @param options.replaceMethod If specified, changes the method used to
* perform a replace ("morph" or else it defaults to "body" replace)
* @param options.historyChanged Specifies whether the browser history has
* already been changed for this visit or not
* @param options.referrer Specifies the referrer of this visit such that
Expand Down
11 changes: 9 additions & 2 deletions src/core/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { ScrollObserver } from "../observers/scroll_observer"
import { StreamMessage } from "./streams/stream_message"
import { StreamMessageRenderer } from "./streams/stream_message_renderer"
import { StreamObserver } from "../observers/stream_observer"
import { clearBusyState, dispatch, findClosestRecursively, getVisitAction, markAsBusy, debounce } from "../util"
import { clearBusyState, dispatch, findClosestRecursively, getVisitAction, getVisitReplaceMethod, markAsBusy, debounce } from "../util"
import { PageView } from "./drive/page_view"
import { FrameElement } from "../elements/frame_element"
import { Preloader } from "./drive/preloader"
Expand Down Expand Up @@ -225,9 +225,10 @@ export class Session {

followedLinkToLocation(link, location) {
const action = this.getActionForLink(link)
const replaceMethod = this.getVisitReplaceMethodForLink(link)
const acceptsStreamResponse = link.hasAttribute("data-turbo-stream")

this.visit(location.href, { action, acceptsStreamResponse })
this.visit(location.href, { action, replaceMethod, acceptsStreamResponse })
}

// Navigator delegate
Expand Down Expand Up @@ -466,6 +467,12 @@ export class Session {
return getVisitAction(link) || "advance"
}

getVisitReplaceMethodForLink(link) {
if (this.getActionForLink(link) !== "replace") return

return getVisitReplaceMethod(link) || "body"
}

get snapshot() {
return this.view.snapshot
}
Expand Down
1 change: 1 addition & 0 deletions src/observers/form_link_click_observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export class FormLinkClickObserver {
const turboFrame = link.getAttribute("data-turbo-frame")
if (turboFrame) form.setAttribute("data-turbo-frame", turboFrame)

// TODO: Determine if I need to check data-turbo-replace-method here — I need to find the corresponding tests
const turboAction = getVisitAction(link)
if (turboAction) form.setAttribute("data-turbo-action", turboAction)

Expand Down
25 changes: 25 additions & 0 deletions src/tests/fixtures/destination_for_morphing.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<!DOCTYPE html>
<html id="one">
<head>
<meta charset="utf-8">
<title>One</title>
<script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
<script src="/src/tests/fixtures/test.js"></script>
<meta name="test" content="foo">
<meta name="turbo-refresh-method" content="morph">
<meta name="turbo-refresh-scroll" content="preserve">
</head>
<body>
<h1>One</h1>

<!--styles ensure that the element will be scrolled to top when navigated to via an anchored link -->
<a name="named-anchor"></a>
<div id="element-id" style="margin-top: 1em; height: 200vh">An element with an ID</div>
<p><a id="redirection-link" href="/__turbo/redirect?path=/src/tests/fixtures/visit.html">Redirection link</a></p>
<p><a id="page-refresh-link" data-turbo-action="replace" href="/src/tests/fixtures/page_refresh.html">Page refresh link</a></p>

<turbo-frame id="navigate-top">
Replaced only the frame
</turbo-frame>
</body>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: I can probably simplify this file a bit.

</html>
25 changes: 25 additions & 0 deletions src/tests/fixtures/navigation.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ <h1>Navigation</h1>
<p><a id="same-origin-unannotated-link" href="/src/tests/fixtures/one.html">Same-origin unannotated link</a></p>
<p><a id="same-origin-unannotated-link-search-params" href="/src/tests/fixtures/one.html?key=value">Same-origin unannotated link ?key=value</a></p>
<p><form id="same-origin-unannotated-form" method="get" action="/src/tests/fixtures/one.html"><button>Same-origin unannotated form</button></form></p>

<p><a id="same-origin-replace-link" href="/src/tests/fixtures/one.html" data-turbo-action="replace">Same-origin data-turbo-action=replace link</a></p>
<p><form id="same-origin-replace-form-get" action="/src/tests/fixtures/one.html" data-turbo-action="replace"><button>Same-origin data-turbo-action=replace form</button></form></p>
<p><form id="same-origin-replace-form-submitter-get" action="/src/tests/fixtures/one.html"><button data-turbo-action="replace">Same-origin data-turbo-action=replace form</button></form></p>
Expand All @@ -31,6 +32,30 @@ <h1>Navigation</h1>
<input type="hidden" name="path" value="/src/tests/fixtures/one.html">
<button data-turbo-action="replace">Same-origin form[method="post"] button[data-turbo-action="replace"]</button>
</form>

<p><a id="same-origin-replace-morph-link" href="/src/tests/fixtures/destination_for_morphing.html" data-turbo-action="replace" data-turbo-replace-method="morph">Same-origin data-turbo-action=replace data-turbo-replace-method=morph link</a></p>
<p><a id="same-origin-advance-morph-link" href="/src/tests/fixtures/destination_for_morphing.html" data-turbo-action="advance" data-turbo-replace-method="morph">Same-origin data-turbo-action=advance data-turbo-replace-method=morph link</a></p>
<p><form id="same-origin-replace-morph-form-get" action="/src/tests/fixtures/destination_for_morphing.html" data-turbo-action="replace" data-turbo-replace-method="morph"><button>Same-origin data-turbo-action=replace data-turbo-replace-method=morph form</button></form></p>
<p><form id="same-origin-advance-morph-form-get" action="/src/tests/fixtures/destination_for_morphing.html" data-turbo-action="advance" data-turbo-replace-method="morph"><button>Same-origin data-turbo-action=advance data-turbo-replace-method=morph form</button></form></p>
<p><form id="same-origin-replace-morph-form-submitter-get" action="/src/tests/fixtures/destination_for_morphing.html"><button data-turbo-action="replace" data-turbo-replace-method="morph">Same-origin data-turbo-action=replace data-turbo-replace-method=morph form</button></form></p>
<p><form id="same-origin-advance-morph-form-submitter-get" action="/src/tests/fixtures/destination_for_morphing.html"><button data-turbo-action="advance" data-turbo-replace-method="morph">Same-origin data-turbo-action=advance data-turbo-replace-method=morph form</button></form></p>
<form id="same-origin-replace-morph-form-post" method="post" action="/__turbo/redirect" data-turbo-action="replace" data-turbo-replace-method="morph">
<input type="hidden" name="path" value="/src/tests/fixtures/destination_for_morphing.html">
<button>Same-origin form[method="post"][data-turbo-action="replace"][data-turbo-replace-method="morph"]</button>
</form>
<form id="same-origin-advance-morph-form-post" method="post" action="/__turbo/redirect" data-turbo-action="advance" data-turbo-replace-method="morph">
<input type="hidden" name="path" value="/src/tests/fixtures/destination_for_morphing.html">
<button>Same-origin form[method="post"][data-turbo-action="advance"][data-turbo-replace-method="morph"]</button>
</form>
<form id="same-origin-replace-morph-form-submitter-post" method="post" action="/__turbo/redirect">
<input type="hidden" name="path" value="/src/tests/fixtures/destination_for_morphing.html">
<button data-turbo-action="replace" data-turbo-replace-method="morph">Same-origin form[method="post"] button[data-turbo-action="replace"][data-turbo-replace-method="morph"]</button>
</form>
<form id="same-origin-advance-morph-form-submitter-post" method="post" action="/__turbo/redirect">
<input type="hidden" name="path" value="/src/tests/fixtures/destination_for_morphing.html">
<button data-turbo-action="advance" data-turbo-advance-method="morph">Same-origin form[method="post"] button[data-turbo-action="advance"][data-turbo-replace-method="morph"]</button>
</form>

<form id="form-post" method="post" action="/__turbo/redirect">
<input type="hidden" name="path" value="/src/tests/fixtures/one.html">
<input type="submit" id="form-post-submit" value="Submit"/>
Expand Down
3 changes: 2 additions & 1 deletion src/tests/fixtures/page_refresh.html
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ <h3>Element with Stimulus controller</h3>
</label>
<button>Form with params to refresh the page</button>
</form>
<p><a id="replace-link" data-turbo-action="replace" href="/src/tests/fixtures/page_refresh.html?param=something">Link with params to refresh the page</a></p>
<p><a id="replace-link" data-turbo-action="replace" href="/src/tests/fixtures/page_refresh.html?tab=something">Link with params should not refresh the page</a></p>
<p><a id="replace-link-method-morph" data-turbo-action="replace" data-turbo-replace-method="morph" href="/src/tests/fixtures/page_refresh.html?tab=something">Link with different params will morph if specified</a></p>
<p><a id="refresh-link" data-turbo-action="replace" href="/src/tests/fixtures/page_refresh.html">Link to the same page</a></p>
<p><a id="link" href="/src/tests/fixtures/one.html">Link to another page</a></p>

Expand Down
Loading