Add browser panel with right panel tabs - #1516
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size:XL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| const closePanel = useCallback(() => { | ||
| void navigate({ | ||
| to: "/$threadId", | ||
| params: { threadId }, | ||
| search: { diff: undefined }, | ||
| }); | ||
| }, [navigate, threadId]); |
There was a problem hiding this comment.
🟠 High routes/_chat.$threadId.tsx:175
The closePanel callback replaces all search params with { diff: undefined }, which drops any non-diff-related params that may exist on the route. This is inconsistent with openPanel and changeTab, which both use the function form search: (previous) => ... to preserve existing params. Consider using search: (previous) => stripDiffSearchParams(previous) to maintain consistency and prevent data loss.
| const closePanel = useCallback(() => { | |
| void navigate({ | |
| to: "/$threadId", | |
| params: { threadId }, | |
| search: { diff: undefined }, | |
| }); | |
| }, [navigate, threadId]); | |
| const closePanel = useCallback(() => { | |
| void navigate({ | |
| to: "/$threadId", | |
| params: { threadId }, | |
| search: (previous) => stripDiffSearchParams(previous), | |
| }); | |
| }, [navigate, threadId]); |
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/routes/_chat.$threadId.tsx around lines 175-181:
The `closePanel` callback replaces all search params with `{ diff: undefined }`, which drops any non-diff-related params that may exist on the route. This is inconsistent with `openPanel` and `changeTab`, which both use the function form `search: (previous) => ...` to preserve existing params. Consider using `search: (previous) => stripDiffSearchParams(previous)` to maintain consistency and prevent data loss.
Evidence trail:
apps/web/src/routes/_chat.$threadId.tsx lines 175-180 (closePanel uses direct object `search: { diff: undefined }`), lines 182-191 (openPanel uses function form `search: (previous) => ...`), lines 193-206 (changeTab uses function form `search: (previous) => ...`). Commit: REVIEWED_COMMIT
| value={inputValue} | ||
| onChange={(e) => setInputValue(e.target.value)} | ||
| onKeyDown={handleKeyDown} | ||
| onBlur={handleSubmit} |
There was a problem hiding this comment.
🟡 Medium components/BrowserPanel.tsx:129
The onBlur={handleSubmit} handler calls onNavigate on every blur, even when the URL hasn't changed. This resets the tab title to the URL via navigateTab, overwriting any page title that was set by setTabTitle. Consider only calling onNavigate when the input value actually differs from the current URL.
- onBlur={handleSubmit}
+ onBlur={() => {
+ if (inputValue !== url && inputValue.trim().length > 0) {
+ handleSubmit();
+ }
+ }}🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/components/BrowserPanel.tsx around line 129:
The `onBlur={handleSubmit}` handler calls `onNavigate` on every blur, even when the URL hasn't changed. This resets the tab `title` to the URL via `navigateTab`, overwriting any page title that was set by `setTabTitle`. Consider only calling `onNavigate` when the input value actually differs from the current URL.
Evidence trail:
apps/web/src/components/BrowserPanel.tsx lines 79-86 (`handleSubmit` function that always calls `onNavigate` without checking if URL changed), line 129 (`onBlur={handleSubmit}`), lines 160-167 (`handleNavigate` calls `navigateTab`); apps/web/src/browserPanelStore.ts lines 122-128 (`navigateTab` sets `title: url`, overwriting any existing title)
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| ); | ||
| }); | ||
| if (onOpenUrlInBrowser) { | ||
| onOpenUrlInBrowser(match.text); |
There was a problem hiding this comment.
Stale closure captures onOpenUrlInBrowser in terminal link provider
High Severity
The onOpenUrlInBrowser prop is captured directly in the link provider closure registered inside a useEffect that only re-runs when [cwd, runtimeEnv, terminalId, threadId] change. Unlike onSessionExited and onAddTerminalContext, which use refs (onSessionExitedRef, onAddTerminalContextRef) kept in sync via separate useEffect hooks, onOpenUrlInBrowser has no ref. After initial mount, clicking a URL link in the terminal will invoke a stale callback, potentially using an outdated project ID or navigation state.
Additional Locations (1)
| <XIcon className="size-2.5" /> | ||
| </button> | ||
| </button> | ||
| ); |
There was a problem hiding this comment.
Nested button elements create invalid HTML
Medium Severity
BrowserTabButton nests a close <button> (line 50) inside the outer tab <button> (line 38). Nesting interactive elements like <button> inside another <button> is invalid HTML. Browsers handle this inconsistently — some may prevent the inner button's click from firing, or always trigger the outer button's onClick regardless of stopPropagation, leading to unreliable close-tab behavior.
| import type { ProjectId } from "@t3tools/contracts"; | ||
| import { | ||
| ArrowLeftIcon, | ||
| ArrowRightIcon, |
There was a problem hiding this comment.
Unused imports ArrowLeftIcon and ArrowRightIcon
Low Severity
ArrowLeftIcon and ArrowRightIcon are imported from lucide-react but never used anywhere in the BrowserPanel component.
| diffToggleShortcutLabel={diffPanelShortcutLabel} | ||
| gitCwd={gitCwd} | ||
| diffOpen={diffOpen} | ||
| browserOpen={browserOpen} |
There was a problem hiding this comment.
Diff toggle button shows pressed when browser tab active
Medium Severity
The diff toggle button's pressed state is bound to diffOpen, which is true whenever the right panel is open regardless of which tab is active. Meanwhile browserOpen is correctly computed as diffOpen && rightPanelTab === "browser". When the browser tab is active, both the diff and browser toggle buttons appear pressed simultaneously, which is visually incorrect and confusing. The diffOpen prop passed to ChatHeader needs to reflect only whether the diff tab is actually active, not just whether the panel is open.
Additional Locations (1)
1 participant
Note
Medium Risk
Introduces a new persisted per-project browser state and integrates it into routing/keyboard shortcuts for the right panel; regressions could affect diff panel toggling behavior and stored UI state across threads/projects.
Overview
Adds a new Browser right-panel tab alongside the existing diff viewer, including a tab switcher (
RightPanelTabs) that lazy-loads either the diff panel or a sandboxed iframe-basedBrowserPanel.Introduces a persisted Zustand store (
browserPanelStore) to manage per-project browser tabs (add/close/switch/navigate) and wires new navigation/search param support (rpt) so the right panel can open to eitherdifforbrowser.Updates chat UX to support
browser.toggle(header button + keybinding) and routes terminal URL clicks into the in-app browser when available, falling back to opening externally otherwise.Written by Cursor Bugbot for commit d1f7c1e. This will update automatically on new commits. Configure here.
Note
Add in-app browser panel with tabbed right panel to chat view
browser.togglekeyboard shortcut and a globe icon toggle button in ChatHeader.tsx.rptquery param in diffRouteSearch.ts.📊 Macroscope summarized d1f7c1e. 9 files reviewed, 3 issues evaluated, 0 issues filtered, 2 comments posted
🗂️ Filtered Issues