Disable Back and Forward when they lead nowhere

Fixes f18b808b76f9528e406f53f70221e1a39b4664c8

src/App.svelte

@@ -98,11 +98,19 @@
     },
     {
       shortcut: "back",
-      run: () => window.history.back(),
+      run: () => {
+        if (get(router.canGoBack)) {
+          window.history.back();
+        }
+      },
     },
     {
       shortcut: "forward",
-      run: () => window.history.forward(),
+      run: () => {
+        if (get(router.canGoForward)) {
+          window.history.forward();
+        }
+      },
     },
     {
       shortcut: "reload",

src/components/AppSidebar.svelte

@@ -316,6 +316,8 @@
   });
 
   const activeRoute = router.activeRouteStore;
+  const canGoBack = router.canGoBack;
+  const canGoForward = router.canGoForward;
 
   function isInbox(): boolean {
     return $activeRoute.resource === "inbox";
@@ -514,6 +516,9 @@
     color: var(--color-text-tertiary);
     flex-shrink: 0;
   }
+  .window-controls :global(.button.disabled .icon) {
+    color: var(--color-text-quaternary);
+  }
   .update-badge {
     margin-left: auto;
     font: var(--txt-body-s-regular);
@@ -614,6 +619,7 @@
               title="{collapsed.value
                 ? 'Expand'
                 : 'Collapse'} sidebar ({toggleShortcut})"
+              ariaLabel="{collapsed.value ? 'Expand' : 'Collapse'} sidebar"
               keyShortcuts={ariaKeyShortcuts("toggleSidebar")}
               onclick={toggleSidebar}
               styleWidth={isMini ? "2rem" : "1.5rem"}
@@ -623,6 +629,9 @@
             </Button>
             <Button
               variant="naked"
+              title="Back"
+              ariaLabel="Back"
+              disabled={!$canGoBack}
               onclick={() => window.history.back()}
               styleWidth={isMini ? "2rem" : "1.5rem"}
               styleHeight={isMini ? "2rem" : "1.5rem"}
@@ -631,6 +640,9 @@
             </Button>
             <Button
               variant="naked"
+              title="Forward"
+              ariaLabel="Forward"
+              disabled={!$canGoForward}
               onclick={() => window.history.forward()}
               styleWidth={isMini ? "2rem" : "1.5rem"}
               styleHeight={isMini ? "2rem" : "1.5rem"}
@@ -640,6 +652,7 @@
             <Button
               variant="naked"
               title="Reload"
+              ariaLabel="Reload"
               onclick={() => window.location.reload()}
               styleWidth={isMini ? "2rem" : "1.5rem"}
               styleHeight={isMini ? "2rem" : "1.5rem"}

src/components/Button.svelte

@@ -14,6 +14,7 @@
     flatRight?: boolean;
     bordered?: boolean;
     title?: string;
+    ariaLabel?: string;
     styleHeight?: "1.5rem" | "1.75rem" | "2rem" | "2.5rem";
     styleWidth?: string;
     styleJustifyContent?: string;
@@ -32,6 +33,7 @@
     flatRight = false,
     bordered = false,
     title,
+    ariaLabel,
     styleHeight = "2rem",
     styleWidth = undefined,
     styleJustifyContent = undefined,
@@ -184,7 +186,9 @@
   style:--color-text={colors[variant]}
   style:--color-text-hover={colorsHover[variant]}
   style:--color-text-active={colorsActive[variant]}
+  aria-label={ariaLabel}
   aria-keyshortcuts={keyShortcuts}
+  aria-disabled={disabled || undefined}
   onclick={!disabled ? onclick : undefined}
   role="button"
   tabindex="0"

src/lib/router.ts

@@ -17,6 +17,32 @@ export const activeUnloadedRouteStore = writable<Route>(InitialStore);
 
 let currentUrl: URL | undefined;
 
+const HISTORY_INDEX = "radicleHistoryIndex";
+
+let historyDepth = 0;
+
+export const canGoBack = writable<boolean>(false);
+export const canGoForward = writable<boolean>(false);
+
+function historyIndex(): number {
+  const state: unknown = window.history.state;
+  const index =
+    state && typeof state === "object"
+      ? (state as Record<string, unknown>)[HISTORY_INDEX]
+      : undefined;
+  return typeof index === "number" ? index : 0;
+}
+
+function stamp(route: Route, index: number): unknown {
+  return { ...route, [HISTORY_INDEX]: index };
+}
+
+function updateNavigationState(index: number, truncated = false): void {
+  historyDepth = truncated ? index : Math.max(historyDepth, index);
+  canGoBack.set(index > 0);
+  canGoForward.set(index < historyDepth);
+}
+
 // Set while a back/forward (popstate) navigation is in flight, then frozen onto
 // `historyNavigation` for the route that becomes active, so views can choose to
 // restore prior scroll state only for history navigations.
@@ -50,6 +76,7 @@ async function navigateToUrl(
     // same-URL popstate would leak it onto the next unrelated navigation,
     // which would then wrongly restore cached list state.
     pendingHistoryNavigation = false;
+    updateNavigationState(historyIndex());
     return;
   }
 
