Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request refactors and extends both Electron and Vite components. In Electron, IPC communication is enhanced, with new handlers for window button positioning, tab navigation, and omnibox integration. A new Omnibox class is introduced along with updates to window management logic and path handling via a new constant. In Vite, numerous new React components, hooks, and utility functions are added to build a richer browser UI—including omnibox pages, enhanced sidebar functionality, and improved routing—along with updates to dependencies and build configurations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Menu
participant MainProcess
participant Omnibox
participant UI
User->>Menu: Click "New Tab" (omnibox mode)
Menu->>MainProcess: Send IPC event for omnibox tab creation
MainProcess->>Omnibox: Conditionally create Omnibox instance
Omnibox->>MainProcess: Load interface & inject dynamic CSS
MainProcess->>UI: Update window to display omnibox
UI-->>User: Render omnibox input and suggestions
User->>UI: Enter query
UI->>Omnibox: Process input (debounced) for suggestions
Omnibox->>UI: Return sorted/culled match results
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (49)
vite/src/routes/omnibox/index.tsx (1)
1-13: Well-structured React entry point for the Omnibox feature.The file follows standard React patterns by using StrictMode and a ThemeProvider wrapper for the main App component.
Consider adding a null check before accessing the root element:
-createRoot(document.getElementById("root")!).render( +const rootElement = document.getElementById("root"); +if (!rootElement) { + throw new Error("Root element not found"); +} +createRoot(rootElement).render(This would provide a more explicit error message if the root element is missing.
vite/src/routes/omnibox/index.html (1)
1-12: Clean HTML structure for the Omnibox feature.The HTML file correctly sets up the document structure with appropriate meta tags and creates a mounting point for the React application.
Consider adding the "dark" class to the body element for consistency with the new tab page:
- <body> + <body class="dark">This would ensure a consistent theme across different parts of the application.
electron/webpack.renderer.config.js (1)
3-16: TypeScript configuration looks good, but consider type checking implications.The added TypeScript configuration uses
transpileOnly: true, which improves build speed but skips type checking during compilation. This is a valid approach, especially if type checking is performed elsewhere in your workflow (like in your IDE or a separate CI process).If you don't have a separate type checking step in your build process, you might want to consider these alternatives:
- options: { - transpileOnly: true - } + options: { + // Option 1: Enable type checking in Webpack + transpileOnly: false + }Or alternatively, add a separate type checking script in package.json:
"scripts": { "typecheck": "tsc --noEmit", // other scripts... }vite/src/routes/omnibox/page.tsx (1)
1-16: Clean and well-structured component hierarchy.The component structure follows good React patterns with clear separation of concerns. The
OmniboxApphandles rendering the Omnibox while theAppcomponent provides the necessary context viaBrowserProvider.Consider adding a brief JSDoc comment to describe the purpose of each component for better documentation:
+/** + * Renders the Omnibox component without context + */ function OmniboxApp() { return <Omnibox />; } +/** + * Main Omnibox page component that provides browser context + */ function App() { return ( <BrowserProvider> <OmniboxApp /> </BrowserProvider> ); }vite/src/components/ui/skeleton.tsx (1)
1-13: Well-implemented Skeleton component with flexible styling options.The component follows best practices by:
- Accepting all standard div props
- Using a utility for class name composition
- Providing sensible default styles while allowing customization
- Using data-slot for targeting in CSS or testing
Consider adding a brief JSDoc comment to document the component's purpose and usage:
import { cn } from "@/lib/utils" +/** + * Skeleton component for displaying loading states + * + * @example + * <Skeleton className="h-10 w-full" /> + */ function Skeleton({ className, ...props }: React.ComponentProps<"div">) { return ( <div data-slot="skeleton" className={cn("bg-accent animate-pulse rounded-md", className)} {...props} /> ) } export { Skeleton }vite/src/components/ui/input.tsx (1)
10-15: Consider extracting the long className string for better readabilityThe className chain is quite lengthy which might make it difficult to maintain. Consider extracting these styles into constants or using a design system pattern to organize them better.
+ const baseStyles = "file:text-foreground placeholder:text-muted-foreground selection:bg-primary selection:text-primary-foreground dark:bg-input/30 border-input flex h-9 w-full min-w-0 rounded-md border bg-transparent px-3 py-1 text-base shadow-xs transition-[color,box-shadow] outline-none file:inline-flex file:h-7 file:border-0 file:bg-transparent file:text-sm file:font-medium disabled:pointer-events-none disabled:cursor-not-allowed disabled:opacity-50 md:text-sm" + const focusStyles = "focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px]" + const invalidStyles = "aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive" className={cn( - "file:text-foreground placeholder:text-muted-foreground selection:bg-primary selection:text-primary-foreground dark:bg-input/30 border-input flex h-9 w-full min-w-0 rounded-md border bg-transparent px-3 py-1 text-base shadow-xs transition-[color,box-shadow] outline-none file:inline-flex file:h-7 file:border-0 file:bg-transparent file:text-sm file:font-medium disabled:pointer-events-none disabled:cursor-not-allowed disabled:opacity-50 md:text-sm", - "focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px]", - "aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive", + baseStyles, + focusStyles, + invalidStyles, className )}vite/src/components/browser-ui/sidebar/address-bar.tsx (2)
8-46: Rename FakeAddressBar to a more descriptive nameThe component name "FakeAddressBar" implies it's not a real address bar, which might be confusing. Consider a more descriptive name like "SidebarAddressInput" or "ClickableAddressBar" to better indicate its purpose.
-function FakeAddressBar() { +function SidebarAddressInput() { const { addressUrl } = useBrowser(); const { open } = useSidebar(); // Rest of component code... } export function SidebarAddressBar() { return ( <SidebarGroup className="pt-0"> - <FakeAddressBar /> + <SidebarAddressInput /> </SidebarGroup> ); }
20-31: Consider using constants for omnibox dimensionsThe hardcoded multipliers (2x width, 8x height) seem arbitrary and could lead to inconsistent UI across different screen sizes. Consider using named constants or a more responsive approach.
+const OMNIBOX_WIDTH_MULTIPLIER = 2; +const OMNIBOX_HEIGHT_MULTIPLIER = 8; + function FakeAddressBar() { // ...existing code... showOmnibox( { x: inputBoxBounds.x, y: inputBoxBounds.y, - width: inputBoxBounds.width * 2, - height: inputBoxBounds.height * 8 + width: inputBoxBounds.width * OMNIBOX_WIDTH_MULTIPLIER, + height: inputBoxBounds.height * OMNIBOX_HEIGHT_MULTIPLIER }, { currentInput: addressUrl, openIn: "current" } );vite/src/components/old-ui/tab-list.tsx (2)
18-22: Remove console.log statement before deploymentDebug logging statements should be removed from production code to avoid cluttering the console and potentially exposing sensitive information.
- // Log for debugging - console.log("TabList render:", { - tabsCount: tabs.length, - tabIds: tabs.map((t) => t.id), - activeTabs: tabs.filter((t) => t.active).map((t) => t.id) - });
35-47: Good improvement to filter tabs by windowThis change properly filters tabs that don't belong to the current window, improving the user experience. For better performance, consider filtering the tabs array before mapping.
const { currentWindow } = useBrowser(); + + // Filter tabs that belong to the current window + const windowTabs = currentWindow?.id + ? tabs.filter(tab => tab.windowId === currentWindow.id) + : []; return ( <div className="w-full h-8 min-h-[calc(env(titlebar-area-height)+1px)]"> <div className="fixed left-[env(titlebar-area-x)] top-[env(titlebar-area-y)] w-[env(titlebar-area-width)] h-8 min-h-[calc(env(titlebar-area-height)+1px)] flex flex-row"> {/* Tab List */} <ul ref={tabListRef} className="h-full flex flex-row min-w-0 overflow-x-none"> <AnimatePresence initial={false}> - {tabs.map((tab) => { - // If tab is not in this window, don't render it - if (tab.windowId !== currentWindow?.id) { - return null; - } + {windowTabs.map((tab) => { return ( <Tab key={`tab-${tab.id}`} // ...rest of props /> ); })} </AnimatePresence> </ul>vite/src/index.css (2)
12-13: Consider revisiting the use of !important for background-colorWhile making the background transparent makes sense for an Electron application, using
!importantmight lead to style specificity issues in the future. Consider whether this can be achieved through more specific selectors or restructuring the CSS hierarchy.
142-144: Consider consolidating duplicate transparent background declarationsThis declaration duplicates the same property already set on
:rootat line 13. Consider consolidating these styles to improve maintainability::root { /* ... */ background-color: transparent !important; /* ... */ } -body { - background-color: transparent !important; -}Alternatively, if both are intentionally needed, add a comment explaining why.
vite/src/components/browser-ui/sidebar/navigation-buttons.tsx (1)
40-44: Consider adding a more descriptive comment for dropdown open logicWhile there is a comment explaining the dropdown open behavior, it could be more descriptive about why this approach was chosen:
// Keep auto-deselect, but disable the dropdown menu from being open as we have our own custom logic for opening it +// We only want the dropdown to open on right-click (context menu), not on regular click if (open === true) return; setOpen(open);vite/src/components/browser-ui/main.tsx (2)
17-19: Consider making sidebar configuration options dynamic.You're initializing state variables for collapseMode, variant, and side with useState, but there doesn't appear to be any functionality to change these values. If these settings are intended to be configurable, consider adding UI controls or loading preferences from storage.
38-55: Optimize loading indicator rendering.The motion.div animation runs continuously even when it's not visible. Consider conditionally rendering the entire motion element instead of just its container to save resources:
- {isActiveTabLoading && ( - <div className="w-28 h-1 bg-gray-200/30 dark:bg-white/10 rounded-full overflow-hidden"> - <motion.div - className="h-full bg-gray-800/90 dark:bg-white/90 rounded-full" - initial={{ x: "-100%" }} - animate={{ x: "100%" }} - transition={{ - duration: 1, - ease: "easeInOut", - repeat: Infinity, - repeatType: "loop", - repeatDelay: 0.1 - }} - /> - </div> - )} + {isActiveTabLoading && ( + <div className="w-28 h-1 bg-gray-200/30 dark:bg-white/10 rounded-full overflow-hidden"> + <motion.div + className="h-full bg-gray-800/90 dark:bg-white/90 rounded-full" + initial={{ x: "-100%" }} + animate={{ x: "100%" }} + transition={{ + duration: 1, + ease: "easeInOut", + repeat: Infinity, + repeatType: "loop", + repeatDelay: 0.1 + }} + /> + </div> + )}vite/src/components/browser-ui/browser-sidebar.tsx (3)
6-7: Combine duplicate imports.The imports from
@/lib/floware duplicated on lines 6 and 7. These can be combined into a single import statement for better code organization.- import { setWindowButtonPosition } from "@/lib/flow"; - import { setWindowButtonVisibility } from "@/lib/flow"; + import { setWindowButtonPosition, setWindowButtonVisibility } from "@/lib/flow";
22-35: Improve effect dependency to handle titlebar reference availability.The useEffect for updating window button position only runs when
variantchanges, but it depends ontitlebarRef.currentwhich might not be available immediately. Consider adding theopenstate to the dependency array to ensure the effect runs when the sidebar is opened.useEffect(() => { const titlebar = titlebarRef.current; if (titlebar) { const titlebarBounds = titlebar.getBoundingClientRect(); setWindowButtonPosition({ x: titlebarBounds.x, y: titlebarBounds.y }); } - }, [variant]); + }, [variant, open]);
46-49: Consider adding a fallback for CSS environment variables.The calculation for titlebar height uses CSS environment variables that might not be available in all environments. Consider adding fallback values to ensure the layout works everywhere.
- className="h-[calc(env(titlebar-area-y)+env(titlebar-area-height)+1px-1.5rem)] w-full app-drag" + className="h-[calc(env(titlebar-area-y, 0px)+env(titlebar-area-height, 28px)+1px-1.5rem)] w-full app-drag"vite/src/components/browser-ui/sidebar/tabs.tsx (1)
47-57: Optimize the tabs array mapping and reversing.The current implementation recreates the reversed array on every render, which can be inefficient for large numbers of tabs. Consider using useMemo to optimize this operation:
export function SidebarTabs() { const { tabs } = useBrowser(); + + const reversedTabs = useMemo(() => [...tabs].reverse(), [tabs]); return ( <SidebarGroup> <SidebarGroupLabel className="select-none">Tabs</SidebarGroupLabel> <SidebarMenu> <NewTabButton /> - {tabs.map((tab) => <SidebarTab key={tab.id} tab={tab} />).reverse()} + {reversedTabs.map((tab) => <SidebarTab key={tab.id} tab={tab} />)} </SidebarMenu> </SidebarGroup> ); }Don't forget to add the import:
import { useMemo } from "react";vite/src/components/browser-ui/sidebar/tab.tsx (1)
36-40: Double-check event propagation.
e.preventDefault()stops the default action for the event, but if you also need to halt any parent click handlers, consider callinge.stopPropagation().vite/src/components/browser-ui/sidebar/action-buttons.tsx (2)
37-48: Ensure error handling for status retrieval.
When the promise fromgetTabNavigationStatus(tabId)fails or returns invalid data, it gracefully exits. If partial data is returned, you may need more robust error handling or logs for debugging.
56-59: Await or handlestopLoadingTabresponse.
stopLoadingTabreturns a promise that is neither awaited nor wrapped in error handling. Consider handling or logging the result to detect potential failures.- stopLoadingTab(activeTab.id); + stopLoadingTab(activeTab.id) + .catch((err) => { + console.error("Failed to stop loading tab:", err); + });vite/package.json (1)
21-22: Consider using semver for the cmdk dependencyWhile the other dependencies use semver ranges (
^), the cmdk dependency is pinned to an exact version. This might cause issues if you need to apply security patches without manually updating the version.- "cmdk": "1.0.0", + "cmdk": "^1.0.0",vite/src/components/omnibox/main.tsx (3)
34-44: Consider using async/await for getCurrentTabThe Promise-based approach works but could be more readable with async/await.
-function getCurrentTab(): Promise<chrome.tabs.Tab | null> { - return new Promise((resolve) => { - chrome.tabs.query({ active: true, currentWindow: true }, (tabs) => { - if (tabs[0]) { - resolve(tabs[0]); - } else { - resolve(null); - } - }); - }); -} +async function getCurrentTab(): Promise<chrome.tabs.Tab | null> { + const tabs = await chrome.tabs.query({ active: true, currentWindow: true }); + return tabs[0] || null; +}
136-148: Simplify switch statement by removing redundant casesThe switch statement has several cases that fall through to the default case, which could be simplified.
const getMatchIcon = (match: OmniboxMatch) => { switch (match.type) { case "search": return <SearchIcon className="h-5 w-5 text-black/80 dark:text-white/80" />; - case "history": - case "bookmark": - case "verbatim": - case "navigation": - case "tab": default: return <GlobeIcon className="h-5 w-5 text-black/80 dark:text-white/80" />; } };🧰 Tools
🪛 Biome (1.9.4)
[error] 140-140: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 141-141: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 142-142: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 143-143: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 144-144: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
150-164: Simplify action text switch statementSimilar to the icon switch, this could be simplified by removing redundant cases.
const getMatchAction = (match: OmniboxMatch) => { switch (match.type) { case "search": return "Search"; case "tab": return "Switch to Tab"; - case "history": - case "bookmark": - case "verbatim": - case "navigation": default: return "Navigate"; } };🧰 Tools
🪛 Biome (1.9.4)
[error] 157-157: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 158-158: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 159-159: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 160-160: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
vite/src/components/ui/tooltip.tsx (2)
21-29: Automatic provider usage withinTooltip.
WrappingTooltipPrimitive.Rootin theTooltipProviderensures the tooltip has a provider context. This is acceptable, though if you ever create many tooltips at once, you might consider having a single provider at a higher level to avoid multiple nested providers.
37-59: Consider adding or verifying accessibility features.
While Radix handles most aria attributes, confirm your custom classes and structure do not disrupt keyboard accessibility or screen reader announcements.vite/src/components/main/browser-context.tsx (1)
279-295: Window state management.
Minimizing, maximizing, and closing the window is handled seamlessly with Chrome’s API calls, but consider providing user feedback or confirming if data is unsaved before closing.vite/src/lib/url.ts (2)
12-62: Robust URL parsing with a clear fallback strategy.
- Returning
nullfor empty or invalid input is a clear approach, but ensure all callers handlenulldefensively.- The domain pattern regex is helpful, though heavily reliant on TLD-based validation. If you need to handle advanced domain formats or punycode, consider adding more flexible checks.
- const urlRegex = /^([-a-zA-Z0-9@:%._+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b(?:[-a-zA-Z0-9()@:%_+.~#?&//=]*))$/; + // Could consider expansions or punycode if you plan to handle internationalized domains
120-142: Neat utility for simplifying URLs.
- Removing “www.” is a common approach to presenting cleaner hostnames.
- Consider additional logic for other subdomains or advanced use cases if needed.
- Good fallback to the original string when parsing fails.
electron/browser/tabs.ts (1)
260-271: IPC handler for updating page bounds.
- Properly updates global
windowsBoundsand emits a targeted event for the impacted window only.- Consider verifying that
boundsare valid (e.g., >= 0 sizes) to avoid unexpected geometry.+ if (bounds.width <= 0 || bounds.height <= 0) { + console.warn("Received invalid bounds", bounds); + return; + }vite/src/components/ui/command.tsx (2)
21-43: Potential ARIA improvements for accessibility.
CommandDialoguses a title and description, which is excellent for screen readers. However, if you anticipate custom or advanced accessibility needs (e.g., dynamic updates inside the dialog), further ARIA attributes might be beneficial. Consider verifying correctness with a screen reader test.
45-59: Verify icon usage for accessibility.
SearchIconis visually decorative, but it might be worth setting anaria-hidden="true"attribute or providing an accessible label if needed for assistive technologies.vite/src/components/ui/dialog.tsx (1)
27-31: Clarify usage of DialogClose.
WhileDialogCloseis convenient, some apps may need custom close logic (e.g., prompting "Are you sure?" on unsaved changes). Ensure your usage meets your UX requirements.electron/preload.ts (1)
16-53: Ensure consistent return types from exposed methods.
ThegetTabNavigationStatusmethod usesipcRenderer.invoke(returns a promise), while others usesend. Ensure consumers offlow.interfacedon’t expect a promise where none is returned, and handle promise-based vs. non-promise-based calls accordingly.electron/browser/main.ts (1)
114-124: Short-Lived Tab Hack
Creating and destroying a tab is a practical workaround to ensureelectron-chrome-extensionsdetects a new window. Consider introducing a slightly longer or configurable delay to reduce race conditions if the user interacts with the blank tab.electron/browser/omnibox.ts (1)
101-120:showMethod & Keep-On-Top Logic
Periodically re-adding the omnibox view ensures it remains on top. You might reduce the interval or cancel it more aggressively to improve performance.vite/src/lib/flow.ts (1)
27-71: Consider error handling on asynchronous methods.
Your interface methods likegetTabNavigationStatusare returning promises, but the interface doesn't specify how errors are handled or propagated. Consider defining structured error handling or signaling for these methods to ensure consistent handling of potential failures (e.g., invalid tab IDs, internal IPC errors, etc.).vite/src/lib/omnibox.ts (2)
66-79: Clarify behavior ofparseInput.
WhenparseInputreturnsOmniboxInputType.UNKNOWN, downstream logic might treat it as either a URL or a query. Ensure calling code handles this condition consistently.
200-241: Handle concurrency and partial results gracefully.
handleOmniboxInputasynchronously fetches search suggestions. If user input changes rapidly or multiple requests are triggered, consider aborting prior requests or checking whether the input changed before updating suggestions to avoid race conditions and stale UI states.vite/src/components/ui/sidebar.tsx (4)
84-95: Provide discoverability for the keyboard shortcut.
Users might not know about the Ctrl/Cmd + B toggle. Consider providing a small tooltip, an in-app message, or documentation in the UI, improving discoverability of this shortcut.
136-233: Extract large component logic for maintainability.
TheSidebarcomponent handles resizing, mobile vs. desktop logic, floating vs. inset styles, and more. Consider splitting it into smaller components or hooks to reduce complexity and enhance maintainability.
442-463: Centralize theming and variant definitions.
You usecvato define multiple variant states and sizes. Make sure these align with your global design tokens to maintain consistency across the app. Also ensure your variant definitions remain DRY by referencing any shared spacing or color tokens in a single location.
464-507: Review tooltip logic on collapsed state.
While the tooltip is hidden ifstate !== "collapsed", consider using media queries or user preference for showing tooltips in more states, especially for accessibility. You might allow an advanced option that shows tooltips even when expanded.Would you like me to open an issue to track making tooltips accessible in all sidebar states?
vite/src/components/ui/resizable-sidebar.tsx (4)
57-95: Consider loading sidebar state from the cookie.
You set a cookie withsidebar_statebut never read it upon mounting. If your goal is to preserve the user's preference across page reloads, consider parsing and initializing theopenstate from the cookie as well, or switch to localStorage if that better meets your requirements.
172-221: Mobile fallback design.
The usage of a Sheet component for the mobile variant and a simple<div>for the desktop variant is a neat approach. Make sure to confirm that both experiences are tested thoroughly, particularly focus trapping or gestures on mobile devices.
293-296: Ref handling can be improved with forwardRef.
Because you're directly passingrefas a prop, it might be more idiomatic to convertSidebarRailto aforwardRefcomponent. That ensures the TypeScript types for refs are enforced seamlessly and avoids potential shadowing of the standardrefprop.- function SidebarRail({ - className, - enableDrag = true, - ref, - ...props - }: React.ComponentProps<"button"> & { enableDrag?: boolean }) { + const SidebarRail = React.forwardRef< + HTMLButtonElement, + React.ComponentProps<"button"> & { enableDrag?: boolean } + >(({ + className, + enableDrag = true, + ...props + }, forwardedRef) => { // ... const combinedRef = React.useMemo( - () => mergeButtonRefs([ref, dragRef]), - [ref, dragRef] + () => mergeButtonRefs([forwardedRef, dragRef]), + [forwardedRef, dragRef] ); // ... } );
325-325: Remove commented-out toggle logic if not needed.
There's a leftover commented-out line// onClick={toggleSidebar}. Consider removing it to keep the codebase clean and ensure clarity for future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (44)
electron/browser/main.ts(11 hunks)electron/browser/menu.ts(3 hunks)electron/browser/omnibox.ts(1 hunks)electron/browser/tabs.ts(6 hunks)electron/index.ts(2 hunks)electron/modules/flags.ts(2 hunks)electron/preload.ts(1 hunks)electron/webpack.renderer.config.js(1 hunks)vite/package.json(1 hunks)vite/src/components/browser-ui/browser-content.tsx(1 hunks)vite/src/components/browser-ui/browser-sidebar.tsx(1 hunks)vite/src/components/browser-ui/main.tsx(1 hunks)vite/src/components/browser-ui/sidebar/action-buttons.tsx(1 hunks)vite/src/components/browser-ui/sidebar/address-bar.tsx(1 hunks)vite/src/components/browser-ui/sidebar/navigation-buttons.tsx(1 hunks)vite/src/components/browser-ui/sidebar/tab.tsx(1 hunks)vite/src/components/browser-ui/sidebar/tabs.tsx(1 hunks)vite/src/components/main/browser-context.tsx(1 hunks)vite/src/components/old-ui/tab-list.tsx(2 hunks)vite/src/components/omnibox/main.tsx(1 hunks)vite/src/components/ui/button.tsx(1 hunks)vite/src/components/ui/command.tsx(1 hunks)vite/src/components/ui/dialog.tsx(1 hunks)vite/src/components/ui/dropdown-menu.tsx(1 hunks)vite/src/components/ui/input.tsx(1 hunks)vite/src/components/ui/resizable-sidebar.tsx(1 hunks)vite/src/components/ui/separator.tsx(1 hunks)vite/src/components/ui/sheet.tsx(1 hunks)vite/src/components/ui/sidebar.tsx(1 hunks)vite/src/components/ui/skeleton.tsx(1 hunks)vite/src/components/ui/tooltip.tsx(1 hunks)vite/src/hooks/use-mobile.ts(1 hunks)vite/src/hooks/use-sidebar-resize.tsx(1 hunks)vite/src/index.css(3 hunks)vite/src/lib/flow.ts(1 hunks)vite/src/lib/merge-button-refs.ts(1 hunks)vite/src/lib/omnibox.ts(1 hunks)vite/src/lib/search.ts(1 hunks)vite/src/lib/url.ts(3 hunks)vite/src/routes/main/page.tsx(1 hunks)vite/src/routes/new/index.html(1 hunks)vite/src/routes/omnibox/index.html(1 hunks)vite/src/routes/omnibox/index.tsx(1 hunks)vite/src/routes/omnibox/page.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (18)
vite/src/components/browser-ui/sidebar/navigation-buttons.tsx (3)
vite/src/components/browser-ui/sidebar/action-buttons.tsx (2) (2)
NavigationEntryWithIndex(9-9)SidebarActionButton(11-24)vite/src/components/ui/dropdown-menu.tsx (4) (4)
DropdownMenu(240-240)DropdownMenuTrigger(242-242)DropdownMenuContent(243-243)DropdownMenuItem(246-246)vite/src/lib/flow.ts (1) (1)
goToNavigationEntry(117-119)
vite/src/components/browser-ui/sidebar/address-bar.tsx (3)
vite/src/components/main/browser-context.tsx (1) (1)
useBrowser(320-328)vite/src/lib/flow.ts (1) (1)
showOmnibox(122-124)vite/src/lib/url.ts (1) (1)
simplifyUrl(121-142)
vite/src/lib/url.ts (1)
vite/src/lib/search.ts (1) (1)
createSearchUrl(1-3)
vite/src/components/omnibox/main.tsx (3)
vite/src/components/ui/command.tsx (4) (4)
CommandItem(128-128)Command(122-122)CommandInput(124-124)CommandList(125-125)vite/src/lib/omnibox.ts (2) (2)
OmniboxMatch(23-38)handleOmniboxInput(203-240)vite/src/lib/flow.ts (1) (1)
hideOmnibox(126-128)
vite/src/components/browser-ui/browser-content.tsx (1)
vite/src/lib/flow.ts (2) (2)
PageBounds(1-6)setPageBounds(97-99)
vite/src/components/browser-ui/sidebar/action-buttons.tsx (2)
vite/src/lib/flow.ts (3) (3)
NavigationEntry(8-11)getTabNavigationStatus(109-111)stopLoadingTab(113-115)vite/src/components/browser-ui/sidebar/navigation-buttons.tsx (2) (2)
GoBackButton(74-91)GoForwardButton(93-110)
vite/src/components/browser-ui/sidebar/tabs.tsx (4)
vite/src/components/main/browser-context.tsx (1) (1)
useBrowser(320-328)vite/src/lib/omnibox.ts (1) (1)
getNewTabMode(4-6)vite/src/lib/flow.ts (1) (1)
showOmnibox(122-124)vite/src/components/browser-ui/sidebar/tab.tsx (1) (1)
SidebarTab(11-108)
vite/src/components/main/browser-context.tsx (1)
vite/src/lib/url.ts (2) (2)
transformUrl(85-119)parseAddressBarInput(68-83)
vite/src/components/browser-ui/main.tsx (2)
vite/src/components/ui/resizable-sidebar.tsx (3) (3)
useSidebar(735-735)SidebarInset(721-721)SidebarProvider(731-731)vite/src/components/browser-ui/browser-sidebar.tsx (1) (1)
BrowserSidebar(17-69)
electron/browser/menu.ts (3)
electron/browser/omnibox.ts (6) (6)
getNewTabMode(133-135)isOmniboxOpen(165-168)hideOmnibox(158-163)loadOmnibox(144-149)setOmniboxBounds(137-142)showOmnibox(151-156)vite/src/lib/omnibox.ts (1) (1)
getNewTabMode(4-6)electron/browser/main.ts (1) (1)
getFocusedWindow(225-227)
vite/src/components/browser-ui/browser-sidebar.tsx (5)
vite/src/components/browser-ui/main.tsx (3) (3)
CollapseMode(9-9)SidebarVariant(10-10)SidebarSide(11-11)vite/src/lib/flow.ts (2) (2)
setWindowButtonVisibility(105-107)setWindowButtonPosition(101-103)vite/src/components/browser-ui/sidebar/action-buttons.tsx (1) (1)
NavigationControls(26-74)vite/src/components/browser-ui/sidebar/address-bar.tsx (1) (1)
SidebarAddressBar(48-54)vite/src/components/browser-ui/sidebar/tabs.tsx (1) (1)
SidebarTabs(47-59)
vite/src/components/ui/command.tsx (1)
vite/src/components/ui/dialog.tsx (5) (5)
Dialog(125-125)DialogHeader(130-130)DialogTitle(133-133)DialogDescription(128-128)DialogContent(127-127)
electron/browser/tabs.ts (2)
vite/src/lib/flow.ts (1) (1)
PageBounds(1-6)electron/modules/flags.ts (1) (1)
FLAGS(7-17)
electron/browser/omnibox.ts (3)
vite/src/components/omnibox/main.tsx (1) (1)
Omnibox(46-224)vite/src/lib/omnibox.ts (1) (1)
getNewTabMode(4-6)vite/src/lib/flow.ts (2) (2)
showOmnibox(122-124)hideOmnibox(126-128)
vite/src/components/ui/resizable-sidebar.tsx (2)
vite/src/hooks/use-sidebar-resize.tsx (1) (1)
useSidebarResize(33-160)vite/src/lib/merge-button-refs.ts (1) (1)
mergeButtonRefs(7-19)
vite/src/lib/omnibox.ts (3)
electron/browser/omnibox.ts (1) (1)
getNewTabMode(133-135)vite/src/lib/url.ts (1) (1)
getURLFromInput(12-62)vite/src/lib/search.ts (2) (2)
createSearchUrl(1-3)getSearchSuggestions(25-30)
vite/src/components/ui/sidebar.tsx (7)
vite/src/hooks/use-mobile.ts (1) (1)
useIsMobile(5-19)vite/src/components/ui/tooltip.tsx (2) (2)
TooltipProvider(61-61)Tooltip(61-61)vite/src/components/ui/sheet.tsx (5) (5)
Sheet(129-129)SheetContent(132-132)SheetHeader(133-133)SheetTitle(135-135)SheetDescription(136-136)vite/src/components/ui/button.tsx (1) (1)
Button(59-59)vite/src/components/ui/input.tsx (1) (1)
Input(21-21)vite/src/components/ui/separator.tsx (1) (1)
Separator(28-28)vite/src/components/ui/skeleton.tsx (1) (1)
Skeleton(13-13)
vite/src/lib/flow.ts (1)
electron/browser/omnibox.ts (2) (2)
showOmnibox(151-156)hideOmnibox(158-163)
🪛 Biome (1.9.4)
vite/src/components/omnibox/main.tsx
[error] 140-140: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 141-141: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 142-142: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 143-143: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 144-144: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 157-157: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 158-158: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 159-159: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 160-160: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
vite/src/components/main/browser-context.tsx
[error] 88-88: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (140)
vite/src/routes/new/index.html (1)
8-8: Added dark theme to the new tab page.The addition of the "dark" class to the body element implements dark mode styling for the new tab page, ensuring a consistent theme across the application.
electron/modules/flags.ts (2)
4-4: New debug flag for disabling tab view.This flag provides a convenient way to disable the tab view functionality during development and debugging.
13-16: Well-documented flag implementation with proper formatting.The addition of the trailing comma after the previous property and the descriptive comment for the new debug flag follow good code practices.
vite/src/lib/merge-button-refs.ts (1)
1-19: Well-implemented ref merging utility with proper TypeScript typing.This utility function correctly handles both callback refs and object refs, following React's ref patterns. The TypeScript generic constraint ensures type safety for button elements.
vite/src/hooks/use-mobile.ts (1)
1-19: Well-implemented responsive hook for mobile detectionThis hook is cleanly implemented with proper event listener setup, initial state calculation, and cleanup on unmount. The mobile breakpoint constant makes the code more maintainable.
vite/src/components/ui/separator.tsx (1)
1-28: Well implemented Separator component using Radix UIThis is a clean implementation of a Separator component using Radix UI's primitive. The component:
- Appropriately forwards all props to the underlying primitive
- Sets sensible defaults for orientation and decorative props
- Uses the
cnutility for classname composition- Implements conditional styling based on orientation using data attributes
- Follows best practices with the "use client" directive for Next.js compatibility
vite/src/index.css (1)
57-59: Good addition for controlling app region dragging behaviorThe
.remove-app-dragclass is a useful counterpart to the existing.app-dragclass, providing fine-grained control over which elements can be used to drag the window.vite/src/components/browser-ui/sidebar/navigation-buttons.tsx (3)
20-72: Well-implemented NavigationButton with dropdown history functionalityThe
NavigationButtoncomponent is well structured with proper separation of concerns:
- Clear handling of navigation actions
- Good use of context menu for showing navigation history
- Appropriate state management for the dropdown menu
- Clear comment explaining the custom dropdown open behavior
The component elegantly handles both click navigation and right-click history access, which provides a good user experience.
74-91: Good component composition for GoBackButtonThe
GoBackButtoncomponent effectively utilizes the baseNavigationButtoncomponent with the appropriate props for back navigation. This demonstrates good component composition and code reuse.
93-110: Good component composition for GoForwardButtonSimilarly, the
GoForwardButtoncomponent follows the same pattern, reusing theNavigationButtoncomponent with forward navigation props. This consistency in implementation is excellent.vite/src/components/ui/button.tsx (2)
7-36: Comprehensive button variant system using class-variance-authorityThe
buttonVariantsconstant defines a robust set of styling options using the class-variance-authority library. This approach:
- Provides consistent styling across button variations
- Makes it easy to add new variants and sizes
- Follows modern CSS utility practices
- Handles focus states, disabled states, and SVG styling appropriately
The variant definitions are well-organized and cover common use cases (default, destructive, outline, etc.).
38-57: Well-implemented flexible Button component with Slot supportThe
Buttoncomponent implementation:
- Uses the Slot pattern for component composition
- Properly types props with React.ComponentProps and VariantProps
- Handles className merging correctly
- Correctly forwards props to the underlying component
This is a solid foundation for button elements throughout the application.
vite/src/components/browser-ui/browser-content.tsx (4)
1-57: Well-structured dimension tracking implementation.This component effectively manages its dimensions using ResizeObserver and updates the page bounds for the Flow interface. The cleanup logic in the useEffect hook properly prevents memory leaks by removing event listeners and observers.
58-69: Good implementation of DEBUG_SHOW_BOUNDS visualization.The debug overlay is a helpful development tool that only appears when DEBUG_SHOW_BOUNDS is true, showing the current dimensions in a non-intrusive way.
70-98: Well-implemented content rendering with proper animations.The AnimatePresence component provides smooth transitions between states, and the conditional rendering logic clearly handles both active tab and empty states. The use of motion.div with appropriate animation properties ensures a polished user experience.
103-104: Good use of memo for performance optimization.Wrapping the component with memo prevents unnecessary re-renders, which is a good practice for components that depend primarily on props and may be rendered frequently.
vite/src/components/browser-ui/main.tsx (2)
23-29: Good implementation of loading state management.The useEffect hook properly updates the loading state based on the activeTab status change, which is a clean approach to derive UI state from application state.
65-71: Proper context provider implementation.The BrowserUI component correctly wraps the internal component with SidebarProvider, ensuring that the sidebar context is available throughout the component tree.
vite/src/components/browser-ui/browser-sidebar.tsx (1)
57-66: Well-implemented dynamic styling for SidebarRail.The conditional styling using the cn utility function is well-structured and covers all the possible combinations of open state, variant, and side. This ensures that the UI adapts correctly to different configurations.
vite/src/components/browser-ui/sidebar/tabs.tsx (1)
12-44: Well-structured NewTabButton with proper animation feedback.The NewTabButton component effectively handles the different modes of creating new tabs (omnibox or direct tab creation) and provides good visual feedback with spring animations. The separation of concerns is clear, making the component easy to understand and maintain.
vite/src/components/browser-ui/sidebar/tab.tsx (2)
31-34: Guard usage oftab.idmore thoroughly.
Iftab.iddoesn't exist or is invalid, calls tohandleTabClickwill be ignored. Consider adding logging or additional guard logic to detect and handle unexpectedtab.idvalues.
42-49: Confirm that middle-click is handled consistently across platforms.
Some devices might not send button code 1 for a middle click. If you aim for cross-platform consistency, consider a fallback or additional checks.vite/src/components/browser-ui/sidebar/action-buttons.tsx (1)
61-74: Good compositional structure.
Your layout and control grouping is clear and responsive. Overall, this code segment looks well-structured to manage sidebar actions.vite/src/lib/search.ts (1)
25-30: Handle network or parsing errors.
Iffetchorresponse.json()fails, the code could throw an error. Consider wrapping these calls in try/catch or adding error handling logic to gracefully handle failures.vite/src/routes/main/page.tsx (1)
4-12: Clean and modular approach.
By wrappingBrowserAppinBrowserProvider, you offload complex state management to a higher-level context. This is a solid design choice that keeps the component simple and maintainable.vite/src/hooks/use-sidebar-resize.tsx (6)
14-18: Clear and well-structured utility functionsThe parseWidth, toPx, and formatWidth utility functions are well-implemented and provide a clean abstraction for handling width conversions between different units.
30-31: Good usage of constants for cookie configurationUsing named constants for cookie parameters improves maintainability and readability.
33-53: Well-implemented state management with refsThe hook properly initializes refs for tracking drag state, positions, and thresholds. The persistWidth callback is correctly memoized and handles cookie storage appropriately.
56-74: Proper event handling in mouseDown callbackThe handleMouseDown callback is correctly memoized with the right dependencies and includes appropriate validation logic before initiating dragging operations.
76-153: Comprehensive resize logic and cleanupThe resize logic handles different units, enforces constraints, and includes auto-collapse functionality. The code properly cleans up event listeners to prevent memory leaks.
155-159: Clean hook return interfaceThe hook returns a minimal, focused set of values that make it easy to integrate with components.
vite/package.json (2)
13-17: Good selection of UI component librariesThe addition of Radix UI components provides accessible, unstyled components that work well with the existing styling approach.
26-26: Good addition of react-resizable-panelsThis package aligns well with the sidebar resize functionality implemented in the hooks.
electron/index.ts (4)
1-1: Updated imports for IPC functionalityThe import statement now includes the necessary Electron modules for the added IPC handlers.
18-29: Well-implemented tab loading controlThe IPC handler correctly retrieves the window and tab objects before stopping the loading, with proper null checks.
31-48: Comprehensive navigation status retrievalThe handler returns all necessary navigation data including history entries, active index, and navigation capabilities.
50-59: Robust navigation entry handlingThe handler includes proper validation before performing the navigation operation, preventing potential errors.
vite/src/components/omnibox/main.tsx (5)
17-32: Well-structured OmniboxItem componentThe component provides a consistent layout for search results with appropriate icon and action display.
51-54: Clean state and ref initializationThe component properly initializes state from URL parameters and sets up necessary refs.
56-73: Good keyboard accessibilityThe component handles focus and keyboard shortcuts appropriately, improving accessibility.
75-112: Robust request management with AbortControllerThe component correctly handles in-flight request cancellation and implements debouncing for input changes.
176-223: Well-structured command UI with appropriate stylingThe component uses the Command components effectively and applies appropriate styling and accessibility attributes.
vite/src/components/ui/tooltip.tsx (4)
1-2: Using"use client"successfully in Next.js.
This directive ensures that the file is rendered on the client side, which is required for using client-specific APIs or stateful components.
8-19: Encapsulation ofTooltipProvideris clear.
This component neatly wrapsTooltipPrimitive.Providerand applies a default delay, making it easy to configure tooltip timing across the application.
31-35:TooltipTriggerusage is straightforward.
Prop forwarding anddata-slot="tooltip-trigger"usage follow best practices for transparency and debugging.
61-61: Exporting all tooltip components together.
Exporting these components together simplifies import statements across the codebase.electron/browser/menu.ts (3)
2-3: Refined named imports forBrowserand omnibox utilities.
Switching to named imports improves clarity and ensures consistent usage of the imported symbols across the file.
85-105: Omnibox-based close logic looks consistent.
You correctly hide the omnibox or close the tab/window. The guardif (!win) return;already avoids null references, making an optional chain superfluous here.
108-108: Corrected Menu Label.
Renaming to"Toggle Developer Tools"reflects the plural form, matching common conventions.vite/src/components/main/browser-context.tsx (8)
1-4: Imports and Chrome extension environment setup.
UsingparseAddressBarInputandtransformUrlproperly sets up URL transformations for the context’s internal logic.
7-25: Well-definedBrowserContextTypeinterface.
It thoroughly lists the state and handlers the provider will supply, ensuring type safety for consumers of the context.
29-66: Proper initialization of state and references inBrowserProvider.
The stateful variables for tabs, active tab, and address URL are correctly set, and references likeaddressUrl_urlRefreduce unnecessary re-renders.
80-92: Fetching initial tabs is handled correctly.
Querying tabs on load ensures consistent state when the provider initializes.🧰 Tools
🪛 Biome (1.9.4)
[error] 88-88: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
94-114: Dynamic title logic is sound.
The approach to updating the title based on the active tab’s title is correct for immediate UI feedback.
128-145: Tab event listener setup with stable callbacks.
Using stable callbacks withuseCallbackand referencing the latest states is a clean approach, reducing subscription overhead and ensuring consistent event handling.
257-265: Cautious URL handling inhandleAddressUrlSubmit.
Parsing the address bar input and updating or creating a tab is correct. Confirm thatchrome.tabs.updateworks reliably on ephemeral tabs in certain extension contexts.
320-328:useBrowsercustom hook usage guard.
Throwing an error when outside the provider prevents incorrect usage of the context, ensuring that components cannot misuse the context inadvertently.vite/src/lib/url.ts (4)
1-1: Good import for creating search URLs.The
createSearchUrlimport is used appropriately to handle search fallback logic.
64-66: Simple utility function to check if input is a URL.This is straightforward and reuses
getURLFromInputeffectively.
68-83: Intuitive address bar parsing with proper search fallback.
- Returning an empty string for empty input might cause confusion in downstream usage. Confirm that no further logic depends on an actual URL.
- The fallback to
createSearchUrlis well-integrated and user-friendly.
5-5:Details
✅ Verification successful
Ensure whitelisted pages are valid and restricted.
Adding
"omnibox"towhitelistedPagesmight inadvertently permit unauthorized pages if there's no strict validation elsewhere.Would you like to confirm usage of
omniboxin the codebase to ensure it’s protected from injection or misconfigurations?
🏁 Script executed:
#!/bin/bash rg -A 3 "omnibox"Length of output: 10600
Action: Confirmed "omnibox" Whitelisting Usage
After verifying the references to
"omnibox"across the codebase—including its dedicated handling in files such asvite/src/lib/omnibox.ts,electron/browser/omnibox.ts, and its use in conditionals likeif (getNewTabMode() === "omnibox")—it appears that whitelisting"omnibox"is an intentional design choice. The omnibox mode is systematically managed through dedicated functions (e.g.,getNewTabMode(),showOmnibox(),hideOmnibox()) and UI components, reducing the risk of injection or misconfiguration.Please ensure that:
- The omnibox page continues to enforce proper access control and validation within its dedicated modules.
- Any future modifications to routing or page validation maintain these security measures.
electron/browser/tabs.ts (10)
2-2: Expanded Electron imports for IPC usage.Great to see
ipcMainincluded, as it enables communication for dynamic window sizing.
5-10: Defines clear bounding parameters for windows.
PageBoundsis a concise type. You might want to ensure coordinates and sizes are validated before usage to prevent off-screen rendering issues.
12-13: Maintains dynamic window bounds mapping and signaling.
windowsBoundsandwindowsBoundsEmitterenable robust tracking of custom bounds transitions across different windows.
23-23: Bounds listener property is well-defined.Initializes to
nullto prevent potential undefined usage.
32-47: Conditional creation of WebContentsView is flexible.
- Good approach to handle existing
webContentsif provided, or create a new one otherwise.- Ensure consistent usage of
webPreferencesmerging from both provided and default sources.
113-122: Starting bounds listener on show.
- This ensures the layout is responsive as soon as the tab is displayed.
- Hiding the tab view when
FLAGS.DEBUG_DISABLE_TAB_VIEWistrueis a useful debug feature.
125-130: Stopping bounds listener on hide.This prevents unnecessary event handling for invisible tabs, improving performance and clarity.
146-155: Applies dynamic layout and rounded corners.Updating bounds from
windowsBoundswhile setting aborderRadiusis consistent with a modern UI.
158-174: Sets up robust event listeners for resizing and custom bounds.
- Merging OS-level “resize” with custom bounds updates ensures all resizing triggers are captured.
- Good housekeeping by stopping old listeners first.
175-184: Graceful detachment of bounds listeners.Clears both “resize” and custom bounds events to prevent memory leaks or stale triggers.
vite/src/components/ui/sheet.tsx (11)
7-9: Sheet root wrapper for Radix UI.This is a straightforward pass-through to
SheetPrimitive.Root, enhancing composition.
11-15: Sheet trigger for opening the sheet.Provides a consistent “trigger” slot—follows Radix UI idioms nicely.
17-21: Sheet close for dismissing the sheet.Simple pass-through ensures consistent close behavior.
23-27: Sheet portal for managing DOM hierarchy.This ensures proper layering and accessibility.
29-43: Sheet overlay with configurable animations.The usage of Radix UI states (
data-[state=open]) to control animations is idiomatic and user-friendly.
45-80: Sheet content with variable side positioning.
- Good usage of utility classes for top/right/bottom/left transitions.
- The close button is well-labeled for screen readers.
82-90: Sheet header with flexible layout.Simple container for header elements.
92-100: Sheet footer placed at the bottom.Placing the footer automatically at the bottom using
mt-autois a neat approach for stacked layout.
102-113: Sheet title for accessible naming.Proper use of
SheetPrimitive.Title, plus a utility class for styling.
115-126: Sheet description for accessible details.Applies
text-muted-foregroundclass, ensuring text is visually distinct yet accessible.
128-137: Clean exports for all sheet components.Neatly grouped exports provide a predictable import structure for consumers.
vite/src/components/ui/command.tsx (3)
1-5: Consider verifying dependency usage and versions.
The code relies on thecmdkandlucide-reactlibraries. Please ensure that the versions specified in your project's package configuration (e.g.,package.json) match the features you're using here.
8-19: Looks good — well-structured component.
TheCommandfunctional component is well-defined, with clear separation of props and the utility functioncnfor conditional class merging. Implementation appears correct.
111-119: Well-thought-out approach for CommandShortcut.
TheCommandShortcutcomponent effectively displays keyboard shortcuts. No issues spotted.vite/src/components/ui/dialog.tsx (3)
9-13: Straightforward usage of Radix UI root.
TheDialogcomponent successfully wrapsDialogPrimitive.Rootwith minimal overhead, keeping the API consistent.
33-46: Overlay styling looks good.
The animated overlay usage withdata-[state=...]classes is solid, ensuring fluid transitions. No issues spotted here.
65-69: Validate close button accessibility.
The close button includes an icon and a visually hidden<span>Close</span>. This is good for screen readers, but confirm that it meets your specific accessibility standards for the chosen audience.electron/preload.ts (3)
4-8: Clear UI distinction checks.
TheisBrowserUIandisOmniboxUIvariables are a clean approach to gating features in the correct context.
10-13: Injected browser action.
CallinginjectBrowserAction()conditionally based onisBrowserUIis succinct and keeps the code organized.
19-42: Consider verifying async check or capturing errors.
Several interface methods callipcRenderer.send, which doesn’t provide a response. If you need confirmation or error handling, preferipcRenderer.invoke. Otherwise, confirm that a one-way send is sufficient.electron/browser/main.ts (12)
1-1: AddedipcMainImport Looks Good
ImportingipcMainis necessary for the newly introduced IPC handlers and appears correct.
14-14: Omnibox Import is Appropriate
ImportinggetNewTabModeandOmniboxis consistent with subsequent usage in this file. No issues found.
18-18: NewWEBPACK_ROOT_DIRConstant
Defining a distinct constant for the Webpack directory path helps improve clarity. This approach is maintainable and avoids repeated path construction.
72-72: Omnibox Property Acknowledged
Declaring a dedicatedomniboxproperty ensures clarity when referencing this feature within the class. No immediate concerns.
110-112: Conditional Omnibox Instantiation
By only instantiatingOmniboxwhenwebuiExtensionIdis available, you gracefully handle environments where the extension isn’t loaded. Overall logic seems solid.
426-427: Reasonable Min Dimensions
Enforcing a minimum window size is acceptable for most use cases. If additional flexibility is desired, consider making these dimensions user-configurable.
432-433: Window Aesthetic Configurations
These additional window properties (titleBarOverlay, transparency, vibrancy, etc.) enhance UI aesthetics on macOS and Windows. Be aware that vibrancy visuals differ per platform, and some properties may have no effect on other OSes.Also applies to: 440-447
487-490: Construction Options Sanity Check
The usage ofconstructionOptionsfor creating a new tab is straightforward. Verify that yourdetailsobject always provides the necessary parameters to prevent runtime issues.
526-527: Allowlist for Flow Utility Paths
IntroducingFLOW_UTILITY_ALLOWED_DIRECTORIESis a logical security measure. Should you need additional directories in the future, this pattern is easily extensible.
595-595: No Meaningful Change
This blank line addition does not affect functionality.
596-609: IPC Handlers for Custom Window Button Management
These IPC handlers correctly retrieve theBrowserWindowand conditionally call customsetWindowButtonPositionandsetWindowButtonVisibilitymethods. Ensure these window methods are well-defined in a subclass or extended type to avoid runtime errors.
33-33:Details
❓ Verification inconclusive
Ensure Preload Path is Valid
You are now pointingPATHS.PRELOADto.webpack/renderer/browser/preload.js. Verify that this file consistently exists after packaging to avoid load failures.
🏁 Script executed:
#!/bin/bash # Validate that the new preload path is present in the build output fd preload.js .webpack --exec ls -lhLength of output: 134
Preload File Path Verification Needed
The current preload path is set as follows:PRELOAD: path.join(WEBPACK_ROOT_DIR, "renderer", "browser", "preload.js"),The original intent was for this to point to
.webpack/renderer/browser/preload.js. However, the automated check failed because the.webpackdirectory was not found in the repository build output. Please verify manually that your packaging process correctly generates the expected file at this path. Additionally, ensure that theWEBPACK_ROOT_DIRvariable is set to a directory that is indeed created during packaging.
- Confirm that the output directory (e.g.
.webpack) is correctly generated.- Verify that
.webpack/renderer/browser/preload.jsexists post-packaging.- Update the build configuration if the output directory or path has changed.
electron/browser/omnibox.ts (13)
1-4: Map-Based Omnibox Storage
Using aMap<BrowserWindow, Omnibox>to track per-window omnibox instances is straightforward. However, ensure entries are removed upon window destruction to prevent memory leaks.
5-5: Simple QueryParams Alias
Defining a basic{ [key: string]: string }alias is sufficient for your needs.
7-49: Omnibox Constructor & Setup
- Creating a
WebContentsViewand attaching it toparentWindowestablishes a cohesive UI.- The
on("blur", ...)event to hide the omnibox on focus loss is a user-friendly approach.setTimeout(...)to load the interface and update bounds, then hiding the omnibox ensures a clean initial state.
Overall, this design is sensible for a dedicated floating omnibox window.
51-68:loadInterfaceMethod
Dynamically constructing the URL with query parameters is well-handled. The conditional reload if the URL matches is also a nice touch. No immediate issues.
70-75: Clearing Keep-On-Top Interval
Properly clearing your interval avoids resource leaks. Good housekeeping.
77-95:updateBoundsHandling
Gracefully handles custom bounds or defaults to center the omnibox. Consider verifying that the resulting positions remain on-screen for smaller displays.
97-100:isVisibleMethod
Delegating visibility checks tothis.view.getVisible()is simple and direct.
122-125:hideMethod
Stopping the keep-on-top interval before hiding prevents concurrency issues or repeated calls. Appropriately implemented.
127-131:setBounds
Storing bounds inthis.boundsand reapplying them is straightforward. No concerns.
133-135:getNewTabModeAlways Returns "omnibox"
Returning a constant "omnibox" is consistent with your logic. If you plan to switch modes dynamically, consider a more robust approach.
137-149: Omnibox Bound & Interface Utility Functions
setOmniboxBoundsandloadOmniboxunify repeated code for a cleaner approach. No issues found.
151-163: Visibility & State Helpers
showOmnibox,hideOmnibox, andisOmniboxOpennicely delegate to yourOmniboxinstance for code reuse.
170-182: IPC Handlers for Omnibox
Providing direct IPC handlers—show-omniboxandhide-omnibox—to manage bounds and parameters is coherent. Ensure comprehensive error-checking ifparentWindowis null or destroyed.vite/src/components/ui/dropdown-menu.tsx (18)
1-4: Importing Radix and Icons
These imports are appropriate for creating a customized dropdown menu with icons. No issues noted.
5-5: Utility Function Import
cnfrom"@/lib/utils"is a standard approach for conditional classnames.
7-11:DropdownMenuRoot Component
WrappingDropdownMenuPrimitive.Rootin a local component is a neat way to add custom data attributes.
13-19:DropdownMenuPortal
Forwarding all props toDropdownMenuPrimitive.Portalis straightforward. The extradata-slothelps with debugging and styling.
21-30:DropdownMenuTrigger
A standard Radix wrapper. Implementation aligns with typical usage.
32-50:DropdownMenuContent
- The additional class naming for open/close animations is well-structured.
- The
sideOffsetdefault is set to 4, which is sensible.- Wrapping in
DropdownMenuPrimitive.Portalensures correct layering.
52-58:DropdownMenuGroup
Simple pass-through toDropdownMenuPrimitive.Group; no problems.
60-81:DropdownMenuItem
- The
data-insetanddata-variantattributes provide fine-grained styling.- The destructured
variantprop is a handy extension.- Good usage of Radix states for focus/disabled.
83-107:DropdownMenuCheckboxItem
- Integrates a checkbox indicator with
CheckIcon.- The absolute positioning for the checkbox is typical of menu items.
- Overall usage of
CheckboxItemis correct.
109-118:DropdownMenuRadioGroup
Acts as a container for radio items. Straightforward.
120-142:DropdownMenuRadioItem
- Provides a
CircleIconfor the radio selection.- The code mirrors the
CheckboxItemapproach.- Good usage of
ItemIndicator.
144-162:DropdownMenuLabel
Simple extension wrappingDropdownMenuPrimitive.Label. The custominsetlogic is on point.
164-175:DropdownMenuSeparator
Appropriate styling for dividing items.
177-191:DropdownMenuShortcut
A quick utility to align keyboard shortcuts to the right. Implementation is minimal and effective.
193-197:DropdownMenuSub
Standard sub-menu usage from Radix.
199-221:DropdownMenuSubTrigger
- Combines a sub-trigger with optional
inset.- Visually indicates a submenu with
ChevronRightIcon.
223-237:DropdownMenuSubContent
Mirrors the rootContentstyling, applying typical sub-menu animations and layout.
239-255: Named Exports Provided
All menu components are exported, supporting modular usage of the dropdown menu API.vite/src/lib/flow.ts (2)
73-83: Validate null checks on omnibox methods.
If for some reason theomniboxobject is not injected or not yet initialized, calls toflow.omnibox.showorflow.omnibox.hidefrom other modules might fail. You could add runtime checks or fallbacks for robust error handling when the omnibox is unavailable.
85-128: Ensure availability of globalflow.
All exported helper functions rely onflowbeing globally present. Depending on your build or preload script timing,flowmight not be attached yet. If that’s guaranteed by design, this is fine. Otherwise, consider adding optional chaining or defensive checks:-export function showOmnibox(bounds: PageBounds | null, params: QueryParams | null) { - return flow.omnibox.show(bounds, params); +export function showOmnibox(bounds: PageBounds | null, params: QueryParams | null) { + if (!flow?.omnibox?.show) { + console.warn("Omnibox unavailable"); + return; + } + return flow.omnibox.show(bounds, params); }vite/src/lib/omnibox.ts (1)
99-118: Review fallback URL.
Wheninput.type === OmniboxInputType.URLbutgetURLFromInputreturnsnull, you fall back to"about:blank". Confirm that this silent fallback is intended, as it may navigate to a blank page instead of prompting the user or surfacing an error.vite/src/components/ui/resizable-sidebar.tsx (3)
103-113: Great use of a keyboard shortcut.
The logic for toggling the sidebar with "Ctrl + b" or "Cmd + b" is straightforward and well-cleaned up with the removal of the event listener in the effect cleanup.
540-565: Tooltips logic is well-structured.
Wrapping the menu button in a tooltip only when a tooltip is provided is a clean approach. The condition to hide the tooltip while not collapsed or on mobile devices is appropriate for usability.
712-736: Robust export structure.
Exporting all components, subcomponents, and hooks from a single file significantly improves discoverability. This modular design will benefit maintainability and code reuse.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
vite/src/components/main/browser-context.tsx (5)
87-90: Add optional chaining for safer property accessThe active tab check could be simplified using optional chaining to make the code more defensive.
- if (activeTab && activeTab.id) { + if (activeTab?.id) { setActiveTabId(activeTab.id); }🧰 Tools
🪛 Biome (1.9.4)
[error] 88-88: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
166-166: Remove console.log statements before productionThere are several console.log statements throughout the file (also at lines 185, 213) that should be removed or replaced with proper logging before production deployment.
- console.log("Tab Created!", tab.id, tab);
256-264: Consider adding error handling for Chrome API callsThe handler doesn't check if the Chrome API is available before using it. In extensions, the API might not always be accessible when running in certain contexts.
const handleAddressUrlSubmit = useCallback(() => { const parsedUrl = parseAddressBarInput(addressUrl); + if (!chrome.tabs) { + console.error("Chrome tabs API not available"); + return; + } if (parsedUrl && activeTabId && activeTabId > 0) { chrome.tabs.update(activeTabId, { url: parsedUrl }); } else if (parsedUrl) { // If no active tab or invalid tab ID, create a new tab with the URL chrome.tabs.create({ url: parsedUrl }); } }, [addressUrl, activeTabId]);
54-58: Simplify conditional logic for clarityThe nested condition can be flattened to improve readability.
if (!transformedUrl) { setAddressUrl(activeTab.url); } else { - if (transformedUrl && transformedUrl !== "flow://new") { + if (transformedUrl !== "flow://new") { setAddressUrl(transformedUrl); } else { setAddressUrl(""); } }
233-244: Optimize tab click handler for clarity and performanceThe current implementation iterates through all tabs to find a matching tab ID. This can be simplified since you already know the tab ID.
const handleTabClick = useCallback( (tabId: number) => { - // Only update if the tab exists - tabs.forEach((tab) => { - if (tab.id === tabId) { - chrome.tabs.update(tabId, { active: true }); - } - }); + // No need to iterate through all tabs when we already have the ID + const tabExists = tabs.some(tab => tab.id === tabId); + if (tabExists) { + chrome.tabs.update(tabId, { active: true }); + } }, [tabs] );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
vite/src/components/main/browser-context.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
vite/src/components/main/browser-context.tsx (1)
vite/src/lib/url.ts (2) (2)
transformUrl(85-119)parseAddressBarInput(68-83)
🪛 Biome (1.9.4)
vite/src/components/main/browser-context.tsx
[error] 88-88: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (1)
vite/src/components/main/browser-context.tsx (1)
1-328: Excellent implementation of browser context provider with solid state managementOverall, this is a well-structured React Context implementation that properly manages browser state. The code demonstrates good practices:
- Proper use of React hooks (useState, useEffect, useRef, useCallback)
- Effective management of Chrome extension API interactions
- Good separation of concerns between state management and event handlers
- Appropriate error handling and edge case management in most areas
- Clean, consistent coding style with good comments
The logic for updating the address bar URL, managing tabs, and handling window state is well thought out. The export of a custom hook with proper error checking ensures consumers use the context correctly.
🧰 Tools
🪛 Biome (1.9.4)
[error] 88-88: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
1 participant
Summary by CodeRabbit
New Features
Refactor & Style