Repository navigation
fix(web): sidebar keeps the thread behind Settings and Usage highlighted - #28710
juliusmarminge wants to merge 4 commits into
Conversation
…d Usage The chat layout kept rendering the thread behind the Settings and Usage modals, but the sidebar read the raw route params, which are empty on a modal route. The thread's row lost its active highlight and showed its own draft as an unsent draft until the modal closed. Move the background-target logic into useShownThreadRouteTarget and use it in both the chat layout and the sidebar so they cannot disagree. Fixes #28692 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The thread shown behind Settings and Usage was remembered in per-component state, so a Sidebar that remounted under the modal (crossing the mobile breakpoint) started from the modal route and lost the thread. Record it at router level beside the modal's return href, on onBeforeLoad so a navigation that is superseded before it settles still counts, and read it through useSyncExternalStore so ChatRouteLayout and Sidebar agree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
/projects/$projectKey only redirects into Settings, so its load no longer clears the thread the sidebar and chat view show behind the modal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — Sidebar highlighting and the chat background now share the remembered thread while Settings or Usage is open, including after sidebar remounts. The change is limited to client-side presentation and introduces no material operational or data risk. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Track and expose modal return targets apps/web/src/components/sidebar/mainAppLocation.ts, apps/web/src/hooks/useThreadRouteTarget.ts |
Location tracking records thread targets during navigation and exposes a subscription and getter. The shared hook returns the background target while an app modal is open, or the current route target otherwise. |
Use shown targets in chat and sidebar apps/web/src/routes/_chat.tsx, apps/web/src/components/Sidebar.tsx, apps/web/src/hooks/useThreadRouteTarget.browser.test.tsx |
The chat layout and sidebar use the shared hook instead of local or current-route-only target logic. Browser tests cover modal routes, thread and draft navigation, remounting, redirects, and navigation from home. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix · Severity of issue fixed: Low
Possibly related PRs
- pingdotgg/t3code#28703: Addresses the same modal highlight behavior and changes the same route-target components.
- pingdotgg/t3code#25239: Introduced the location-tracking function that this change extends.
- pingdotgg/t3code#24847: Introduced Settings and Usage as modal routes over a retained thread.
Suggested labels: bugfix, size:M
Suggested reviewers: t3dotgg, maria-rcks
Merge Risk: 
fd172
-
No merge-blocking risk was established; normal checks remain appropriate.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: preserving the sidebar thread highlight while Settings and Usage are open. Description check The description includes all required sections. It explains the problem and reproduction steps, the implementation, linked issue scope, focused tests, visual verification, limitations, and agent detai… Linked Issues check Directly linked issue #28692has one coding requirement group: while Settings or Usage is open over a thread, the sidebar must keep that thread's active highlight, and must not show the thread's own u…Out of Scope Changes check Every changed area supports the #28692fix:
- mainAppLocation.ts: target tracking and listeners are needed for the fix. The /projects/ exclusion keeps the thread behind a project link that redirects …Full details: Description check
-
Explanation
The description includes all required sections. It explains the problem and reproduction steps, the implementation, linked issue scope, focused tests, visual verification, limitations, and agent details.
Full details: Linked Issues check
-
Explanation
Directly linked issue
#28692has one coding requirement group: while Settings or Usage is open over a thread, the sidebar must keep that thread's active highlight, and must not show the thread's own unsent-draft indicator. The draft route (/draft/$draftId) must get the same treatment. The reviewed head implements this:- apps/web/src/components/sidebar/mainAppLocation.ts records the background thread target at router level. The recordTarget function runs on onBeforeLoad and on the initial location. It skips app modal pages and /projects/ paths, and it notifies subscribers when the target key changes. subscribeAppModalReturnTarget and getAppModalReturnTarget expose the target.
- apps/web/src/hooks/useThreadRouteTarget.ts adds useShownThreadRouteTarget. It reads modalOpen from useMatches (matches whose pathname is an app modal page). It reads the background target through useSyncExternalStore only while a modal is open. Otherwise it returns the current route target. Draft targets are handled through draftId in resolveThreadRouteTarget.
- apps/web/src/routes/_chat.tsx now takes shownTarget and modalOpen from useShownThreadRouteTarget. Its local backgroundTarget state is removed.
- apps/web/src/components/Sidebar.tsx reads useShownThreadRouteTarget instead of useThreadRouteTarget (+2/-2, per the change summary). Because the route thread is then the shown thread, isActive and isRouteThread stay set for that row. The draft indicator condition (useThreadHasUnsentDraft && !isRouteThread) therefore stops applying to the open thread.
The router-level store also covers a Sidebar that remounts under the modal, which the issue's per-component design would not cover.
The PR description reports browser tests for Settings, Usage, drafts, remount, project redirect, and Settings from home (apps/web/src/hooks/useThreadRouteTarget.browser.test.tsx). I did not read that test file or run it. The Sidebar.tsx hunk is confirmed only through the change summary, not by reading the file, because the inspection budget was reached. No coding requirement from
#28692is unmet on the evidence available.
Full details: Out of Scope Changes check
-
Explanation
Every changed area supports the
#28692fix:- mainAppLocation.ts: target tracking and listeners are needed for the fix. The /projects/ exclusion keeps the thread behind a project link that redirects into Settings. The existing return-URL tracking (record, mainAppHref, appModalReturnHref) is unchanged in behavior.
- useThreadRouteTarget.ts: useShownThreadRouteTarget is the shared read path for ChatRouteLayout and Sidebar.
- _chat.tsx: the duplicate local backgroundTarget logic is replaced by the shared hook. This removes code that the fix makes redundant.
- Sidebar.tsx: switches the route target source to the shared hook.
- useThreadRouteTarget.browser.test.tsx: tests the new behavior.
I found no change that is unrelated to the issue. The PR description images and the visual comparison are non-code evidence and do not affect this check. Electron and mobile behavior are not covered by the PR. The issue does not require them, and they are not changed.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
|
Closing as a duplicate of #28703, which fixes the same issue and was opened first. |
Problem
While Settings or Usage is open as a modal over a thread, the sidebar drops that thread's active highlight. If the thread has unsent composer text, the sidebar also shows the yellow draft tint and pen icon that normally appear only for other threads. Closing the modal restores both. Reproduced on main (82b10d5) in the web dev server on a fresh database:
The row's
bg-sidebar-row-activeis replaced bybg-warning/4with the pen icon. A/draft/$idthread loses its highlight the same way.Change
ChatRouteLayoutalready kept rendering the last thread while a modal route was matched, butSidebarread the raw route params, which are empty on/settings/*and/usage. Now both readuseShownThreadRouteTarget(), so they cannot disagree.The thread behind the modal is recorded at router level, next to the existing return-location tracking in
mainAppLocation.ts, and read throughuseSyncExternalStore. Per-component state was not enough:Sidebarremounts when the viewport crosses the mobile breakpoint, and a remount under the modal would start with no thread. The target is recorded when each load starts (onBeforeLoad), becauseonResolvedis skipped when navigations overlap./projects/$projectKeyis excluded: it only redirects into/settings/projectsand must not clear the thread behind it.Scope and approval
Fixes #28692
Verification
apps/web/src/hooks/useThreadRouteTarget.browser.test.tsx(new, 3 tests, pass). It covers:The remount case fails on the per-component-state version, and the project-link case fails without its exclusion.
vp run check:changed: format, lint, knip, typecheck, tests, React compiler all pass.Real web app, before 82b10d5 vs after 44bdd64. The head adds only the
/projects/*exclusion and its test on top, which no captured row exercises.T3CODE_DEV_SEED=0), data typed through the app.R4 recording. Same scripted steps on both sides: Settings open over the thread at 1440×900 (0 s); request 390×900 (2 s); request 1440×900 (6 s); end (~12.3 s). Before, the row is never active. After, the row is active at 1440 before and after the resize.
What else changed: nothing outside the affected sidebar row and its blur halo (zero-threshold diffs). R5 has zero changed pixels.
Not checked: the Electron desktop shell (it runs the same web code), and mobile (React Native, not affected).
Model and harness: Claude Opus 5.5 (lead, implementation, review B) and GPT-6.1-sol (review A, proof audit, captures) in T3 Code.
🤖 Generated with Claude Code