@@ -103,13 +130,20 @@ async function navigate(
 
   if (
     action === "push" &&
+    path !== "" &&
     path !== window.location.pathname + window.location.search
   ) {
     // Pushing the route that is already active would mint a duplicate
-    // history entry, making Back appear to do nothing.
-    window.history.pushState(newRoute, "", path);
+    // history entry, making Back appear to do nothing. Booting has no path.
+    const index = historyIndex() + 1;
+    window.history.pushState(stamp(newRoute, index), "", path);
+    updateNavigationState(index, true);
   } else if (action === "replace") {
-    window.history.replaceState(newRoute, "");
+    const index = historyIndex();
+    window.history.replaceState(stamp(newRoute, index), "");
+    updateNavigationState(index);
+  } else {
+    updateNavigationState(historyIndex());
   }
   currentUrl = new URL(window.location.href);
   const currentLoadedRoute = get(activeRouteStore);

tests/e2e/navigation.spec.ts

@@ -0,0 +1,117 @@
+import type { Page } from "@playwright/test";
+
+import { expect, test } from "@tests/support/fixtures.js";
+
+function backButton(page: Page) {
+  return page.getByRole("button", { name: "Back", exact: true });
+}
+
+function forwardButton(page: Page) {
+  return page.getByRole("button", { name: "Forward", exact: true });
+}
+
+async function expectInbox(page: Page) {
+  await expect(page).toHaveURL(/\/inbox$/);
+  await expect(page.getByRole("link", { name: "Issues" })).toBeHidden();
+}
+
+async function expectRepo(page: Page) {
+  await expect(page).toHaveURL(/\/repos\/rad:[^/]+\/home$/);
+  await expect(page.getByRole("link", { name: "Issues" })).toBeVisible();
+}
+
+async function expectIssues(page: Page) {
+  await expect(page).toHaveURL(/\/issues/);
+  await expect(page.getByText("This title has **markdown**")).toBeVisible();
+}
+
+async function openRepo(page: Page) {
+  await page.getByRole("link", { name: "cobs" }).click();
+  await expectRepo(page);
+}
+
+async function openIssues(page: Page) {
+  await page.getByRole("link", { name: "Issues" }).click();
+  await expectIssues(page);
+}
+
+async function launch(page: Page) {
+  await page.addInitScript(() =>
+    localStorage.setItem("appFirstLaunch", "false"),
+  );
+  await page.goto("/inbox");
+  await expect(page.getByRole("link", { name: "cobs" })).toBeVisible();
+}
+
+test("back and forward are disabled on launch", async ({ page }) => {
+  await launch(page);
+  await expect(backButton(page)).toBeDisabled();
+  await expect(forwardButton(page)).toBeDisabled();
+});
+
+test("back and forward follow the history", async ({ page }) => {
+  await launch(page);
+  await openRepo(page);
+  await expect(backButton(page)).toBeEnabled();
+  await expect(forwardButton(page)).toBeDisabled();
+
+  await backButton(page).click();
+  await expectInbox(page);
+  await expect(backButton(page)).toBeDisabled();
+  await expect(forwardButton(page)).toBeEnabled();
+
+  await forwardButton(page).click();
+  await expectRepo(page);
+  await expect(backButton(page)).toBeEnabled();
+  await expect(forwardButton(page)).toBeDisabled();
+});
+
+test("navigating after going back discards forward entries", async ({
+  page,
+}) => {
+  await launch(page);
+  await openRepo(page);
+  await openIssues(page);
+  await backButton(page).click();
+  await expectRepo(page);
+  await backButton(page).click();
+  await expectInbox(page);
+  await expect(forwardButton(page)).toBeEnabled();
+
+  await openRepo(page);
+  await expect(backButton(page)).toBeEnabled();
+  await expect(forwardButton(page)).toBeDisabled();
+});
+
+test("history position survives a reload", async ({ page }) => {
+  await launch(page);
+  await openRepo(page);
+  await page.waitForLoadState("networkidle");
+
+  await page.reload();
+  await expectRepo(page);
+  await expect(backButton(page)).toBeEnabled();
+
+  await backButton(page).click();
+  await expectInbox(page);
+  await expect(backButton(page)).toBeDisabled();
+});
+
+test("shortcuts stay within the app's history", async ({ page }) => {
+  await launch(page);
+  const mac = await page.evaluate(
+    () =>
+      navigator.platform.includes("Mac") ||
+      navigator.userAgent.includes("OS X"),
+  );
+  const backShortcut = mac ? "Meta+BracketLeft" : "Alt+ArrowLeft";
+  const forwardShortcut = mac ? "Meta+BracketRight" : "Alt+ArrowRight";
+  await openRepo(page);
+
+  await page.keyboard.press(backShortcut);
+  await expectInbox(page);
+
+  await page.keyboard.press(backShortcut);
+  await page.keyboard.press(forwardShortcut);
+  await expectRepo(page);
+});