From 2ac08937655e943f42b4b322ae43264766995902 Mon Sep 17 00:00:00 2001
From: Sean Doyle <seanpdoyle@users.noreply.github.com>
Date: Fri, 17 Nov 2023 11:10:56 -0500
Subject: [PATCH] Hide Progress bar on `turbo:load` (#1036)

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       | 11 +++++++----
 src/tests/fixtures/navigation.html       |  1 +
 src/tests/functional/navigation_tests.js | 19 ++++++++++++-------
 src/tests/server.mjs                     |  3 ++-
 4 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/src/core/native/browser_adapter.js b/src/core/native/browser_adapter.js
index 15da5dd41..6f4c73bfe 100644
--- a/src/core/native/browser_adapter.js
+++ b/src/core/native/browser_adapter.js
@@ -54,18 +54,21 @@ export class BrowserAdapter {
     }
   }
 
-  visitRequestFinished(_visit) {
+  visitRequestFinished(_visit) {}
+
+  visitCompleted(_visit) {
     this.progressBar.setValue(1)
     this.hideVisitProgressBar()
   }
 
-  visitCompleted(_visit) {}
-
   pageInvalidated(reason) {
     this.reload(reason)
   }
 
-  visitFailed(_visit) {}
+  visitFailed(_visit) {
+    this.progressBar.setValue(1)
+    this.hideVisitProgressBar()
+  }
 
   visitRendered(_visit) {}
 
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 e5c3986f8..73871e1c2 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("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("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,14 @@ test("navigating does not render a progress bar before expiring the delay", asyn
   assert.notOk(await hasSelector(page, ".turbo-progress-bar"), "does not show progress bar before delay")
 })
 
+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")
+  await waitUntilNoSelector(page, ".turbo-progress-bar")
+})
+
 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 0d35cb57d..5cb585b4c 100644
--- a/src/tests/server.mjs
+++ b/src/tests/server.mjs
@@ -79,8 +79,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) => {