Merge bug/access-review-tab — access-requests UX + general back-navigation fixes

frontend/src/App.tsx

@@ -32,7 +32,7 @@ import Playlists from "./components/Playlists";
 import Stats from "./components/Stats";
 import Scheduler from "./components/Scheduler";
 import ConfigPanel from "./components/ConfigPanel";
-import AdminUsers from "./components/AdminUsers";
+import AdminUsers, { focusAccessRequestsTab } from "./components/AdminUsers";
 import SettingsPanel from "./components/SettingsPanel";
 import NotificationsPanel from "./components/NotificationsPanel";
 import Messages from "./components/Messages";
@@ -186,7 +186,8 @@ export default function App() {
       // Push an in-app history entry so the browser/mouse Back button steps through pages
       // instead of leaving the app (e.g. back to the OAuth redirect). The URL stays clean —
       // the page rides in history.state, not the query string (filters never go in the URL).
-      window.history.pushState({ ...window.history.state, sfPage: next }, "");
+      // A fresh { sfPage } (no carried-over _sub/_ov markers) so each page starts at its root.
+      window.history.pushState({ sfPage: next }, "");
     };
     // Guard leaving the Settings page with unsaved preference changes.
     if (page === "settings" && prefsDirtyRef.current) {
@@ -234,8 +235,8 @@ export default function App() {
   }, []); // eslint-disable-line react-hooks/exhaustive-deps
 
   // In-app Back/Forward: stamp the initial entry with the current page, then sync `page`
