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)
📝 Coding Plan
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 Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
github-actions
bot
added
the
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| (store) => store.decrementBlockingOverlayCount, | ||
| ); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
🟡 Medium ui/dialog.tsx:68
useEffect in DialogPopup increments blockingOverlayCount on mount, but this component is always rendered in the React tree regardless of whether the dialog is open. The count increments on the first parent render even when the dialog is closed, not when it visually opens. Consider moving the effect inside DialogPrimitive.Popup so it only runs when the portal actually mounts, or use the dialog's open state to conditionally apply the effect.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/ui/dialog.tsx around line 68:
`useEffect` in `DialogPopup` increments `blockingOverlayCount` on mount, but this component is always rendered in the React tree regardless of whether the dialog is open. The count increments on the first parent render even when the dialog is closed, not when it visually opens. Consider moving the effect inside `DialogPrimitive.Popup` so it only runs when the portal actually mounts, or use the dialog's open state to conditionally apply the effect.
Evidence trail:
1. dialog.tsx lines 51-73: `DialogPopup` function component with `useEffect` that increments/decrements `blockingOverlayCount` on mount/unmount, defined OUTSIDE of `DialogPrimitive.Popup`
2. dialog.tsx lines 12-15: `Dialog = DialogPrimitive.Root`, `DialogPortal = DialogPrimitive.Portal`
3. PullRequestThreadDialog.tsx lines 173-181: Usage pattern showing `<Dialog open={open}><DialogPopup>...</DialogPopup></Dialog>` where `DialogPopup` is always in the JSX tree
4. Base UI Dialog documentation (https://base-ui.com/react/components/dialog): Dialog.Root "Groups all parts of the dialog. Doesn't render its own HTML element" (renders children unconditionally); Dialog.Portal `keepMounted: false` default affects DOM rendering, not React component mounting
apps/web/src/components/ChatView.tsx
Outdated
| : "Toggle diff panel"} | ||
| </TooltipPopup> | ||
| </Tooltip> | ||
| <ToggleGroup className="shrink-0" variant="outline" size="xs"> |
There was a problem hiding this comment.
🟠 High components/ChatView.tsx:4367
The ToggleGroup at line 4367 is uncontrolled (no value prop), but the inner Toggle components pass pressed props based on selectedSidePanel. In base-ui, when a Toggle is inside a ToggleGroup, it ignores the pressed prop and derives its state from the group's internal groupValue (see the controlled logic in the base-ui Toggle reference). This causes the visual toggle state to desynchronize from selectedSidePanel when it's set programmatically or via keyboard shortcuts. The ToggleGroup should be controlled with value={selectedSidePanel ? [selectedSidePanel] : []} and onValueChange to stay in sync with the parent state.
- <ToggleGroup className="shrink-0" variant="outline" size="xs">
+ <ToggleGroup
+ className="shrink-0"
+ variant="outline"
+ size="xs"
+ value={selectedSidePanel ? [selectedSidePanel] : []}
+ onValueChange={(value) => {
+ onSelectSidePanel(value[0] ?? null);
+ }}
+ >🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/ChatView.tsx around line 4367:
The `ToggleGroup` at line 4367 is uncontrolled (no `value` prop), but the inner `Toggle` components pass `pressed` props based on `selectedSidePanel`. In base-ui, when a `Toggle` is inside a `ToggleGroup`, it ignores the `pressed` prop and derives its state from the group's internal `groupValue` (see the `controlled` logic in the base-ui `Toggle` reference). This causes the visual toggle state to desynchronize from `selectedSidePanel` when it's set programmatically or via keyboard shortcuts. The `ToggleGroup` should be controlled with `value={selectedSidePanel ? [selectedSidePanel] : []}` and `onValueChange` to stay in sync with the parent state.
Evidence trail:
- ChatView.tsx lines 4367-4415: ToggleGroup with no `value` prop (uncontrolled), Toggle components with `value` and `pressed` props
- ChatView.tsx lines 4373, 4397: `pressed={selectedSidePanel === "diff"}` and `pressed={selectedSidePanel === "browser"}`
- ChatView.tsx lines 1405-1406: `forcedSelectedSidePanel` from URL parameters sets `selectedSidePanel`
- ChatView.tsx line 2404: keyboard shortcut calls `onSelectSidePanel()` programmatically
- apps/web/package.json line 17: `"@base-ui/react": "^1.2.0"`
- apps/web/src/components/ui/toggle-group.tsx: local wrapper using `@base-ui/react/toggle-group`
- GitHub issue https://github.com/mui/base-ui/issues/3465: confirms "Toggle checks if the provided prop value is truthy, and only if so, it will check if the groupContext contains it" - Toggle derives pressed state from groupContext when it has a value prop
| ); | ||
| } | ||
|
|
||
| function isValidBrowserTab(tab: BrowserTab): boolean { |
There was a problem hiding this comment.
🟢 Low src/browserStateStore.ts:39
isValidBrowserTab defensively checks that tab.url is a string before accessing it, but calls tab.id.trim() without a type guard. If persisted data contains a malformed tab where id is null, undefined, or not a string, this throws a TypeError. Consider adding typeof tab.id === "string" before calling .trim().
-function isValidBrowserTab(tab: BrowserTab): boolean {
- return tab.id.trim().length > 0 && typeof tab.url === "string" && tab.url.length > 0;
-}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/browserStateStore.ts around line 39:
`isValidBrowserTab` defensively checks that `tab.url` is a string before accessing it, but calls `tab.id.trim()` without a type guard. If persisted data contains a malformed tab where `id` is `null`, `undefined`, or not a string, this throws a `TypeError`. Consider adding `typeof tab.id === "string"` before calling `.trim()`.
Evidence trail:
apps/web/src/browserStateStore.ts lines 39-41 (REVIEWED_COMMIT) - shows `isValidBrowserTab` function with `tab.id.trim()` called without type guard, but `typeof tab.url === "string"` check applied to `tab.url`.
apps/web/src/browser.ts lines 3-12 - shows `BrowserTab` type with `id: string` and `url: string` (both typed as non-nullable strings), indicating the runtime checks are for defending against corrupted persisted data.
apps/web/src/browserStateStore.ts lines 1-5 - shows imports from `zustand/middleware` with `persist` and `createJSONStorage`, confirming this is handling persisted data.
…-browser # Conflicts: # apps/web/src/components/ChatView.tsx
|
This is awesome. Thank you |
github-actions
bot
added
the
size:XXL
2 participants
I read the README and understand you are not actively accepting contributions right now. I built this for myself, but I’m sharing it in case any part of it is useful as a starting point or source of ideas. This PR also includes a reconstruction plan and screenshots in case they are helpful.
What Changed
This PR adds an in-app browser to the chat right sidebar.
Main pieces:
BrowserPanelshell in the web app with:mod+btoggle browsermod+tnew browser tabmod+wclose active browser tabbrowserManagerthat renders real browser content withWebContentsView..plans/18-browser-panel-shell-and-runtime.md.Why
This adds a built-in browser directly inside the app so users can preview local apps and external pages without leaving the chat workflow. #37
The browser lives in the existing right sidebar so it fits naturally next to the diff panel instead of introducing a separate surface. Browser tabs and right-panel visibility are remembered per thread, which makes the feature usable while moving between conversations.
On desktop, the browser is backed by a real Electron runtime rather than a fake preview shell, so the panel can render actual web content. To keep resource usage predictable, hidden tabs are bounded by a small warm-tab cache and older hidden tabs are evicted and later restored by
reloading their URL.
UI Changes
This PR changes the chat header and right sidebar UI, and adds live browser rendering in the desktop app.
in-app-browser.mp4
Browser toggle button
Browser panel
Browser panel of small window
Browser view hidden when dialogs are open (because it's impossible to overlay the web view)
Checklist
Note
Implement in-app browser panel with tab management and native Electron WebContents integration
BrowserPanelUI component with tab strip, address bar, and navigation controls (back/forward/reload, open external), displayed as a right panel alongside the existing diff viewer in the chat thread route.BrowserManagerin the Electron main process (browserManager.ts) that manages up to 3 liveWebContentsViewtabs per window, attaches/detaches them based on bounds and visibility, and emitstab-stateevents on navigation, title, favicon, load, and crash.desktopBridgemethods (preload.ts) forensureTab,navigate,goBack,goForward,reload,closeTab,syncHost,clearThread, andonBrowserEvent.useBrowserStateStore) and right panel selection state (useRightPanelStateStore), both stored in localStorage via Zustand.Mod+Bto toggle the browser panel,Mod+Tfor new tab,Mod+Wto close the current tab.ChatHeaderto show a segmented diff/browser toggle control replacing the previous diff-only toggle.ResizeObserver,MutationObserver, and an animation-frame loop; any layout timing issues could cause the native view to appear misaligned.Macroscope summarized cbd79d3.