Skip to content

Commit

Permalink
data-turbo-stream links should not set aria-busy (#681)
Browse files Browse the repository at this point in the history
Normally when following a link Turbo sets `[aria-busy="true"]` on the
document element. When the visit is complete, that attribute is removed.

However, when using `[data-turbo-stream]` to request a stream response
to a link, there is no visit to complete. That presented as a bug where
`aria-busy` was set at the start of the operation, but never cleared.

To solve this, we avoid setting the `aria-busy` attribute for links that
have a `data-turbo-stream` attribute.
  • Loading branch information
kevinmcconnell authored Aug 16, 2022
1 parent 256418f commit 0b606ef
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 4 deletions.
6 changes: 4 additions & 2 deletions src/core/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,17 @@ export class Session
}

visitStarted(visit: Visit) {
if (!visit.acceptsStreamResponse) {
markAsBusy(document.documentElement)
}
extendURLWithDeprecatedProperties(visit.location)
if (!visit.silent) {
this.notifyApplicationAfterVisitingLocation(visit.location, visit.action)
}
}

visitCompleted(visit: Visit) {
clearBusyState(document.documentElement)
this.notifyApplicationAfterPageLoad(visit.getTimingMetrics())
}

Expand Down Expand Up @@ -346,7 +350,6 @@ export class Session
}

notifyApplicationAfterVisitingLocation(location: URL, action: Action) {
markAsBusy(document.documentElement)
return dispatch<TurboVisitEvent>("turbo:visit", { detail: { url: location.href, action } })
}

Expand All @@ -366,7 +369,6 @@ export class Session
}

notifyApplicationAfterPageLoad(timing: TimingData = {}) {
clearBusyState(document.documentElement)
return dispatch<TurboLoadEvent>("turbo:load", {
detail: { url: this.location.href, timing },
})
Expand Down
6 changes: 4 additions & 2 deletions src/tests/fixtures/visit.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<!DOCTYPE html>
<html>
<html id="html">
<head>
<meta charset="utf-8">
<title>Turbo</title>
Expand All @@ -19,7 +19,9 @@ <h1>Visit</h1>
<hr class="push-below-fold">
<p><a id="below-the-fold-link" href="/src/tests/fixtures/one.html">one.html</a></p>
<hr class="push-below-fold">
<p><a id="stream-link" href="/src/tests/fixtures/one.html?key=value" data-turbo-stream>Stream link with ?key=value</a></p>
<p><a id="stream-link" href="/__turbo/stream-response?content=link&type=stream&key=value" data-turbo-stream>Stream link with ?key=value</a></p>
</section>

<div id="messages"></div>
</body>
</html>
10 changes: 10 additions & 0 deletions src/tests/functional/visit_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
isScrolledToTop,
nextBeat,
nextEventNamed,
noNextAttributeMutationNamed,
readEventLogs,
scrollToSelector,
visitAction,
Expand Down Expand Up @@ -187,6 +188,15 @@ test("test visits with data-turbo-stream include MIME type & search params", asy
assert.equal(getSearchParam(url, "key"), "value")
})

test("test visits with data-turbo-stream do not set aria-busy", async ({ page }) => {
await page.click("#stream-link")

assert.ok(
await noNextAttributeMutationNamed(page, "html", "aria-busy"),
"never sets [aria-busy] on the document element"
)
})

test("test cache does not override response after redirect", async ({ page }) => {
await page.evaluate(() => {
const cachedElement = document.createElement("some-cached-element")
Expand Down
9 changes: 9 additions & 0 deletions src/tests/helpers/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,15 @@ export async function nextAttributeMutationNamed(
return attributeValue
}

export async function noNextAttributeMutationNamed(
page: Page,
elementId: string,
attributeName: string
): Promise<boolean> {
const records = await readMutationLogs(page, 1)
return !records.some(([name]) => name == attributeName)
}

export async function noNextEventNamed(page: Page, eventName: string): Promise<boolean> {
const records = await readEventLogs(page, 1)
return !records.some(([name]) => name == eventName)
Expand Down
11 changes: 11 additions & 0 deletions src/tests/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,17 @@ router.post("/messages", (request, response) => {
}
})

router.get("/stream-response", (request, response) => {
const params = { ...request.body, ...request.query }
const { content, target, targets } = params
if (acceptsStreams(request)) {
response.type("text/vnd.turbo-stream.html; charset=utf-8")
response.send(targets ? renderMessageForTargets(content, targets) : renderMessage(content, target))
} else {
response.sendStatus(422)
}
})

router.put("/messages/:id", (request, response) => {
const { content, type } = request.body
const { id } = request.params
Expand Down

0 comments on commit 0b606ef

Please sign in to comment.