-  // from history.state on popstate. Runs after the strip-params effect above (which resets
-  // history.state to null), so the initial stamp isn't clobbered.
+  // from history.state on popstate. (stripUrlParams preserves history.state, so this stamp
+  // survives a later query-string strip.)
   useEffect(() => {
     window.history.replaceState(
       { ...window.history.state, sfPage: page },
@@ -288,6 +289,13 @@ export default function App() {
     refetchInterval: (query) => (query.state.data ? 60_000 : false),
   });
 
+  // Once signed in, drop any leftover query string from the pre-login flow (e.g.
+  // ?access=requested, ?verify, ?reset). The logged-in app reads nothing from the URL, so the
+  // address bar stays clean. Gated on auth so the logged-out Welcome page still reads its params.
+  useEffect(() => {
+    if (meQuery.data) stripUrlParams();
+  }, [meQuery.data?.id]); // eslint-disable-line react-hooks/exhaustive-deps
+
   // Any request that 401s means the session ended server-side. If we were signed in (cached
   // `me` exists), reload to the login page at once (option 2 — also covers any click that hits
   // the API). Guarded on cached `me` so the public login page's own /api/me 401 can't loop.
@@ -358,24 +366,6 @@ export default function App() {
       saveLayoutLocal(l);
     }
     if (isSupported(prefs.language)) setLanguage(prefs.language);
-    // Nudge admins when access requests are waiting (in lieu of a server-push bell).
-    const pending = meQuery.data?.pending_invites ?? 0;
-    if (meQuery.data?.role === "admin") {
-      // Keep a single, current nudge: drop any prior one (persisted copies reload as dismissed
-      // history, so they'd otherwise pile up one-per-reload) before re-issuing the live notice.
-      removeByMetaKind("access-requests");
-      if (pending > 0) {
-        notify({
-          level: "warning",
-          title: t("common.accessRequestsTitle"),
-          message: t("common.accessRequestsMessage", { count: pending }),
-          // Durable inbox link (survives reload, unlike a live action callback); the toast also
-          // gets a click via the action.
-          meta: { kind: "access-requests" },
-          action: { label: t("common.accessRequestsReview"), onClick: () => setPage("users") },
-        });
-      }
-    }
     // The demo account is shared: there are no subscriptions, so default it to the whole
     // library (the "my" feed would be empty). The communal-state warning is a permanent
     // banner (see DemoBanner below), not a toast that re-pops on every reload.
@@ -385,6 +375,35 @@ export default function App() {
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [meQuery.data?.id]);
 
+  // Nudge admins when access requests are waiting (in lieu of a server-push bell). Keyed on the
+  // pending COUNT so it re-resolves the moment an approval/denial changes it — not only on a
+  // fresh load (which is why the notice used to linger until F5 after an approval).
+  useEffect(() => {
+    if (meQuery.data?.role !== "admin") return;
+    // Keep a single, current nudge: drop any prior one (persisted copies reload as dismissed
+    // history, so they'd otherwise pile up one-per-reload) before re-issuing the live notice.
+    removeByMetaKind("access-requests");
+    const pending = meQuery.data.pending_invites ?? 0;
+    if (pending > 0) {
+      notify({
+        level: "warning",
+        title: t("common.accessRequestsTitle"),
+        message: t("common.accessRequestsMessage", { count: pending }),
+        // Durable inbox link (survives reload, unlike a live action callback); the toast also
+        // gets a click via the action.
+        meta: { kind: "access-requests" },
+        action: {
+          label: t("common.accessRequestsReview"),
+          onClick: () => {
+            focusAccessRequestsTab();
+            setPage("users");
+          },
+        },
+      });
+    }
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [meQuery.data?.role, meQuery.data?.pending_invites]);
+
   // Theme is part of the Settings draft: apply locally for preview, persist on Save.
   function setTheme(next: ThemePrefs) {
     setThemeState(next);

frontend/src/components/AdminUsers.tsx

@@ -11,9 +11,19 @@ import Tabs, { usePersistedTab } from "./Tabs";
 // Admin-only user & access management: roles, access requests (the Invite whitelist), and the
 // demo whitelist + reset. Each section is its own tab (persisted across reloads); the Access tab
 // carries a badge with the count of pending requests so it's visible without switching to it.
+// The persisted-tab storage key + the access-requests tab id, exported so a notification's
+// "Review" link can pre-select that tab before navigating here (usePersistedTab reads the key
+// on mount, and this page mounts fresh on navigation).
+export const ADMIN_USERS_TAB_KEY = "siftlode.adminUsersTab";
+export const ADMIN_USERS_ACCESS_TAB = "access";
+
+export function focusAccessRequestsTab(): void {
+  localStorage.setItem(ADMIN_USERS_TAB_KEY, ADMIN_USERS_ACCESS_TAB);
+}
+
 export default function AdminUsers({ me }: { me: Me }) {
   const { t } = useTranslation();
-  const [tab, setTab] = usePersistedTab("siftlode.adminUsersTab", "roles");
+  const [tab, setTab] = usePersistedTab(ADMIN_USERS_TAB_KEY, "roles");
   const tabs = [
     { id: "roles", label: t("users.tabs.roles") },
     { id: "access", label: t("users.tabs.access"), badge: me.pending_invites },

frontend/src/components/Messages.tsx

@@ -6,6 +6,7 @@ import { api, type Conversation, type MessageUser } from "../lib/api";
 import { useLiveQuery } from "../lib/useLiveQuery";
 import { relativeTime } from "../lib/format";
 import { partnerPub, SYSTEM_ID, useKeyState } from "../lib/messaging";
+import { useHistorySubview } from "../lib/history";
 import { openChat } from "../lib/chatDock";
 import * as e2ee from "../lib/e2ee";
 import Avatar from "./Avatar";
@@ -17,7 +18,9 @@ const POLL_MS = 20000;
 type View = "list" | "new" | { partnerId: number; name?: string; avatar?: string | null; system?: boolean };
 
 export default function Messages({ meId }: { meId: number }) {
-  const [view, setView] = useState<View>("list");
+  // Sub-views live in browser history, so Back returns to the conversation list before leaving
+  // the module (open() pushes an entry; back() = history.back()).
+  const { view, open, back } = useHistorySubview<View>("list");
 
   if (typeof view === "object") {
     return (
@@ -27,18 +30,18 @@ export default function Messages({ meId }: { meId: number }) {
         isSystem={!!view.system}
         seedName={view.name}
         seedAvatar={view.avatar}
-        onBack={() => setView("list")}
+        onBack={back}
       />
     );
   }
   if (view === "new") {
-    return <Directory onPick={(u) => setView({ partnerId: u.id, name: u.name, avatar: u.avatar_url })} onBack={() => setView("list")} />;
+    return <Directory onPick={(u) => open({ partnerId: u.id, name: u.name, avatar: u.avatar_url })} onBack={back} />;
   }
   return (
     <ConversationList
       meId={meId}
-      onOpen={(c) => setView({ partnerId: c.partner.id, name: c.partner.name, avatar: c.partner.avatar_url, system: c.partner.id === SYSTEM_ID })}
-      onNew={() => setView("new")}
+      onOpen={(c) => open({ partnerId: c.partner.id, name: c.partner.name, avatar: c.partner.avatar_url, system: c.partner.id === SYSTEM_ID })}
+      onNew={() => open("new")}
     />
   );
 }

frontend/src/components/Modal.tsx

@@ -1,6 +1,7 @@
 import { useEffect, useRef, type ReactNode } from "react";
 import { createPortal } from "react-dom";
 import { X } from "lucide-react";
+import { useBackToClose } from "../lib/history";
 
 // Stack of open modals so ESC only closes the topmost one — e.g. an error dialog over the
 // tag editor: pressing ESC dismisses just the error and returns to the editor underneath.
@@ -19,6 +20,9 @@ export default function Modal({
   children: ReactNode;
   maxWidth?: string;
 }) {
+  // Browser/mouse Back closes the topmost modal instead of navigating away.
+  useBackToClose(onClose);
+
   // Keep a stable handler across renders so the stack id is assigned once per mount.
   const onCloseRef = useRef(onClose);
   onCloseRef.current = onClose;

frontend/src/components/NotificationsPanel.tsx

@@ -4,6 +4,7 @@ import { useMutation, useQueryClient } from "@tanstack/react-query";
 import { Activity, Bell, Check, CheckCheck, ExternalLink, Eye, RotateCcw, Search, Trash2, Tv, UserPlus, X } from "lucide-react";
 import { api, type AppNotification, type FeedFilters } from "../lib/api";
 import { useLiveQuery } from "../lib/useLiveQuery";
+import { focusAccessRequestsTab } from "./AdminUsers";
 import {
   clearAll as clearClient,
   dismiss as dismissClient,
@@ -190,7 +191,10 @@ export default function NotificationsPanel({
                     onFind={locate}
                     onRevert={revertState}
                     onFocusChannel={onFocusChannel}
-                    onReviewAccess={() => setPage("users")}
+                    onReviewAccess={() => {
+                      focusAccessRequestsTab();
+                      setPage("users");
+                    }}
                     onRemove={() => removeClient(n.id)}
                   />
                 ))}

frontend/src/components/PlayerModal.tsx

@@ -8,6 +8,7 @@ import Avatar from "./Avatar";
 import AddToPlaylist from "./AddToPlaylist";
 import { api, type Video } from "../lib/api";
 import { channelYouTubeUrl, formatDuration, formatViews, relativeTime } from "../lib/format";
+import { useBackToClose } from "../lib/history";
 
 // Turn a description into clickable nodes:
 //  - bare timestamps (mm:ss / hh:mm:ss) → seek the inline player
@@ -203,6 +204,8 @@ export default function PlayerModal({
 }) {
   const { t, i18n } = useTranslation();
   const qc = useQueryClient();
+  // Browser/mouse Back closes the player instead of leaving the page.
+  useBackToClose(onClose);
   // Precise upload date shown inline after the relative time (e.g. "9 yr ago · 12 Jan 2017").
   const fullDate = (d: string | null | undefined) =>
     d

frontend/src/lib/history.ts

@@ -0,0 +1,76 @@
+// In-app browser-history integration so Back/Forward step through a module's sub-views and
+// overlays before leaving the page — not straight back to the previous module.
+//
+// Two primitives share the existing page-history (App stamps `sfPage` in history.state):
+//   - useHistorySubview: a module's active sub-view rides in history.state._sub, so Back inside
+//     a module (e.g. Messages thread → list) returns to the parent view first.
+//   - useBackToClose: while an overlay/modal is mounted it occupies one history entry; Back
+//     closes the topmost one. A module-level stack keeps nesting correct — closing one modal by
+//     its own button pops exactly its entry without tripping the modals underneath.
+//
+// setPage() pushes a clean { sfPage } entry (dropping _sub/_ov), so each page starts at its root.
+import { useEffect, useRef, useState } from "react";
+
+// --- Sub-views (value carried in history.state._sub) ---------------------------------------
+export function useHistorySubview<T>(root: T): {
+  view: T;
+  open: (next: T) => void;
+  back: () => void;
+} {
+  const [view, setView] = useState<T>(() => (window.history.state?._sub as T) ?? root);
+  useEffect(() => {
+    const onPop = () => setView((window.history.state?._sub as T) ?? root);
+    window.addEventListener("popstate", onPop);
+    return () => window.removeEventListener("popstate", onPop);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, []);
+  return {
+    view,
+    open: (next: T) => {
+      setView(next);
+      window.history.pushState({ ...window.history.state, _sub: next }, "");
+    },
+    back: () => window.history.back(),
+  };
+}
+
+// --- Overlays / modals (one history entry each, nesting-safe) -------------------------------
+const overlayStack: number[] = [];
+let overlayCounter = 0;
+// Set while we pop our OWN entry programmatically (button/ESC close) so the other overlays'
+// popstate handlers don't mistake it for a user Back and close themselves too.
+let suppressPop = false;
+
+/** While the calling component is mounted, Back (or the browser/mouse back button) closes it via
+ * `onClose` instead of navigating away. Mount the component only while the overlay is open. */
+export function useBackToClose(onClose: () => void): void {
+  const cb = useRef(onClose);
+  cb.current = onClose;
+  useEffect(() => {
+    const token = ++overlayCounter;
+    overlayStack.push(token);
+    window.history.pushState({ ...window.history.state, _ov: token }, "");
+    const onPop = () => {
+      if (suppressPop) {
+        suppressPop = false;
+        return;
+      }
+      if (overlayStack[overlayStack.length - 1] === token) {
+        overlayStack.pop();
+        cb.current();
+      }
+    };
+    window.addEventListener("popstate", onPop);
+    return () => {
+      window.removeEventListener("popstate", onPop);
+      const idx = overlayStack.lastIndexOf(token);
+      if (idx !== -1) {
+        // Closed programmatically (not via Back): drop our own history entry, and tell the
+        // remaining overlays' handlers to ignore the resulting popstate.
+        overlayStack.splice(idx, 1);
+        suppressPop = true;
+        window.history.back();
+      }
+    };
+  }, []);
+}

frontend/src/lib/urlState.ts

@@ -115,10 +115,11 @@ export function shareUrl(f: FeedFilters, page: Page = "feed"): string {
   return `${window.location.origin}${window.location.pathname}${qs ? `?${qs}` : ""}`;
 }
 
-/** Strip any filter/page query params from the address bar (after hydrating from a share
- * link), leaving a clean URL without touching history. */
+/** Strip the query string from the address bar (after hydrating from a share/auth link),
+ * leaving a clean URL. Preserves the current history.state (e.g. the in-app `sfPage` stamp)
+ * so it's safe to call at any time, not just before that stamp is written. */
 export function stripUrlParams(): void {
   if (window.location.search) {
-    window.history.replaceState(null, "", window.location.pathname);
+    window.history.replaceState(window.history.state, "", window.location.pathname);
   }
 }