From 1aa94c42d57943dc650ef9bce2460f67a4dd76c2 Mon Sep 17 00:00:00 2001
From: Sean Doyle <sean.p.doyle24@gmail.com>
Date: Thu, 12 Oct 2023 14:28:44 -0400
Subject: [PATCH] Hide Progress bar on `turbo:load`

Closes [#962][]

Prior to this commit, the progress bar is hidden between the Turbo Drive
visits' `turbo:before-fetch-response` event and the `turbo:render`
event.

This commit changes that behavior to hide the bar when the Visit is
complete (some time prior to the `turbo:load` event).

[#962]: https://github.com/hotwired/turbo/issues/962
---
 src/core/native/browser_adapter.js       |  6 +++---
 src/tests/fixtures/navigation.html       |  1 +
 src/tests/functional/navigation_tests.js | 27 ++++++++++++++++++------
 src/tests/server.mjs                     |  3 ++-
 4 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/core/native/browser_adapter.js b/src/core/native/browser_adapter.js
index c3f1086de..3ccfeb8d9 100644
--- a/src/core/native/browser_adapter.js
+++ b/src/core/native/browser_adapter.js
@@ -45,13 +45,13 @@ export class BrowserAdapter {
     }
   }
 
-  visitRequestFinished(_visit) {
+  visitRequestFinished(_visit) {}
+
+  visitCompleted(_visit) {
     this.progressBar.setValue(1)
     this.hideVisitProgressBar()
   }
 
-  visitCompleted(_visit) {}
-
   pageInvalidated(reason) {
     this.reload(reason)
   }
diff --git a/src/tests/fixtures/navigation.html b/src/tests/fixtures/navigation.html
index f2fd53d86..12b0bc537 100644
--- a/src/tests/fixtures/navigation.html
+++ b/src/tests/fixtures/navigation.html
@@ -80,6 +80,7 @@ <h1>Navigation</h1>
         </turbo-toggle>
       </p>
       <p><a id="delayed-link" href="/__turbo/delayed_response">Delayed link</a></p>
+      <p><a id="delayed-failure-link" href="/__turbo/delayed_response?status=500">Delayed failure link</a></p>
       <p><a id="link-target-iframe" href="/src/tests/fixtures/one.html" target="iframe">Targets iframe[name="iframe"]</a></p>
       <p><a id="link-target-empty-name-iframe" href="/src/tests/fixtures/one.html" target="">Targets iframe[name=""]</a></p>
       <p>
diff --git a/src/tests/functional/navigation_tests.js b/src/tests/functional/navigation_tests.js
index c4263d0f3..c4435dd7a 100644
--- a/src/tests/functional/navigation_tests.js
+++ b/src/tests/functional/navigation_tests.js
@@ -27,19 +27,16 @@ test.beforeEach(async ({ page }) => {
   await readEventLogs(page)
 })
 
-test("test navigating renders a progress bar", async ({ page }) => {
-  assert.equal(
-    await page.locator("style").evaluate((style) => style.nonce),
-    "123",
-    "renders progress bar stylesheet inline with nonce"
-  )
-
+test("test navigating renders a progress bar until the next turbo:load", async ({ page }) => {
   await page.evaluate(() => window.Turbo.setProgressBarDelay(0))
   await page.click("#delayed-link")
 
   await waitUntilSelector(page, ".turbo-progress-bar")
   assert.ok(await hasSelector(page, ".turbo-progress-bar"), "displays progress bar")
 
+  await nextEventNamed(page, "turbo:render")
+  assert.ok(await hasSelector(page, ".turbo-progress-bar"), "displays progress bar")
+
   await nextEventNamed(page, "turbo:load")
   await waitUntilNoSelector(page, ".turbo-progress-bar")
 
@@ -53,6 +50,22 @@ test("test navigating does not render a progress bar before expiring the delay",
   assert.notOk(await hasSelector(page, ".turbo-progress-bar"), "does not show progress bar before delay")
 })
 
+test("test navigating hides the progress bar on failure", async ({ page }) => {
+  await page.evaluate(() => window.Turbo.setProgressBarDelay(0))
+  await page.click("#delayed-failure-link")
+
+  await waitUntilSelector(page, ".turbo-progress-bar")
+  assert.ok(await hasSelector(page, ".turbo-progress-bar"), "displays progress bar")
+
+  await nextEventNamed(page, "turbo:render")
+  assert.ok(await hasSelector(page, ".turbo-progress-bar"), "displays progress bar")
+
+  await nextEventNamed(page, "turbo:load")
+  await waitUntilNoSelector(page, ".turbo-progress-bar")
+
+  assert.notOk(await hasSelector(page, ".turbo-progress-bar"), "hides progress bar")
+})
+
 test("test after loading the page", async ({ page }) => {
   assert.equal(pathname(page.url()), "/src/tests/fixtures/navigation.html")
   assert.equal(await visitAction(page), "load")
diff --git a/src/tests/server.mjs b/src/tests/server.mjs
index 0628f2e30..4c777f87b 100644
--- a/src/tests/server.mjs
+++ b/src/tests/server.mjs
@@ -74,8 +74,9 @@ router.get("/headers", (request, response) => {
 })
 
 router.get("/delayed_response", (request, response) => {
+  const { status } = request.query
   const fixture = path.join(__dirname, "../../src/tests/fixtures/one.html")
-  setTimeout(() => response.status(200).sendFile(fixture), 1000)
+  setTimeout(() => response.status(parseInt(status || "200")).sendFile(fixture), 1000)
 })
 
 router.post("/messages", (request, response) => {