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 InstantClick behavior #1101

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
273456a
Move doesNotTargetIFrame to util.js
davidalejandroaguilar Dec 5, 2023
1df1428
Move findLinkFromClickTarget to util.js
davidalejandroaguilar Dec 5, 2023
2f8c272
Move getLocationForLink to util.js
davidalejandroaguilar Dec 5, 2023
de3ffcf
Allow request to be intercepted and overriden on turbo:before-fetch-r…
davidalejandroaguilar Dec 3, 2023
94ff0e1
Add instantclick behavior
davidalejandroaguilar Dec 5, 2023
e662690
Allow customizing the event that triggers prefetching
davidalejandroaguilar Dec 6, 2023
15517d4
Allow customizing the cache time for prefetching
davidalejandroaguilar Dec 6, 2023
ab0e538
Rename LinkPrefetchOnMouseoverObserver to LinkPrefetchObserver
davidalejandroaguilar Dec 6, 2023
fca147e
Use private methods in LinkPrefetchObserver
davidalejandroaguilar Dec 6, 2023
3baf81c
Reorganize methods on LinkPrefetchObserver
davidalejandroaguilar Dec 6, 2023
d2ec021
Require a shorter sleep time in the test
davidalejandroaguilar Dec 6, 2023
7c7ded9
Standardize anchor IDs in link_prefetch_observer_tests
davidalejandroaguilar Dec 6, 2023
806aa9d
Don't try traverse DOM to determine if the target is a link
davidalejandroaguilar Dec 9, 2023
39e3dda
Keep the closing tag on the same line as the rest of the tag
davidalejandroaguilar Dec 9, 2023
9cdff6d
Remove unnecessary nesting in tests
davidalejandroaguilar Dec 9, 2023
368225a
Add missing newline at end of file
davidalejandroaguilar Dec 9, 2023
3e75161
Check for prefetch meta tag before prefetching (on hover event)
davidalejandroaguilar Dec 9, 2023
b9e82f2
Use FetchRequest to build request for LinkPrefetchObserver
davidalejandroaguilar Dec 9, 2023
ab01099
Add Turbo Stream header to Accept header when link has data-turbo-stream
davidalejandroaguilar Dec 9, 2023
c735036
Bring back prefetching links with inner elements
davidalejandroaguilar Dec 9, 2023
b27bc42
Add cancelable delay to prefetching links on hover
davidalejandroaguilar Dec 9, 2023
4c86295
Fix clearing cache on every prefetch after b9e82f2
davidalejandroaguilar Dec 9, 2023
4170ddb
Add tests for the delay on the meta tag
davidalejandroaguilar Dec 9, 2023
5078e0b
Use mouseenter and mouseleave instead of mouseover and mouseout
davidalejandroaguilar Dec 9, 2023
3d2665a
Remove unneeded comment
davidalejandroaguilar Dec 9, 2023
3a724ca
Merge branch 'main' into davidramos-add-instantclick-behavior
davidalejandroaguilar Dec 11, 2023
efd58fb
Use double quotes instead of single quotes for consistency
davidalejandroaguilar Jan 7, 2024
10e1311
Move link variable declaration inside if statement
davidalejandroaguilar Jan 7, 2024
5326a0f
Use correct key name for mouseenter event on LinkPrefetchObserver.tri…
davidalejandroaguilar Jan 7, 2024
a551b1f
Allow prefetching when visiting page without meta, then visiting one …
davidalejandroaguilar Jan 7, 2024
ebea951
Allow create and delete posts with comments on the test server
davidalejandroaguilar Jan 7, 2024
aa6f013
Clear prefetch cache after form submission
davidalejandroaguilar Jan 8, 2024
851dabb
Add test for nested data-turbo-prefetch=true within data-turbo-prefet…
davidalejandroaguilar Jan 16, 2024
ccab6b2
No longer allow customizing the prefetch trigger event
davidalejandroaguilar Jan 16, 2024
06181be
No longer allow customizing the prefetch delay
davidalejandroaguilar Jan 16, 2024
0704cef
Add touchstart event to prefetch observer
davidalejandroaguilar Jan 16, 2024
80524d5
Merge branch 'main' into davidramos-add-instantclick-behavior
davidalejandroaguilar Jan 16, 2024
c975a7b
Fix flaky tests
davidalejandroaguilar Jan 16, 2024
e87bf75
Use double quotes instead of single quotes
davidalejandroaguilar Jan 16, 2024
1b376c8
Only cache the link you're currently hovering
afcapel Jan 17, 2024
c944326
Remove unused files after ETA template rendered removal
davidalejandroaguilar Jan 18, 2024
63908d0
Remove unused variable
davidalejandroaguilar Jan 18, 2024
1c9b41c
Clear prefetch cache when the link is no longer hovered
davidalejandroaguilar Jan 18, 2024
ac310e2
Merge remote-tracking branch 'origin/main' into davidramos-add-instan…
afcapel Jan 22, 2024
70935c4
Style changes
afcapel Jan 22, 2024
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
6 changes: 4 additions & 2 deletions playwright.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ const config = {
...devices["Desktop Chrome"],
contextOptions: {
timeout: 60000
}
},
hasTouch: true
}
},
{
Expand All @@ -17,7 +18,8 @@ const config = {
...devices["Desktop Firefox"],
contextOptions: {
timeout: 60000
}
},
hasTouch: 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.

Decided to enable touchscreen for existing devices, instead of adding a new device (e.g. iPhone 13), because we could still test the screen taps while avoiding extra test runs on another device.

}
}
],
Expand Down
10 changes: 9 additions & 1 deletion src/core/drive/form_submission.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { FetchRequest, FetchMethod, fetchMethodFromString, fetchEnctypeFromStrin
import { expandURL } from "../url"
import { clearBusyState, dispatch, getAttribute, getMetaContent, hasAttribute, markAsBusy } from "../../util"
import { StreamMessage } from "../streams/stream_message"
import { prefetchCache } from "./prefetch_cache"

export const FormSubmissionState = {
initialized: "initialized",
Expand Down Expand Up @@ -126,13 +127,20 @@ export class FormSubmission {
}

requestPreventedHandlingResponse(request, response) {
prefetchCache.clear()

this.result = { success: response.succeeded, fetchResponse: response }
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even if the event is default prevented after a form submission, I think we should still clear the cache to avoid showing stale content.


requestSucceededWithResponse(request, response) {
if (response.clientError || response.serverError) {
this.delegate.formSubmissionFailedWithResponse(this, response)
} else if (this.requestMustRedirect(request) && responseSucceededWithoutRedirect(response)) {
return
}

prefetchCache.clear()

if (this.requestMustRedirect(request) && responseSucceededWithoutRedirect(response)) {
const error = new Error("Form responses must redirect to another location")
this.delegate.formSubmissionErrored(this, error)
} else {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there's a client or server error, we don't clear the cache, as we can assume no mutation happened.

Expand Down
34 changes: 34 additions & 0 deletions src/core/drive/prefetch_cache.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
const PREFETCH_DELAY = 100

class PrefetchCache {
#prefetchTimeout = null
#prefetched = null

get(url) {
if (this.#prefetched && this.#prefetched.url === url && this.#prefetched.expire > Date.now()) {
return this.#prefetched.request
}
}

setLater(url, request, ttl) {
this.clear()

this.#prefetchTimeout = setTimeout(() => {
request.perform()
this.set(url, request, ttl)
this.#prefetchTimeout = null
}, PREFETCH_DELAY)
}

set(url, request, ttl) {
this.#prefetched = { url, request, expire: new Date(new Date().getTime() + ttl) }
}

clear() {
if (this.#prefetchTimeout) clearTimeout(this.#prefetchTimeout)
this.#prefetched = null
}
}

export const cacheTtl = 10 * 1000
export const prefetchCache = new PrefetchCache()
13 changes: 13 additions & 0 deletions src/core/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { CacheObserver } from "../observers/cache_observer"
import { FormSubmitObserver } from "../observers/form_submit_observer"
import { FrameRedirector } from "./frames/frame_redirector"
import { History } from "./drive/history"
import { LinkPrefetchObserver } from "../observers/link_prefetch_observer"
import { LinkClickObserver } from "../observers/link_click_observer"
import { FormLinkClickObserver } from "../observers/form_link_click_observer"
import { getAction, expandURL, locationIsVisitable } from "./url"
Expand All @@ -26,6 +27,7 @@ export class Session {

pageObserver = new PageObserver(this)
cacheObserver = new CacheObserver()
linkPrefetchObserver = new LinkPrefetchObserver(this, document)
linkClickObserver = new LinkClickObserver(this, window)
formSubmitObserver = new FormSubmitObserver(this, document)
scrollObserver = new ScrollObserver(this)
Expand Down Expand Up @@ -53,6 +55,7 @@ export class Session {
if (!this.started) {
this.pageObserver.start()
this.cacheObserver.start()
this.linkPrefetchObserver.start()
this.formLinkClickObserver.start()
this.linkClickObserver.start()
this.formSubmitObserver.start()
Expand All @@ -74,6 +77,7 @@ export class Session {
if (this.started) {
this.pageObserver.stop()
this.cacheObserver.stop()
this.linkPrefetchObserver.stop()
this.formLinkClickObserver.stop()
this.linkClickObserver.stop()
this.formSubmitObserver.stop()
Expand Down Expand Up @@ -199,6 +203,15 @@ export class Session {

submittedFormLinkToLocation() {}

// Link hover observer delegate

canPrefetchRequestToLocation(link, location) {
return (
this.elementIsNavigatable(link) &&
locationIsVisitable(location, this.snapshot.rootLocation)
)
}

// Link click observer delegate

willFollowLinkToLocation(link, location, event) {
Expand Down
13 changes: 11 additions & 2 deletions src/http/fetch_request.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,17 @@ export class FetchRequest {
async perform() {
const { fetchOptions } = this
this.delegate.prepareRequest(this)
await this.#allowRequestToBeIntercepted(fetchOptions)
const event = await this.#allowRequestToBeIntercepted(fetchOptions)
try {
this.delegate.requestStarted(this)
const response = await fetch(this.url.href, fetchOptions)

if (event.detail.fetchRequest) {
this.response = event.detail.fetchRequest.response
} else {
this.response = fetch(this.url.href, fetchOptions)
}

const response = await this.response
return await this.receive(response)
} catch (error) {
if (error.name !== "AbortError") {
Expand Down Expand Up @@ -186,6 +193,8 @@ export class FetchRequest {
})
this.url = event.detail.url
if (event.defaultPrevented) await requestInterception

return event
}

#willDelegateErrorHandling(error) {
Expand Down
10 changes: 10 additions & 0 deletions src/observers/form_link_click_observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ export class FormLinkClickObserver {
this.linkInterceptor.stop()
}

// Link hover observer delegate

canPrefetchRequestToLocation(link, location) {
return false
}

prefetchAndCacheRequestToLocation(link, location) {
return
}

// Link click observer delegate

willFollowLinkToLocation(link, location, originalEvent) {
Expand Down
27 changes: 3 additions & 24 deletions src/observers/link_click_observer.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { expandURL } from "../core/url"
import { findClosestRecursively } from "../util"
import { doesNotTargetIFrame, findLinkFromClickTarget, getLocationForLink } from "../util"

export class LinkClickObserver {
started = false
Expand Down Expand Up @@ -31,9 +30,9 @@ export class LinkClickObserver {
clickBubbled = (event) => {
if (event instanceof MouseEvent && this.clickEventIsSignificant(event)) {
const target = (event.composedPath && event.composedPath()[0]) || event.target
const link = this.findLinkFromClickTarget(target)
const link = findLinkFromClickTarget(target)
if (link && doesNotTargetIFrame(link)) {
const location = this.getLocationForLink(link)
const location = getLocationForLink(link)
if (this.delegate.willFollowLinkToLocation(link, location, event)) {
event.preventDefault()
this.delegate.followedLinkToLocation(link, location)
Expand All @@ -53,24 +52,4 @@ export class LinkClickObserver {
event.shiftKey
)
}

findLinkFromClickTarget(target) {
return findClosestRecursively(target, "a[href]:not([target^=_]):not([download])")
}

getLocationForLink(link) {
return expandURL(link.getAttribute("href") || "")
}
}

function doesNotTargetIFrame(anchor) {
if (anchor.hasAttribute("target")) {
for (const element of document.getElementsByName(anchor.target)) {
if (element instanceof HTMLIFrameElement) return false
}

return true
} else {
return true
}
}
Loading