feat: stop the unibox conversation list resetting to the top when an email is opened, by keying the app shell's route boundary and Suspense on route identity rather than the raw pathname (the open thread is a path segment, so every click was remounting the whole page), remembering each list's scroll offset across remounts and the mobile display:none pane, patching mark-as-read into the query cache instead of refetching every loaded page, and auto-loading the next page at the end of the list (issue #396)

This commit is contained in:
Matthew Meszaros
2026-09-09 06:52:57 -07:00
parent 51fe8f051a
commit cdbdad09ee
12 changed files with 697 additions and 46 deletions
+2
View File
@@ -47,6 +47,8 @@ Search (press `/`) searches within your current scope, across subjects and the t
Each row is a conversation, not a message, with a badge showing how many messages are inside. Opening it shows every message in order with participants and the owning mailbox. Replies and new inbound mail land in the same thread. Rows group under `Today`, `Yesterday`, `This week`, and `Earlier` headers.
The list loads more conversations as you reach the end of it, and it keeps its place: opening a conversation, or leaving the inbox and coming back, returns you to the row you were on rather than the top of the list.
Threading applies on both sides. A reply you send carries the provider's thread id, which nests it in your own mailbox, and an `In-Reply-To` header naming the last message in the conversation, which is what nests it for the recipient. Their mail client only ever sees the header: a thread id means nothing outside the mailbox that issued it.
<Callout type="info" title="Keyboard navigation">
+7 -2
View File
@@ -19,7 +19,7 @@ import { ChevronLeftIcon, InboxIcon } from "lucide-react";
import { ConversationList } from "@/components/app/unibox/ConversationList";
import { ScheduledList } from "@/components/app/unibox/ScheduledList";
import { ThreadView } from "@/components/app/unibox/ThreadView";
import { ScopeRail, type UniboxScope } from "@/components/app/unibox/ScopeRail";
import { ScopeRail, scopeKey, type UniboxScope } from "@/components/app/unibox/ScopeRail";
import { ScopeSheet } from "@/components/app/unibox/ScopeSheet";
import { UniboxHeader } from "@/components/app/unibox/UniboxHeader";
import useFeatureAccess from "@/hooks/useFeatureAccess";
@@ -336,6 +336,7 @@ export default function UniboxPage() {
)}
>
<ConversationList
scopeKey={scopeKey(scope)}
scopeLabel={scopeLabel}
params={params}
setParams={setParams}
@@ -359,7 +360,11 @@ export default function UniboxPage() {
Inbox
</button>
<div className="flex-1 min-h-0 flex flex-col overflow-hidden">
<ThreadView threadId={urlThread} />
{/* Keyed: the list is what has to survive a thread
change, the reader is what has to start clean, so a
half-typed reply never follows you to the next
conversation. */}
<ThreadView key={urlThread} threadId={urlThread} />
</div>
</>
) : (
@@ -0,0 +1,275 @@
// Issue #396: opening an email sent the conversation list back to the top.
//
// The cause was structural, not visual: the shell keyed its route boundary on
// the pathname, and the unibox puts the open thread IN the pathname, so every
// click tore the page down and built a new one. This mounts the real shell
// (RootAppLayout -> AppShell -> RouteBoundary -> Suspense -> Outlet) around the
// real unibox route and pins both halves of the fix: the list survives the
// click, and a remembered offset is put back whenever something else does zero
// it (the mobile pane, or leaving the inbox and coming back).
import React from "react";
import { describe, it, expect, vi, beforeAll, beforeEach } from "vitest";
import { render, screen, act, fireEvent } from "@testing-library/react";
import { createMemoryRouter, RouterProvider } from "react-router-dom";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
// jsdom has no layout: scrollTop is a hard 0 and the height properties do not
// exist. Back them with real values so a scroll offset is something the test
// can set, read, and watch survive.
let tops = new WeakMap<Element, number>();
beforeAll(() => {
Object.defineProperty(Element.prototype, "scrollTop", {
configurable: true,
get(this: Element) {
return tops.get(this) ?? 0;
},
set(this: Element, value: number) {
tops.set(this, value);
},
});
Object.defineProperty(Element.prototype, "clientHeight", {
configurable: true,
get: () => 400,
});
Object.defineProperty(Element.prototype, "scrollHeight", {
configurable: true,
get: () => 4000,
});
(Element.prototype as unknown as { scrollTo: () => void }).scrollTo = () => {};
(Element.prototype as unknown as { scrollIntoView: () => void }).scrollIntoView = () => {};
});
vi.mock("@/lib/api/client/Request", () => ({
default: (cfg: { url?: string }) => Promise.resolve(route(String(cfg?.url ?? ""))),
}));
vi.mock("@/lib/helper/getToken", () => ({
default: () => ({
access_token: "a",
refresh_token: "r",
access_token_expires_at: new Date(Date.now() + 3600e3).toISOString(),
refresh_token_expires_at: new Date(Date.now() + 3600e3).toISOString(),
}),
}));
vi.mock("@/hooks/SocketProvider", () => ({
default: ({ children }: { children: React.ReactNode }) => <>{children}</>,
}));
vi.mock("@/hooks/context/socket", async (orig) => {
const actual = (await orig()) as Record<string, unknown>;
return {
...actual,
useSocket: () => ({
isConnected: false,
subscribeToChannel: () => () => {},
pushToChannel: () => {},
socket: null,
status: "closed",
}),
useChannel: () => ({ state: "closed", push: () => {}, channel: null }),
useChannelEvent: () => {},
useChannelSubscription: () => {},
};
});
const EMPTY_LIST = { data: [], pagination: { total: 0, next_cursor: null, has_more: false } };
const ROWS = Array.from({ length: 12 }, (_, i) => ({
id: `msg-${i}`,
email_id: "mbox-1",
thread_id: `thread-${i}`,
from_addr: [`Sender ${i} <s${i}@example.com>`],
to_addr: ["me@warmbly.com"],
subject: `Subject ${i}`,
snippet: `Snippet ${i}`,
internal_date: new Date(Date.now() - i * 3600e3).toISOString(),
seen: true,
message_count: 1,
has_unread: false,
labels: [],
}));
function route(url: string): unknown {
if (url === "/auth/me" || url === "/me") {
return {
id: "u1", email: "d@w.com", first_name: "D", last_name: "W",
onboarding_completed_at: new Date().toISOString(),
tags: [], categories: [], folders: [], roles: [],
};
}
// billing_enabled:false unlocks every feature gate, so the inbox renders
// for real instead of behind the upgrade overlay.
if (url.startsWith("/auth/config")) {
return {
captcha: false, password_login: true, login_code: "off",
registration: "invite_only", invites_required: true,
email_verification: false, mail_delivers: false, passkeys: false,
providers: [], self_hosted: true, billing_enabled: false,
setup_required: false, docs_url: "",
};
}
if (url.startsWith("/organization")) return [{ id: "org-1", name: "Org", slug: "org", role: "owner" }];
if (url.startsWith("/subscription/credits")) {
return { monthly_balance: 100, monthly_allowance: 100, purchased_balance: 0, spent_today: 0, spent_week: 0, spent_month: 0 };
}
if (url.startsWith("/subscription")) return { plan: { name: "Pro" }, status: "active" };
if (url.startsWith("/unibox/overview")) {
return { total: ROWS.length, unread: 0, awaiting_reply: 0, snoozed: 0, today: 0, week: 0, mailboxes: [], tags: [], categories: [], folders: [] };
}
if (url.startsWith("/unibox/count")) return { count: 0 };
if (url.startsWith("/unibox/thread")) {
return { data: [{ ...ROWS[0], seen: true }], pagination: { has_more: false, next_cursor: null } };
}
if (url === "/unibox" || url.startsWith("/unibox?")) {
return { data: ROWS, pagination: { has_more: false, next_cursor: null } };
}
if (url.startsWith("/analytics")) return { summary: {}, steps: [], data: [] };
if (url.startsWith("/advisor")) return { findings: [], data: [], total: 0 };
return EMPTY_LIST;
}
const RootAppLayout = (await import("../layout")).default;
const UniboxPage = (await import("./page")).default;
function Elsewhere() {
return <div>Somewhere else</div>;
}
function mount(initial = "/app/unibox/all") {
const router = createMemoryRouter(
[
{
path: "/app",
element: <RootAppLayout />,
children: [
{
path: "unibox/:scope?/:threadId?",
element: <UniboxPage />,
handle: { stableParams: ["scope", "threadId"] },
},
{ path: "analytics", element: <Elsewhere /> },
],
},
],
{ initialEntries: [initial] },
);
render(
<QueryClientProvider client={new QueryClient({ defaultOptions: { queries: { retry: false } } })}>
<RouterProvider router={router} />
</QueryClientProvider>,
);
return router;
}
async function settle() {
await act(async () => {
await new Promise((r) => setTimeout(r, 300));
});
}
function scroller(): HTMLElement {
const row = document.querySelector("[data-thread-id]");
const el = row?.closest<HTMLElement>(".overflow-y-auto");
if (!el) throw new Error("conversation list scroll container not found");
return el;
}
async function scrollTo(top: number) {
const el = scroller();
el.scrollTop = top;
await act(async () => {
fireEvent.scroll(el);
});
}
describe("unibox scroll position", () => {
// The fake offsets are per element and the hook's own memory is per list
// identity, so tests reset the first and take a scope of their own for the
// second. Otherwise a passing assertion could be the previous test's.
beforeEach(() => {
tops = new WeakMap<Element, number>();
});
it("keeps the list where it was when a thread is opened", async () => {
const router = mount("/app/unibox/all");
await settle();
expect(screen.queryByText("Subject 4")).toBeTruthy();
const before = scroller();
await scrollTo(1200);
await act(async () => {
fireEvent.click(screen.getByText("Subject 4").closest("button")!);
});
await settle();
expect(router.state.location.pathname).toBe("/app/unibox/all/thread-4");
// Same DOM node: the page was never torn down, which is the whole fix.
expect(scroller()).toBe(before);
expect(scroller().scrollTop).toBe(1200);
});
it("keeps what the user typed into the list search when a thread is opened", async () => {
mount("/app/unibox/unread");
await settle();
const search = screen.getByPlaceholderText(/^Search unread/i) as HTMLInputElement;
await act(async () => {
fireEvent.change(search, { target: { value: "invoice" } });
});
await settle();
await act(async () => {
fireEvent.click(screen.getByText("Subject 2").closest("button")!);
});
await settle();
expect(
(screen.getByPlaceholderText(/^Search unread/i) as HTMLInputElement).value,
).toBe("invoice");
});
it("puts a remembered offset back after the pane is hidden and shown again", async () => {
// Below `md` the list is display:none while a thread is open, and the
// browser zeroes a hidden scroller without firing a scroll event. Same
// thing here: move the offset behind the component's back, then render.
mount("/app/unibox/today");
await settle();
await scrollTo(700);
await act(async () => {
fireEvent.click(screen.getByText("Subject 3").closest("button")!);
});
await settle();
scroller().scrollTop = 0;
// The thread pane's back link, the mobile way back to the list.
const back = screen
.getAllByRole("button", { name: "Inbox" })
.find((b) => b.className.includes("md:hidden"))!;
await act(async () => {
fireEvent.click(back);
});
await settle();
expect(scroller().scrollTop).toBe(700);
});
it("puts a remembered offset back when the inbox is re-entered", async () => {
const router = mount("/app/unibox/week");
await settle();
await scrollTo(900);
await act(async () => {
await router.navigate("/app/analytics");
});
await settle();
expect(screen.queryByText("Somewhere else")).toBeTruthy();
await act(async () => {
await router.navigate("/app/unibox/week");
});
await settle();
expect(scroller().scrollTop).toBe(900);
});
});
@@ -16,6 +16,8 @@ import React from "react";
import { Loader2Icon, SearchIcon, Settings2Icon } from "lucide-react";
import { ConversationItem } from "./ConversationItem";
import useUniboxSearch from "@/lib/api/hooks/app/unibox/useUniboxSearch";
import useDebouncedValue from "@/hooks/useDebouncedValue";
import { useScrollMemory } from "@/hooks/useScrollMemory";
import { useAppStore } from "@/stores";
import { UniboxFilterSheet } from "./UniboxFilterSheet";
import type { UniboxSearchParams } from "@/lib/api/models/app/unibox/UniboxSearch";
@@ -46,34 +48,83 @@ function bucketFor(d: Date): Bucket {
}
interface ConversationListProps {
/** Identity of the current scope; a change clears the local search. */
scopeKey: string;
scopeLabel: string;
params: UniboxSearchParams;
setParams: React.Dispatch<React.SetStateAction<UniboxSearchParams>>;
}
export function ConversationList({
scopeKey,
scopeLabel,
params,
setParams,
}: ConversationListProps) {
const [search, setSearch] = React.useState("");
const [sheetOpen, setSheetOpen] = React.useState(false);
// The page keeps this component mounted across a scope switch (that is what
// holds the scroll offset when a thread opens), so the search box has to be
// cleared here or a query typed for one scope would silently filter the next.
// Set during render, like the page's own param reset, so the stale query
// never reaches the request.
const [searchScope, setSearchScope] = React.useState(scopeKey);
if (searchScope !== scopeKey) {
setSearchScope(scopeKey);
setSearch("");
}
const searchRef = React.useRef<HTMLInputElement>(null);
const listRef = React.useRef<HTMLDivElement>(null);
const sentinelRef = React.useRef<HTMLDivElement>(null);
const selectedThreadId = useAppStore((s) => s.selectedThreadId);
const setSelectedThreadId = useAppStore((s) => s.setSelectedThreadId);
const setSelectedAccountId = useAppStore((s) => s.setSelectedAccountId);
// Debounced into the query, immediate in the box: the search text is part of
// the query key, so a raw binding fires a request and parks a cached page per
// keystroke.
const debouncedSearch = useDebouncedValue(search);
const merged: UniboxSearchParams = React.useMemo(() => {
const next: UniboxSearchParams = { ...params };
if (search.trim()) next.query = search.trim();
if (debouncedSearch.trim()) next.query = debouncedSearch.trim();
return next;
}, [params, search]);
}, [params, debouncedSearch]);
const q = useUniboxSearch(merged);
const emails = q.emails;
const totalShown = emails.length;
// Where this exact list was left. Opening a thread keeps the page mounted
// (see the route's stableParams in main.tsx), so this covers what that
// cannot: leaving the inbox and coming back, and the mobile pane, which the
// browser scrolls to the top while it is display:none.
const listKey = React.useMemo(() => JSON.stringify(merged), [merged]);
useScrollMemory(listRef, listKey);
// Infinite scroll: reaching the end of the list loads the next page instead
// of asking for a click. The button below stays as the manual fallback, and
// isFetchingNextPage is a dependency so a page landing re-arms the observer:
// a sentinel still on screen keeps pulling instead of stalling one page in.
const { hasNextPage, isFetchingNextPage, isFetchNextPageError, fetchNextPage } = q;
React.useEffect(() => {
const sentinel = sentinelRef.current;
const root = listRef.current;
// A page that failed stays failed until the user asks again; re-arming on
// an on-screen sentinel would retry it on a loop.
if (!sentinel || !root || !hasNextPage || isFetchNextPageError) return;
if (typeof IntersectionObserver === "undefined") return;
const io = new IntersectionObserver(
(entries) => {
if (entries.some((e) => e.isIntersecting)) void fetchNextPage();
},
{ root, rootMargin: "400px 0px" },
);
io.observe(sentinel);
return () => io.disconnect();
}, [hasNextPage, isFetchingNextPage, isFetchNextPageError, fetchNextPage]);
// Group rows by time bucket. The server already orders newest →
// oldest so a single pass preserves both global order and group
// adjacency.
@@ -201,9 +252,9 @@ export function ConversationList({
</div>
<div ref={listRef} className="flex-1 overflow-y-auto">
{q.isPending ? (
{q.isPending && emails.length === 0 ? (
<SkeletonRows />
) : q.isError ? (
) : q.isError && emails.length === 0 ? (
<div className="px-5 py-12 text-center">
<p className="text-[12.5px] text-slate-900 font-medium mb-1">
Couldn't load inbox
@@ -269,22 +320,33 @@ export function ConversationList({
</div>
</section>
))}
{q.hasNextPage && (
<div className="px-3 py-3 flex justify-center border-t border-slate-200/60">
<button
onClick={() => q.fetchNextPage()}
disabled={q.isFetchingNextPage}
className="h-7 px-3 rounded-md border border-slate-200 hover:border-slate-300 text-[12px] text-slate-700 hover:text-slate-900 inline-flex items-center gap-1.5 transition-colors disabled:opacity-50"
>
{q.isFetchingNextPage ? (
<>
<Loader2Icon className="w-3 h-3 animate-spin" />
Loading
</>
) : (
`Load more · ${totalShown} shown`
)}
</button>
{hasNextPage && (
<div
ref={sentinelRef}
className="px-3 py-3 flex flex-col items-center gap-1.5 border-t border-slate-200/60"
>
{isFetchingNextPage ? (
<span className="h-7 text-[12px] text-slate-400 inline-flex items-center gap-1.5">
<Loader2Icon className="w-3 h-3 animate-spin" />
Loading more…
</span>
) : (
<>
{isFetchNextPageError && (
<span className="text-[11.5px] text-rose-600">
Couldn't load more conversations
</span>
)}
<button
onClick={() => fetchNextPage()}
className="h-7 px-3 rounded-md border border-slate-200 hover:border-slate-300 text-[12px] text-slate-700 hover:text-slate-900 inline-flex items-center gap-1.5 transition-colors"
>
{isFetchNextPageError
? "Try again"
: `Load more · ${totalShown} shown`}
</button>
</>
)}
</div>
)}
</>
+7 -6
View File
@@ -236,10 +236,11 @@ export function ThreadView({ threadId, emailId }: ThreadViewProps) {
replyState ? "replying" : "viewing",
);
// Opening a thread marks its unseen messages as read. The hook invalidates
// ["unibox"], so the unread badge, the collapsed list, and the overview all
// refresh. Once everything is seen the id list is empty and this no-ops, so
// it self-terminates after the post-mark refetch (no loop).
// Opening a thread marks its unseen messages as read. The hook writes the
// flip straight into the cached list and thread and refetches only the
// counters, so the conversation list the user came from does not re-order
// under them. Passing threadId is what lets it find the row. Once everything
// is seen the id list is empty and this no-ops, so it self-terminates.
const markSeen = useMarkSeen();
const markSeenMutate = markSeen.mutate;
React.useEffect(() => {
@@ -247,7 +248,7 @@ export function ThreadView({ threadId, emailId }: ThreadViewProps) {
.filter((m) => !m.seen)
.map((m) => m.id);
if (unseenIds.length === 0) return;
markSeenMutate({ ids: unseenIds });
markSeenMutate({ ids: unseenIds, threadId });
}, [threadId, q.data, markSeenMutate]);
const snooze = useMutation({
@@ -257,7 +258,7 @@ export function ThreadView({ threadId, emailId }: ThreadViewProps) {
toast.success("Snoozed");
queryClient.invalidateQueries({ queryKey: ["unibox", "search"] });
queryClient.invalidateQueries({ queryKey: ["unibox", "overview"] });
queryClient.invalidateQueries({ queryKey: ["unibox", "count"] });
queryClient.invalidateQueries({ queryKey: ["unibox", "unseen-count"] });
setSnoozeOpen(false);
setCustomMode(false);
},
+7 -2
View File
@@ -29,6 +29,7 @@ import { CommandPalette } from "@/components/shared/CommandPalette";
import { useKeyboardShortcuts } from "@/hooks/useKeyboardShortcuts";
import { GlobalCursorsProvider } from "@/components/app/presence/GlobalCursors";
import AgentPanel from "@/components/app/agent/AgentPanel";
import { useRouteKey } from "@/hooks/useRouteKey";
export function AppShell() {
useKeyboardShortcuts();
@@ -46,9 +47,13 @@ export function AppShell() {
// Pages scroll this inner container, not the window, so nothing resets the
// offset between routes: navigating from halfway down a long list used to
// land mid-page on the next one. Reset before paint so it never flashes.
// Keyed on the route identity, not the raw pathname, so a page that keeps
// in-page state in the URL (the unibox's open thread) is not scrolled away
// from what the user was reading.
const routeKey = useRouteKey();
useLayoutEffect(() => {
scrollRef.current?.scrollTo({ top: 0, left: 0 });
}, [pathname]);
}, [routeKey]);
return (
<div className="fixed inset-0 flex flex-col">
@@ -80,7 +85,7 @@ export function AppShell() {
content area and stays that way until
the query lands (only a reload fixes
it). This is that boundary. */}
<Suspense fallback={<RouteFallback />}>
<Suspense key={routeKey} fallback={<RouteFallback />}>
<SubscriptionGate>
<Outlet />
</SubscriptionGate>
+23 -7
View File
@@ -16,18 +16,31 @@ import { AlertTriangleIcon, RefreshCcwIcon } from "lucide-react";
interface State {
error: Error | null;
info: React.ErrorInfo | null;
/** Last `resetKey` seen, so a change clears the error without a remount. */
seenResetKey?: string;
}
export class ErrorBoundary extends React.Component<
{ children: React.ReactNode; onReset?: () => void },
State
> {
interface BoundaryProps {
children: React.ReactNode;
onReset?: () => void;
/** Change this to drop a caught error and re-render the children. */
resetKey?: string;
}
export class ErrorBoundary extends React.Component<BoundaryProps, State> {
state: State = { error: null, info: null };
static getDerivedStateFromError(error: Error): Partial<State> {
return { error };
}
// Clearing the error here rather than remounting the boundary is what lets
// a healthy page keep its state across a URL change.
static getDerivedStateFromProps(props: BoundaryProps, state: State): Partial<State> | null {
if (state.seenResetKey === props.resetKey) return null;
return { seenResetKey: props.resetKey, error: null, info: null };
}
componentDidCatch(error: Error, info: React.ErrorInfo) {
this.setState({ info });
if (typeof window !== "undefined" && (window as unknown as { Sentry?: { captureException: (e: Error) => void } }).Sentry) {
@@ -111,7 +124,10 @@ function BoundaryFallback({ error, info, reset }: { error: Error; info: React.Er
*/
export function RouteBoundary({ children }: { children: React.ReactNode }) {
const { pathname } = useLocation();
// Key forces a fresh ErrorBoundary instance on every route change, which
// both clears stale errors and lets the new page mount cleanly.
return <ErrorBoundary key={pathname}>{children}</ErrorBoundary>;
// resetKey, not key: a `key` here would tear down and re-create the whole
// routed subtree on every URL change, which throws away the scroll offset
// of any list whose page encodes in-page state in the path (issue #396).
// Mounting the next page cleanly is the Suspense boundary's job in
// AppShell, which keys off the route identity instead.
return <ErrorBoundary resetKey={pathname}>{children}</ErrorBoundary>;
}
+38
View File
@@ -0,0 +1,38 @@
// Identity of the page the router is currently showing.
//
// The shell uses it for the two things that must happen when you move to a
// different page and must NOT happen when you only move around inside one:
// remounting the route's Suspense boundary, and putting the content panel back
// at the top.
//
// Default identity is the full pathname, so every URL change is a new page.
// A route whose path also carries in-page state opts those params out:
//
// { path: "unibox/:scope?/:threadId?", element: <UniboxPage />,
// handle: { stableParams: ["scope", "threadId"] } }
//
// Changing only a stable param keeps the page mounted, so its lists keep their
// scroll offset and its inputs keep their text (issue #396).
import { useLocation, useMatches } from "react-router-dom";
export interface RouteHandle {
/** Route params that address state inside the page, not a different page. */
stableParams?: string[];
}
export function useRouteKey(): string {
const { pathname } = useLocation();
const matches = useMatches();
const deepest = matches[matches.length - 1];
const stable = (deepest?.handle as RouteHandle | undefined)?.stableParams;
if (!deepest || !stable || stable.length === 0) return pathname;
const params = (deepest.params ?? {}) as Record<string, string | undefined>;
const rest = Object.keys(params)
.filter((name) => !stable.includes(name))
.sort()
.map((name) => `${name}=${params[name] ?? ""}`)
.join("&");
return `${deepest.id}?${rest}`;
}
+164
View File
@@ -0,0 +1,164 @@
// Remembers where a scroll container was left, per list identity, and puts it
// back.
//
// Two things throw a scroll offset away and neither is under the component's
// control: unmounting (leaving the page and coming back) and `display: none`
// (the responsive panes below `md`, where the browser silently zeroes
// scrollTop). Both land the user back at the top of a list they had scrolled
// through, which is issue #396.
//
// Offsets live in a module-level map, so they survive a remount for as long as
// the tab does, and are keyed by whatever identifies the list's contents (scope
// + filters). A different key is a different list: it starts at the top.
//
// The restore is deliberately timid. It stops the moment the user touches the
// wheel, a finger or a key, and it gives up after a short window. A container
// that merely came back from `display: none` is only touched while it sits at
// the top, which is the one offset that has to be the browser's doing rather
// than the user's: a top the user scrolled to was recorded as a top. It keeps
// retrying while the list is still filling in, so a return to a list whose rows
// arrive a frame or two later still lands where it should.
import * as React from "react";
/** How long to keep trying to reach a remembered offset (ms). */
const RESTORE_WINDOW_MS = 2000;
/** Frames to wait for a short list to grow before settling for its bottom. */
const STALL_FRAMES = 30;
/** Bound on remembered lists, so a session of filter-typing can't grow it. */
const MAX_KEYS = 40;
const positions = new Map<string, number>();
function remember(key: string, top: number): void {
// Re-insert so the map's iteration order is least-recently-used first.
positions.delete(key);
positions.set(key, top);
while (positions.size > MAX_KEYS) {
const oldest = positions.keys().next().value;
if (oldest === undefined) break;
positions.delete(oldest);
}
}
export function useScrollMemory(
ref: React.RefObject<HTMLElement | null>,
key: string,
): void {
// The offset we are still trying to reach; null means "not restoring",
// which is also what tells the scroll listener it may record again.
const pending = React.useRef<number | null>(null);
const deadline = React.useRef(0);
const frame = React.useRef(0);
const lastHeight = React.useRef(0);
const stalled = React.useRef(0);
const stop = React.useCallback(() => {
pending.current = null;
if (frame.current) cancelAnimationFrame(frame.current);
frame.current = 0;
}, []);
const step = React.useCallback(() => {
frame.current = 0;
const el = ref.current;
const want = pending.current;
if (!el || want == null) return;
if (el.clientHeight > 0) {
const max = Math.max(el.scrollHeight - el.clientHeight, 0);
const next = Math.min(want, max);
if (Math.abs(el.scrollTop - next) > 1) el.scrollTop = next;
if (next >= want - 1) return stop();
// Short of the target: only worth holding on while rows are still
// arriving. A list that has stopped growing is as long as it is
// going to get, and pinning the user to its bottom for the rest of
// the window would be the same fight this hook exists to end.
if (el.scrollHeight === lastHeight.current) {
if (++stalled.current > STALL_FRAMES) return stop();
} else {
stalled.current = 0;
lastHeight.current = el.scrollHeight;
}
}
if (performance.now() >= deadline.current) return stop();
frame.current = requestAnimationFrame(step);
}, [ref, stop]);
// `fresh` means the list itself changed (new scope, new filters): it gets
// positioned deliberately, at its own remembered offset or at the top,
// never at wherever the previous list happened to be parked. Otherwise this
// is a container that came back from `display: none`, where the only offset
// worth undoing is the one the browser wrote.
const arm = React.useCallback(
(fresh: boolean) => {
const el = ref.current;
// A hidden container has nothing to position, and arming one would
// leave a frame loop running for as long as it stays hidden. It
// gets its turn when it comes back.
if (!el || el.clientHeight === 0) return;
const want = positions.get(key) ?? 0;
if (want <= 0) {
if (fresh) el.scrollTop = 0;
return;
}
if (!fresh && el.scrollTop !== 0) return;
pending.current = want;
deadline.current = performance.now() + RESTORE_WINDOW_MS;
lastHeight.current = el.scrollHeight;
stalled.current = 0;
if (!frame.current) frame.current = requestAnimationFrame(step);
},
[ref, key, step],
);
React.useLayoutEffect(() => {
const el = ref.current;
if (!el) return;
const onScroll = () => {
// A hidden container reports 0 for everything; recording that would
// overwrite the very offset we are holding on to.
if (pending.current != null || el.clientHeight === 0) return;
remember(key, el.scrollTop);
};
// Any deliberate move hands control back to the user for good.
const onUserScroll = () => stop();
el.addEventListener("scroll", onScroll, { passive: true });
el.addEventListener("wheel", onUserScroll, { passive: true });
el.addEventListener("touchmove", onUserScroll, { passive: true });
el.addEventListener("keydown", onUserScroll);
// A scrollbar drag produces neither a wheel nor a touch, only this.
el.addEventListener("pointerdown", onUserScroll);
// Catches a pane coming back from `display: none` without this
// component re-rendering.
let wasVisible = el.clientHeight > 0;
const observer = new ResizeObserver(() => {
const visible = el.clientHeight > 0;
if (visible && !wasVisible) arm(false);
wasVisible = visible;
});
observer.observe(el);
arm(true);
return () => {
stop();
observer.disconnect();
el.removeEventListener("scroll", onScroll);
el.removeEventListener("wheel", onUserScroll);
el.removeEventListener("touchmove", onUserScroll);
el.removeEventListener("keydown", onUserScroll);
el.removeEventListener("pointerdown", onUserScroll);
};
}, [ref, key, arm, stop]);
// ResizeObserver is allowed to skip an element with no box, so a pane that
// is hidden and shown again may never report the round trip. Every render
// is the other moment that can happen, and the check costs nothing unless
// the container is sitting at a top it did not choose.
React.useLayoutEffect(() => {
arm(false);
});
}
@@ -1,15 +1,91 @@
import { useMutation, useQueryClient } from "@tanstack/react-query";
import { useMutation, useQueryClient, type InfiniteData } from "@tanstack/react-query";
import markSeen from "@/lib/api/client/app/unibox/markSeen";
import type { UniboxListRow } from "@/lib/api/client/app/unibox/searchIncoming";
import type UniboxThread from "@/lib/api/models/app/unibox/UniboxThread";
interface SearchPage {
data: UniboxListRow[];
pagination: { has_more: boolean; next_cursor: string | null };
}
interface MarkSeenInput {
ids?: string[];
folder?: string;
seen?: boolean;
/** Conversation the ids belong to, so the open list can be patched in place. */
threadId?: string;
}
export default function useMarkSeen() {
const queryClient = useQueryClient();
return useMutation({
mutationFn: (data: { ids?: string[]; folder?: string; seen?: boolean }) => markSeen(data),
onSuccess: () => {
queryClient.invalidateQueries({
queryKey: ["unibox"],
})
}
})
mutationFn: (data: MarkSeenInput) => markSeen(data),
// Reading a thread must not move the list the user is reading from.
// Refetching every loaded page of ["unibox","search"] would re-order
// rows around whatever arrived since, so the read/unread flip is
// written straight into the cache instead; only the counters, which no
// pointer is aimed at, are refetched.
onMutate: async ({ ids, threadId, seen = true, folder }) => {
if (folder || (!threadId && !ids?.length)) return;
// A refetch already in flight would land on top of the patch below
// and put the row back to unread. Only refetches: cancelling a
// first load would leave that list with no data and nothing queued
// to fetch it again.
await queryClient.cancelQueries({
queryKey: ["unibox", "search"],
predicate: (query) => query.state.data !== undefined,
});
const idSet = new Set(ids ?? []);
const matches = (row: { id: string; thread_id?: string }) =>
(threadId != null && row.thread_id === threadId) || idSet.has(row.id);
queryClient.setQueriesData<InfiniteData<SearchPage>>(
{ queryKey: ["unibox", "search"] },
(old) =>
!old
? old
: {
...old,
pages: old.pages.map((page) => ({
...page,
data: (page.data ?? []).map((row) =>
matches(row) && row.has_unread === seen
? { ...row, has_unread: !seen, seen }
: row,
),
})),
},
);
if (threadId) {
queryClient.setQueriesData<UniboxThread>(
{ queryKey: ["unibox", "thread", threadId] },
(old) =>
!old
? old
: {
...old,
data: (old.data ?? []).map((m) =>
m.seen === seen ? m : { ...m, seen },
),
},
);
}
},
// A patch the server rejected has to come back off; re-reading the
// lists is both the rollback and the resync.
onError: () => {
queryClient.invalidateQueries({ queryKey: ["unibox"] });
},
onSuccess: (_data, { folder }) => {
// A folder sweep touches rows we have no ids for, so that one still
// has to re-read the list.
if (folder) queryClient.invalidateQueries({ queryKey: ["unibox"] });
else {
queryClient.invalidateQueries({ queryKey: ["unibox", "overview"] });
queryClient.invalidateQueries({ queryKey: ["unibox", "unseen-count"] });
}
},
});
}
@@ -20,7 +20,10 @@ export default function useUniboxSearch(params: UniboxSearchParams, enabled = tr
initialPageParam: null,
getNextPageParam: (last) => (last.pagination.has_more ? last.pagination.next_cursor : undefined),
staleTime: 30_000,
gcTime: 5 * 60 * 1000,
// Held long enough that leaving the inbox and coming back restores every
// page the user had loaded, which is what the remembered scroll offset
// needs to land on (issue #396).
gcTime: 30 * 60 * 1000,
// Scope/filter switches change the query key; keep showing the
// previous list while the new one loads instead of flashing the
// whole pane to skeletons on every switch.
+4
View File
@@ -401,8 +401,12 @@ const router = createBrowserRouter([
{
// Path-based, readable inbox URLs: /app/unibox/<scope>[/<threadId>].
// Both segments optional, so /app/unibox is the default "all" view.
// Both are state inside one page, not different pages, so the shell
// keeps the page mounted across them and the conversation list holds
// its scroll offset when a thread opens (issue #396).
path: "unibox/:scope?/:threadId?",
element: <UniboxPage />,
handle: { stableParams: ["scope", "threadId"] },
},
{
// Legacy /app/team entry points → the Members settings section.