Conversation
WalkthroughThis update introduces a comprehensive keyboard shortcuts management system, including backend storage, IPC integration, and a full React-based UI for editing, resetting, and displaying shortcuts. It also adds a logging module, improves Picture-in-Picture tab control, enhances menu shortcut handling, and refactors several settings UI components for better structure, feedback, and accessibility. Changes
Sequence Diagram(s)Keyboard Shortcut Update FlowsequenceDiagram
participant Renderer as Renderer (React UI)
participant Preload as Preload Script
participant Main as Main Process
participant Storage as Shortcuts Storage
Renderer->>Preload: flow.shortcuts.setShortcut(actionId, shortcut)
Preload->>Main: IPC "shortcuts:set" (actionId, shortcut)
Main->>Storage: updateModifiedShortcut(actionId, shortcut)
Storage-->>Main: Success/Failure
Main->>Preload: Returns success status
Preload->>Renderer: Returns success status
Main-->>Renderer: Broadcast "shortcuts-changed" via IPC
Renderer->>Renderer: UI updates with new shortcuts
Copy Link Action FlowsequenceDiagram
participant Renderer as Renderer (UI)
participant Preload as Preload Script
participant Main as Main Process
Renderer->>Preload: (User clicks Copy Link)
Preload->>Main: IPC "actions:on-copy-link"
Main->>Renderer: IPC event "actions:on-copy-link"
Renderer->>Renderer: Copies link to clipboard, shows toast
Possibly related PRs
Suggested labels
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 18
🔭 Outside diff range comments (4)
src/main/browser/tabs/tab-manager.ts (1)
231-251: 🛠️ Refactor suggestionImprove documentation for the new parameter and use optional chaining
The added
goBackToTabparameter provides helpful flexibility for controlling tab focus behavior after disabling Picture-in-Picture mode. Two improvements would enhance this code:
- Add JSDoc comments to explain the purpose of the new parameter
- Use optional chaining as suggested by static analysis
/** * Disable Picture in Picture mode for a tab + * @param tabId The ID of the tab to disable Picture in Picture mode for + * @param goBackToTab Whether to focus the window and set the tab as active after disabling PiP + * @returns true if PiP was disabled, false if the tab wasn't in PiP mode */ public disablePictureInPicture(tabId: number, goBackToTab: boolean) { const tab = this.getTabById(tabId); - if (tab && tab.isPictureInPicture) { + if (tab?.isPictureInPicture) { tab.updateStateProperty("isPictureInPicture", false); if (goBackToTab) { // Set the space for the window const win = tab.getWindow(); setWindowSpace(win, tab.spaceId); // Focus window win.window.focus(); // Set active tab this.setActiveTab(tab); } return true; } return false; }🧰 Tools
🪛 Biome (1.9.4)
[error] 233-233: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/main/modules/logs.ts (1)
1-50: 🛠️ Refactor suggestionConsider adding error handling for log file operations
The logging implementation currently doesn't handle potential errors when writing to log files, which could lead to unhandled exceptions.
Add error handling to prevent crashes if log writing fails:
+// Set up error handling for the log stream +logStream.on('error', (error) => { + console.error(`Error writing to log file: ${error.message}`); + // Restore original stdout to prevent infinite loops if logging fails + if (process.stdout.write !== originalStdoutWrite) { + process.stdout.write = originalStdoutWrite; + } +}); function newStdoutWrite( chunk: Uint8Array | string, encodingOrCallback?: BufferEncoding | Callback, callback?: Callback ) { if (typeof chunk === "string") { // Remove ANSI escape codes from console output // This regex targets escape sequences that control terminal colors and formatting // eslint-disable-next-line no-control-regex chunk = chunk.replace(/\x1b\[[0-9;]*[a-zA-Z]/g, ""); } + try { logStream.write(chunk); + } catch (error) { + console.error(`Failed to write to log file: ${error.message}`); + } // @ts-expect-error: This is a workaround to log to the log file return originalStdoutWrite.call(process.stdout, chunk, encodingOrCallback, callback); }🧰 Tools
🪛 Biome (1.9.4)
[error] 41-41: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
src/main/browser/tabs/tab.ts (1)
759-779: 🛠️ Refactor suggestion
enterPiPrelies on globalflow– verify preload exposureInside the injected
enterPiPfunction,flow.tabs.disablePictureInPictureis assumed to be globally available in the page context.
If extensions or CSPs inject an isolated world, or the preload fails,flowmay be undefined, causing an uncaught exception that aborts the promise and hides the PiP failure.Add a presence check and fall back gracefully:
- flow.tabs.disablePictureInPicture(goBackToTab); + if (window.flow?.tabs?.disablePictureInPicture) { + window.flow.tabs.disablePictureInPicture(goBackToTab); + } else { + console.warn("flow API unavailable – cannot disable PiP"); + }src/preload/index.ts (1)
47-72:⚠️ Potential issue
"all"permission bypasses every check – confirm this is intentional
hasPermission("all")always returnstrue, making any API wrapped with that permission callable from any renderer (including un-trusted ones if they ever get this preload).
In particular,tabs.disablePictureInPictureis now exposed under"all"(see lines 510-512).Double-check the threat model and consider limiting
"all"toisInternalProtocolsinstead of unconditional access.
🧹 Nitpick comments (26)
src/main/ipc/app/actions.ts (2)
4-6: Consider removing unnecessary async keywordThis function doesn't contain any awaited operations. Unless there are plans to add asynchronous operations in the future, consider removing the
asynckeyword to avoid setting false expectations.-export async function fireCopyLinkAction(win: TabbedBrowserWindow) { +export function fireCopyLinkAction(win: TabbedBrowserWindow) { sendMessageToListenersInWindow(win, "actions:on-copy-link"); }
8-10: Consider removing unnecessary async keywordThis function doesn't contain any awaited operations. Unless there are plans to add asynchronous operations in the future, consider removing the
asynckeyword to avoid setting false expectations.-export async function fireFrontendAction(action: string) { +export function fireFrontendAction(action: string) { sendMessageToListeners("actions:on-incoming", action); }src/renderer/src/components/settings/sections/shortcuts/components/search-header.tsx (1)
12-33: Enhance accessibility.The component has good UI/UX with responsive design and visual feedback, but could benefit from improved accessibility.
Add aria-label attributes to the buttons for screen readers:
<Input type="search" placeholder="Search shortcuts..." className="pl-9 w-full" value={searchTerm} onChange={(e) => onSearchChange(e.target.value)} + aria-label="Search shortcuts" /> ... <Button variant="outline" onClick={onResetClick} disabled={isLoading} className="group" + aria-label="Reset all shortcuts to default values" >src/renderer/src/components/browser-ui/sidebar/header/address-bar/copy-link-button.tsx (1)
65-68: Extract magic number to constant.The timeout duration for resetting the copied state is hardcoded to 3000ms. For better maintainability, consider extracting this to a named constant.
+const COPY_FEEDBACK_DURATION_MS = 3000; export function AddressBarCopyLinkButton() { // ...existing code... setTimeout(() => { setCopied(false); - }, 3000); + }, COPY_FEEDBACK_DURATION_MS);src/main/modules/logs.ts (2)
38-42: Control character in regex needs to be justifiedThe regex to remove ANSI escape codes uses a control character sequence. While this is intentional and necessary for cleaning logs, it's triggered a linting error.
Add a more specific comment explaining why the control character regex is necessary:
- // remove ANSI escape codes + // Remove ANSI escape codes from console output + // This regex targets escape sequences that control terminal colors and formatting🧰 Tools
🪛 Biome (1.9.4)
[error] 41-41: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
46-48: Consider improving the type safety of stdout write overrideThe current implementation uses
@ts-expect-errorto bypass TypeScript checking when calling the original stdout write method.Consider using a more type-safe approach by explicitly casting the function types:
- // @ts-expect-error: This is a workaround to log to the log file - return originalStdoutWrite.call(process.stdout, chunk, encodingOrCallback, callback); + // Cast the function to ensure type compatibility + return (originalStdoutWrite as Function).call( + process.stdout, + chunk, + encodingOrCallback, + callback + );src/main/browser/tabs/tab.ts (1)
791-799: SurfaceenterPiPexecution errors to the caller
executeJavaScriptfailures are swallowed and only logged to the console.
Emitting an event (or at least updating a state flag) would allow UI components to react and show feedback to the user.Consider emitting a dedicated
"pip-error"event or returning the rejection so the caller can decide.src/renderer/src/components/settings/sections/shortcuts/components/modified-shortcuts-section.tsx (2)
1-6: Verify Motion One React package name
motion/reactbelongs to Motion One’s React bindings (@motionone/react).
If the project uses Framer Motion elsewhere (framer-motion), this import will resolve differently and bundle two animation libraries.Double-check
package.jsonto ensure only one motion library is installed and update the import accordingly:-import { AnimatePresence, motion } from "motion/react"; +import { AnimatePresence, motion } from "@motionone/react"; // or "framer-motion"
50-69: Keyboard handler & ref passed to every item
onKeyDownandinputRefare only meaningful for the item currently being edited, yet they are forwarded to everyShortcutItem.
Passing the same ref to multiple elements breaks the “one-ref-per-node” contract and may lead to unexpected focus behaviour.Refactor so those props are only provided to the editing item:
-<ShortcutItem +const isEditing = editingActionId === shortcut.id; +<ShortcutItem ... - inputRef={inputRef} - onKeyDown={onKeyDown} + inputRef={isEditing ? inputRef : undefined} + onKeyDown={isEditing ? onKeyDown : undefined} ... />src/main/browser/utility/protocols.ts (2)
132-176:parseIntwithout radix & missing fetch-error handling
- Always pass a radix to
parseIntto avoid subtle bugs in browsers that still treat"08"/"09"as octal.- If
profile.session.fetchfails (network error, 4xx/5xx), the resulting response is still cached and returned; subsequent callers will always see the error response.
Checkresponse.okbefore caching.-const tabIdInt = parseInt(tabId); +const tabIdInt = parseInt(tabId, 10); -const faviconResponse = await profile.session.fetch(faviconURL); -if (!faviconResponse.ok) return faviconResponse; // don’t cache failures +const faviconResponse = await profile.session.fetch(faviconURL); +if (!faviconResponse.ok) return faviconResponse;
195-199: Route precedence commentNice integration of the new
active-faviconroute.
Consider documenting it in a central protocol routing table to avoid future collisions with additional hosts.src/renderer/src/components/settings/settings-sidebar.tsx (2)
6-10: Icon prop typing is off – useReactElementnot component generic
React.ReactElement<LucideIcon>is not the intended generic – the first generic parameter is the element’s props shape, not the component type.
Simplest fix:- icon: React.ReactElement<LucideIcon>; + icon: React.ReactElement;or expose the icon component instead (
icon: LucideIcon) and render it as<section.icon className="…" />.
22-33: Accessibility: convey current section to assistive techAdd
aria-current="page"(or"true") to the active button so screen-reader users know which section is selected.<motion.button … + aria-current={activeSection === section.id ? "page" : undefined} >src/renderer/src/components/providers/shortcuts-provider.tsx (2)
32-41:.replacewithout/gonly replaces first occurrenceIf a shortcut somehow contains multiple
+signs or other tokens, only the first instance is replaced. Minor now, but safer to use global regexes:-return shortcut - .replace(/\+/g, " + ") - .replace("CommandOrControl", "⌘/Ctrl") +return shortcut + .replace(/\+/g, " + ") + .replace(/CommandOrControl/g, "⌘/Ctrl")
96-107: Reset-all performs N IPC round-trips – expose bulk API instead
resetAllShortcutsissues one IPC call per action, increasing latency and load.
Consider adding a dedicatedflow.shortcuts.resetAll()in the main process that resets everything in one call, then refresh once.This will also avoid partial failure states if one of the awaits rejects midway.
src/renderer/src/components/settings/sections/general/update-card.tsx (2)
240-246: Progress bar shows 0 % until first tick – minor UX nitConsider rendering the bar only after the first progress event to avoid a momentary 0 % bar that may feel like a glitch.
E.g.isDownloadingUpdate && updateProgress > 0.
250-271:cnhelper used but not needed – small bundle win
className={cn("rounded-md border p-3 …", "bg-orange-500/10 …")}merges only two static strings.
Direct string concatenation is enough here and keeps the generated class list predictable.-className={cn( - "rounded-md border p-3 mt-4 text-sm", - "bg-orange-500/10 border-orange-500/30 text-orange-700 dark:text-orange-400" -)} +className="rounded-md border p-3 mt-4 text-sm bg-orange-500/10 border-orange-500/30 text-orange-700 dark:text-orange-400"src/renderer/src/components/providers/actions-provider.tsx (2)
41-49: Improve error handling for copyLink function.The function currently returns silently when
addressUrlis undefined. Consider providing feedback to the user in this case.const copyLink = useCallback(async (): Promise<void> => { if (!addressUrl) { + showToast("No URL available to copy.", { + type: "error" + }); return; } copyTextToClipboard(addressUrl, false).then((success) => { if (success) { showToast("Copied to clipboard!"); } }); }, [addressUrl, showToast]);
64-71: Properly managed event subscriptions.The event subscriptions are properly set up and cleaned up on component unmount, preventing memory leaks. However, the dependency arrays could be simplified since the refs themselves don't change.
// Listener for the onCopyLink event from the main process useEffect(() => { const unsubscribe = flow.actions.onCopyLink(() => { return copyLinkCallbackRef.current(); }); return () => { unsubscribe(); }; - }, [copyLinkCallbackRef]); + }, []); // Listener for the onIncomingAction event from the main process useEffect(() => { const unsubscribe = flow.actions.onIncomingAction((action) => { return handleIncomingActionCallbackRef.current(action); }); return () => { unsubscribe(); }; - }, [handleIncomingActionCallbackRef]); + }, []);Also applies to: 74-81
src/renderer/src/components/settings/sections/shortcuts/section.tsx (1)
140-175: Minor: “Modified” sort rule is redundantYou treat
"Modified"as a special category when sorting, yet that string never occurs ingroupedKeybindsbecause modified shortcuts are handled in a dedicated array.
The extra comparison adds an unnecessary branch and can be safely removed to simplify the sorter.src/renderer/src/components/settings/sections/general/basic-settings-cards.tsx (1)
55-93:TooltipProviderwrapper is unused
TooltipProvideris rendered but no descendant Tooltip is present inside the card.
Unless tooltips are added later, this wrapper adds one extra React node and context provider per card without benefit.If not required, consider removing it to keep the DOM tree shallow.
src/preload/index.ts (1)
482-495: Specify expected return types onipcRenderer.invoketo regain type-safety
ipcRenderer.invokereturnsPromise<unknown>unless a generic is supplied. Right now the compiler can’t tell thatsetShortcutresolves toboolean, etc.-return ipcRenderer.invoke("shortcuts:set", actionId, shortcut); +return ipcRenderer.invoke<boolean>("shortcuts:set", actionId, shortcut);Apply the same pattern to every
invokecall inshortcutsAPI.src/renderer/src/components/settings/sections/external-apps/section.tsx (1)
37-42: Replace deprecatedonKeyPresswithonKeyDownfor accessibility
onKeyPressis deprecated in React 18 and doesn’t fire for all keyboards. UseonKeyDownand filter forEnter/Space.- onKeyPress={(e) => (e.key === "Enter" || e.key === " ") && setExpanded(!expanded)} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + setExpanded(!expanded); + } + }}src/main/saving/shortcuts.ts (1)
29-40: Consider cleaning up invalid data from the datastoreCurrently, invalid shortcut data is logged but remains in the datastore, which could lead to repeated errors on application restart.
Consider removing invalid data:
for (const key of Object.keys(rawModifiedShortcuts)) { const rawModifiedShortcutData = rawModifiedShortcuts[key]; const parseResult = ModifiedShortcutData.safeParse(rawModifiedShortcutData); if (!parseResult.success) { console.error(`Invalid shortcut data for ${key}:`, parseResult.error); + // Remove invalid data to prevent repeated errors + ShortcutsDataStore.remove(key).catch(err => { + console.error(`Failed to remove invalid shortcut data for ${key}:`, err); + }); continue; } const modifiedShortcutData = parseResult.data; modifiedShortcuts.set(key, modifiedShortcutData); hasChanged = true; }src/renderer/src/components/ui/alert-dialog.tsx (2)
7-13: Consider using explicit type declarations for componentsThe AlertDialog and AlertDialogTrigger components use ComponentProps for type definitions, which works but could be more explicit about the props they accept.
For clearer type definitions:
-function AlertDialog({ ...props }: React.ComponentProps<typeof AlertDialogPrimitive.Root>) { +type AlertDialogProps = React.ComponentProps<typeof AlertDialogPrimitive.Root>; + +function AlertDialog({ ...props }: AlertDialogProps) { return <AlertDialogPrimitive.Root data-slot="alert-dialog" {...props} />; } -function AlertDialogTrigger({ ...props }: React.ComponentProps<typeof AlertDialogPrimitive.Trigger>) { +type AlertDialogTriggerProps = React.ComponentProps<typeof AlertDialogPrimitive.Trigger>; + +function AlertDialogTrigger({ ...props }: AlertDialogTriggerProps) { return <AlertDialogPrimitive.Trigger data-slot="alert-dialog-trigger" {...props} />; }
1-6: Consider adding accessibility documentationWhile the implementation correctly uses Radix UI primitives which have good accessibility support, it might be helpful to document the accessibility features for developers using these components.
Add a comment at the top of the file explaining the accessibility features:
import * as React from "react"; import * as AlertDialogPrimitive from "@radix-ui/react-alert-dialog"; +/** + * Alert Dialog Component + * + * Built on Radix UI's AlertDialog primitives with the following accessibility features: + * - Manages focus correctly + * - Supports keyboard navigation + * - Properly implements ARIA attributes + * - Provides screen reader announcements + * + * Usage guidelines: + * - Use for important confirmations that require user attention + * - Provide clear, concise messages in the title and description + * - Always include a cancel option + */ + import { cn } from "@/lib/utils"; import { buttonVariants } from "@/components/ui/button";
📜 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 (59)
.github/ISSUE_TEMPLATE/bug_report.md(2 hunks).github/ISSUE_TEMPLATE/feature_request.md(1 hunks)components.json(1 hunks)docs/references/logs.md(1 hunks)package.json(1 hunks)src/main/browser/tabs/tab-manager.ts(2 hunks)src/main/browser/tabs/tab.ts(4 hunks)src/main/browser/utility/menu.ts(3 hunks)src/main/browser/utility/menu/items/archive.ts(2 hunks)src/main/browser/utility/menu/items/edit.ts(2 hunks)src/main/browser/utility/menu/items/file.ts(2 hunks)src/main/browser/utility/menu/items/view.ts(5 hunks)src/main/browser/utility/protocols.ts(2 hunks)src/main/browser/window.ts(4 hunks)src/main/ipc/app/actions.ts(1 hunks)src/main/ipc/app/shortcuts.ts(1 hunks)src/main/ipc/browser/interface.ts(2 hunks)src/main/ipc/browser/tabs.ts(1 hunks)src/main/ipc/main.ts(1 hunks)src/main/modules/logs.ts(1 hunks)src/main/modules/output.ts(1 hunks)src/main/modules/shortcuts.ts(1 hunks)src/main/saving/shortcuts.ts(1 hunks)src/preload/index.ts(7 hunks)src/renderer/src/components/browser-ui/main.tsx(2 hunks)src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx(3 hunks)src/renderer/src/components/browser-ui/sidebar/header/address-bar/address-bar.tsx(1 hunks)src/renderer/src/components/browser-ui/sidebar/header/address-bar/copy-link-button.tsx(2 hunks)src/renderer/src/components/omnibox/main.tsx(1 hunks)src/renderer/src/components/providers/actions-provider.tsx(1 hunks)src/renderer/src/components/providers/minimal-toast-provider.tsx(1 hunks)src/renderer/src/components/providers/shortcuts-provider.tsx(1 hunks)src/renderer/src/components/settings/sections/about/browser-info-card.tsx(1 hunks)src/renderer/src/components/settings/sections/about/section.tsx(0 hunks)src/renderer/src/components/settings/sections/external-apps/section.tsx(6 hunks)src/renderer/src/components/settings/sections/general/basic-settings-cards.tsx(3 hunks)src/renderer/src/components/settings/sections/general/reset-onboarding-card.tsx(2 hunks)src/renderer/src/components/settings/sections/general/set-as-default-browser-setting.tsx(2 hunks)src/renderer/src/components/settings/sections/general/update-card.tsx(5 hunks)src/renderer/src/components/settings/sections/shortcuts/components/category-section.tsx(1 hunks)src/renderer/src/components/settings/sections/shortcuts/components/empty-state.tsx(1 hunks)src/renderer/src/components/settings/sections/shortcuts/components/modified-shortcuts-section.tsx(1 hunks)src/renderer/src/components/settings/sections/shortcuts/components/reset-dialog.tsx(1 hunks)src/renderer/src/components/settings/sections/shortcuts/components/search-header.tsx(1 hunks)src/renderer/src/components/settings/sections/shortcuts/hooks/use-keyboard-normalizer.tsx(1 hunks)src/renderer/src/components/settings/sections/shortcuts/section.tsx(1 hunks)src/renderer/src/components/settings/sections/shortcuts/shortcut-item.tsx(1 hunks)src/renderer/src/components/settings/settings-layout.tsx(3 hunks)src/renderer/src/components/settings/settings-sidebar.tsx(1 hunks)src/renderer/src/components/settings/settings-titlebar.tsx(1 hunks)src/renderer/src/components/settings/settings-topbar.tsx(0 hunks)src/renderer/src/components/ui/alert-dialog.tsx(1 hunks)src/renderer/src/hooks/use-debounce.tsx(1 hunks)src/renderer/src/lib/utils.ts(1 hunks)src/shared/flow/flow.ts(2 hunks)src/shared/flow/interfaces/app/actions.ts(1 hunks)src/shared/flow/interfaces/app/shortcuts.ts(1 hunks)src/shared/flow/interfaces/browser/tabs.ts(1 hunks)src/shared/types/shortcuts.ts(1 hunks)
💤 Files with no reviewable changes (2)
- src/renderer/src/components/settings/sections/about/section.tsx
- src/renderer/src/components/settings/settings-topbar.tsx
🧰 Additional context used
🧬 Code Graph Analysis (24)
src/renderer/src/components/browser-ui/sidebar/header/address-bar/address-bar.tsx (1)
src/renderer/src/components/browser-ui/sidebar/header/address-bar/copy-link-button.tsx (1)
AddressBarCopyLinkButton(54-79)
src/main/browser/utility/menu.ts (2)
src/main/browser/utility/menu/items/edit.ts (1)
createEditMenu(6-30)src/main/saving/shortcuts.ts (1)
shortcutsEmitter(18-18)
src/main/ipc/app/shortcuts.ts (2)
src/main/modules/shortcuts.ts (1)
getShortcuts(78-91)src/main/saving/shortcuts.ts (3)
updateModifiedShortcut(50-62)resetModifiedShortcut(65-74)shortcutsEmitter(18-18)
src/main/ipc/app/actions.ts (1)
src/main/browser/window.ts (1)
TabbedBrowserWindow(29-274)
src/main/browser/utility/menu/items/view.ts (1)
src/main/modules/shortcuts.ts (1)
getCurrentShortcut(101-105)
src/shared/flow/flow.ts (2)
src/shared/flow/interfaces/app/actions.ts (1)
FlowActionsAPI(4-14)src/shared/flow/interfaces/app/shortcuts.ts (1)
FlowShortcutsAPI(7-27)
src/main/ipc/browser/interface.ts (1)
src/main/index.ts (1)
browser(14-14)
src/renderer/src/components/browser-ui/main.tsx (3)
src/renderer/src/components/providers/actions-provider.tsx (1)
ActionsProvider(36-93)src/renderer/src/lib/utils.ts (1)
cn(5-7)src/renderer/src/components/browser-ui/sidebar/hover-detector.tsx (1)
SidebarHoverDetector(5-35)
src/renderer/src/components/browser-ui/sidebar/header/address-bar/copy-link-button.tsx (1)
src/renderer/src/components/providers/actions-provider.tsx (1)
useActions(24-30)
src/main/modules/logs.ts (1)
src/main/modules/paths.ts (1)
FLOW_DATA_DIR(16-16)
src/main/ipc/browser/tabs.ts (1)
src/main/index.ts (1)
browser(14-14)
src/renderer/src/components/settings/sections/shortcuts/components/category-section.tsx (2)
src/shared/types/shortcuts.ts (1)
ShortcutAction(1-7)src/renderer/src/components/settings/sections/shortcuts/shortcut-item.tsx (1)
ShortcutItem(24-141)
src/renderer/src/components/providers/shortcuts-provider.tsx (1)
src/shared/types/shortcuts.ts (1)
ShortcutAction(1-7)
src/main/browser/utility/protocols.ts (1)
src/main/index.ts (1)
browser(14-14)
src/renderer/src/components/settings/sections/general/basic-settings-cards.tsx (2)
src/renderer/src/lib/utils.ts (1)
cn(5-7)src/renderer/src/components/settings/sections/general/set-as-default-browser-setting.tsx (1)
SetAsDefaultBrowserSetting(6-59)
src/renderer/src/components/settings/sections/general/update-card.tsx (2)
src/main/modules/auto-update.ts (3)
isAutoUpdateSupported(52-56)checkForUpdates(58-86)downloadUpdate(122-184)src/renderer/src/lib/utils.ts (1)
cn(5-7)
src/renderer/src/components/settings/sections/general/set-as-default-browser-setting.tsx (1)
src/main/modules/default-browser.ts (1)
setDefaultBrowser(16-42)
src/shared/flow/interfaces/app/shortcuts.ts (1)
src/shared/types/shortcuts.ts (1)
ShortcutAction(1-7)
src/renderer/src/components/settings/sections/shortcuts/shortcut-item.tsx (2)
src/shared/types/shortcuts.ts (1)
ShortcutAction(1-7)src/renderer/src/lib/utils.ts (1)
cn(5-7)
src/renderer/src/components/settings/sections/external-apps/section.tsx (1)
src/renderer/src/lib/utils.ts (1)
cn(5-7)
src/renderer/src/components/providers/actions-provider.tsx (2)
src/renderer/src/components/providers/minimal-toast-provider.tsx (1)
useToast(14-20)src/renderer/src/lib/utils.ts (1)
copyTextToClipboard(15-34)
src/renderer/src/components/ui/alert-dialog.tsx (1)
src/renderer/src/lib/utils.ts (1)
cn(5-7)
src/main/modules/shortcuts.ts (2)
src/shared/types/shortcuts.ts (1)
ShortcutAction(1-7)src/main/saving/shortcuts.ts (1)
getAllModifiedShortcuts(94-99)
src/preload/index.ts (2)
src/shared/flow/interfaces/app/actions.ts (1)
FlowActionsAPI(4-14)src/shared/flow/interfaces/app/shortcuts.ts (2)
FlowShortcutsAPI(7-27)ShortcutsData(4-4)
🪛 Biome (1.9.4)
src/main/browser/tabs/tab-manager.ts
[error] 233-233: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/main/modules/logs.ts
[error] 41-41: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
🔇 Additional comments (94)
.github/ISSUE_TEMPLATE/feature_request.md (1)
6-6: Standardize YAML quoting forassigneesfield
Changed the empty single-quoted string to double quotes to align with the bug-report template and maintain consistency across issue templates..github/ISSUE_TEMPLATE/bug_report.md (3)
4-6: Standardize YAML quoting fortitle,labels, andassigneesfields
Converted the empty single-quoted values to double-quoted strings to ensure uniform syntax across all issue templates.
14-14: Insert blank line before reproduction steps
Added a blank line before the numbered list under “To Reproduce” to improve readability in the rendered Markdown.
27-29: Add spacing before device information list
Inserted a blank line and ensured proper indentation for the device info items to enhance visual separation and clarity.src/renderer/src/components/settings/sections/about/browser-info-card.tsx (5)
6-18: Well-structured reusable component for consistent displayThe new InfoRow component improves code reusability and ensures consistent styling for the browser information display. The component properly handles text overflow with
break-wordsand uses semantic class names to indicate the purpose of each element.
22-22: Good addition of loading state trackingAdding an explicit loading state improves the user experience by providing feedback during data fetching.
24-37: Improved error handling with proper state managementThe enhanced error handling now properly manages loading state, clears stale data on errors, and uses a finally block to ensure loading state is always updated regardless of success or failure.
40-45: Enhanced semantic structure with better accessibilityReplacing Card with a styled div that includes proper heading hierarchy (
h3) and descriptive text improves accessibility and semantic structure.
47-66: Comprehensive UI states with user feedbackThe component now properly handles all three states (loading, success, error) with appropriate visual feedback:
- Loading state shows an animated spinner
- Success state displays structured information
- Error state shows a clear error message with appropriate styling
This significantly improves the user experience by providing clear feedback at each stage.
src/main/modules/output.ts (1)
4-4: Good integration of logging moduleAdding the logs module import ensures that application logs are captured from the beginning of the application lifecycle. This aligns with the PR objective to "pipe logs to files" and follows the principle of separation of concerns by keeping logging functionality in a dedicated module.
package.json (2)
63-63: Dependency version updateUpdating the
@radix-ui/react-slotdependency to version 1.2.2 ensures the application uses the latest bug fixes and improvements.
67-67: New UI component dependency for dialog functionalityAdding the
@radix-ui/react-alert-dialogpackage supports the implementation of accessible dialog components, which are likely used in the new Shortcuts Editor feature mentioned in the PR objectives.src/main/ipc/main.ts (1)
5-5: Integration of shortcuts IPC moduleAdding the shortcuts IPC module import ensures that keyboard shortcut management functionality is properly integrated into the main process. This addition correctly supports the new Shortcuts Editor feature mentioned in the PR objectives by enabling communication between the renderer and main processes for shortcut operations.
src/renderer/src/components/browser-ui/sidebar/header/address-bar/address-bar.tsx (1)
63-63: Clean refactoring of AddressBarCopyLinkButton implementation.The removal of the
addressUrlprop aligns with the component's refactored implementation that now uses the centralizedcopyLinkaction from theuseActionscontext. This approach improves maintainability by decoupling the button from direct data dependencies.docs/references/logs.md (1)
1-7: Helpful documentation for log locations.This documentation clearly lists the log file locations for different operating systems, which will be valuable for users and developers troubleshooting issues. The paths follow standard OS conventions.
src/renderer/src/components/omnibox/main.tsx (1)
211-211: UI styling simplification.The background styling has been simplified to use consistent opacity values rather than dynamically changing based on the
openInprop. This reduces complexity while maintaining the visual aesthetic of the omnibox.src/shared/flow/interfaces/browser/tabs.ts (1)
46-48: Good enhancement to Picture-in-Picture control.Adding the
goBackToTabparameter provides more flexibility in handling Picture-in-Picture mode, allowing callers to specify whether they want to return to the tab after disabling PiP. This is a user-friendly improvement that enhances the tab control experience.src/main/browser/utility/menu/items/archive.ts (1)
4-4: Great improvement with dynamic shortcut retrieval!The change from hardcoded accelerator strings to dynamic retrieval via
getCurrentShortcut()successfully integrates the Archive menu with the new shortcuts management system. This enables runtime customization of navigation shortcuts while maintaining the same functionality.Also applies to: 11-11, 25-25
src/main/browser/utility/menu.ts (2)
14-14: Proper adaptation to the EditMenu changesThe removal of the
browserparameter fromcreateEditMenu()call correctly aligns with the signature change in that function.Also applies to: 23-23
36-36: Well-implemented dynamic menu updates for shortcut changesThe addition of the event listener for "shortcuts-changed" ensures the application menu is automatically rebuilt whenever shortcuts are modified. This maintains consistency between user-customized shortcuts and the displayed menu accelerators.
src/shared/flow/flow.ts (1)
21-22: Clean integration of new API interfacesThe addition of
actionsandshortcutsproperties to the globalflowAPI interface properly exposes the new functionality to the renderer process. This follows the established pattern and maintains consistency with the rest of the API structure.Also applies to: 35-36
src/renderer/src/components/settings/settings-titlebar.tsx (1)
1-9: Clean and well-structured title bar componentThis new
SettingsTitlebarcomponent is simple yet effective, with appropriate styling for a modern titlebar. The use of Tailwind classes and the "app-drag" class for window dragging functionality is well-implemented. The component integrates well with the new sidebar-based settings layout.src/renderer/src/lib/utils.ts (1)
60-76: Well-implemented utility function.The new
craftActiveFaviconURLutility is well-structured, properly documented, and serves a clear purpose in the Flow Browser's favicon handling system. The approach of using query parameters to identify the tab and trigger cache invalidation is elegant.src/renderer/src/hooks/use-debounce.tsx (1)
1-18: Clean and well-implemented debounce hook.This implementation of
useDebouncefollows React best practices by properly:
- Using generics to maintain type safety
- Handling cleanup with the effect's return function
- Managing dependencies correctly in the dependency array
This will be useful for optimizing performance in UI components that handle frequent state updates.
src/main/browser/utility/menu/items/file.ts (3)
5-5: Good addition for centralized shortcut management.This import enables the dynamic shortcut resolution system described in the PR objectives.
12-12: Improved flexibility with dynamic shortcut resolution.Replacing the hardcoded accelerator with
getCurrentShortcut("tabs.new")aligns with the Shortcuts Editor feature described in the PR objectives, allowing users to customize keyboard shortcuts.
25-25: Consistent application of dynamic shortcut resolution.This change maintains consistency with the approach used for the "New Tab" menu item, allowing user customization of the "New Window" shortcut.
src/main/ipc/browser/interface.ts (1)
2-2: Browser import needed for window management.This import provides access to the global browser instance, which is necessary for the updated window button visibility handling.
components.json (1)
1-21: Configuration looks well-structured for the UI framework integration.This configuration file correctly sets up shadcn/ui components with Tailwind CSS. The path aliases are properly defined to support modular imports in the project, which will help with the new UI components for the Shortcuts Editor feature.
src/main/ipc/browser/tabs.ts (1)
206-214: Good enhancement to the Picture-in-Picture functionality.The new
goBackToTabparameter allows users to control whether the tab becomes active after disabling PiP mode, aligning with the PR objective of "implementing detection for closing or returning to a tab when using picture-in-picture mode."src/shared/flow/interfaces/app/actions.ts (1)
1-14: Well-structured interface for IPC actions.The
FlowActionsAPIinterface is clearly defined with descriptive JSDoc comments. The interface properly establishes the contract for IPC event handling related to app actions, which supports the improved application functionality described in the PR objectives.src/main/browser/utility/menu/items/view.ts (2)
7-7: Good integration with the new shortcuts system.The import of
getCurrentShortcutreplaces hardcoded shortcuts, enabling the Shortcuts Editor feature mentioned in the PR objectives.
40-40: Consistently applied dynamic shortcuts across all menu items.The replacement of hardcoded accelerators with
getCurrentShortcutcalls is implemented consistently across all menu items. This change enables runtime customization of keyboard shortcuts through the new Shortcuts Editor feature.Also applies to: 52-52, 61-61, 70-70, 77-77
src/shared/types/shortcuts.ts (1)
1-7: Well-structured interface for keyboard shortcuts managementThe
ShortcutActioninterface is well-designed with clear property names and helpful comments. The inclusion oforiginalShortcutis a good practice for allowing users to reset to default values.src/renderer/src/components/settings/sections/general/reset-onboarding-card.tsx (1)
30-34: Clean UI refactoring to direct markupThe refactoring from Card components to direct HTML elements with utility classes looks good and follows the project's standardization efforts.
src/renderer/src/components/settings/sections/shortcuts/components/reset-dialog.tsx (2)
1-15: Well-structured component interface and importsThe component has a clean interface with appropriate props for handling the dialog state and confirmation action. The imports are well-organized, following best practices by importing specific components from the UI library.
17-44: LGTM! Clean implementation of the reset dialogThe implementation is concise and correctly handles state transitions. The dialog properly shows a warning about the irreversible nature of the reset action and uses appropriate UI components for the dialog structure. The button handlers are well-implemented, ensuring the dialog closes before calling the confirmation callback.
src/renderer/src/components/providers/minimal-toast-provider.tsx (1)
100-113: Good fix for toast animation when showing sequential messagesThe changes address the issue mentioned in PR objectives by ensuring proper animation when transitioning between toasts. By introducing a small delay when replacing an existing toast, you allow the exit animation to complete before starting the entry animation of the new toast.
The implementation uses a smart approach:
- Checking if there's already a message showing
- Clearing it first if needed
- Using a small (100ms) delay to ensure smooth transitions
- Keeping the original functionality for the first toast
src/shared/flow/interfaces/app/shortcuts.ts (2)
1-5: Well-defined type for shortcuts data collectionThe
ShortcutsDatatype is properly defined as an array ofShortcutActionobjects, creating a clear data structure for the shortcuts system.
6-27: Well-designed API interface with comprehensive methodsThe
FlowShortcutsAPIinterface provides a complete set of methods needed for shortcuts management:
- Getting all shortcuts
- Setting individual shortcuts
- Resetting shortcuts to defaults
- Subscribing to shortcut updates
The JSDoc comments provide good documentation, and the method signatures with appropriate return types make the interface clear and type-safe.
src/renderer/src/components/browser-ui/main.tsx (3)
17-17: Good integration of the ActionsProviderAdding the ActionsProvider import prepares for the integration of the centralized action handling system as mentioned in the PR objectives.
80-143: Well-implemented ActionsProvider integrationThe ActionsProvider is correctly positioned to wrap the entire UI subtree, making its context available to all child components. This enables centralized handling of actions like copying links as mentioned in the PR objectives.
97-122: Improved loading animation with better visual transitionsThe topbar loading animation has been refined by:
- Adjusting the height from 2.5 to 2 units
- Using nested motion components for better animation layering
- Maintaining the same functionality with improved visual appearance
This change enhances the user experience with smoother animations.
src/main/browser/utility/menu/items/edit.ts (4)
1-4: Clean import organization.The imports have been properly updated to reflect the architectural changes. The removal of the direct clipboard dependency in favor of the IPC action system (
fireCopyLinkAction) is a good move toward better separation of concerns.
6-6: Improved function signature.Removing the
browserparameter fromcreateEditMenuhelps decouple the menu creation from specific browser instances, making the code more modular and easier to maintain.
16-16: Good use of dynamic shortcut retrieval.Using
getCurrentShortcutinstead of hardcoded values allows for user customization of shortcuts. This change integrates well with the new Shortcuts Editor feature.
18-22: Improved URL copy implementation.The implementation now uses an IPC action system instead of directly accessing the clipboard. This is a better architectural approach that separates concerns and allows for centralized handling of clipboard operations.
src/main/ipc/app/shortcuts.ts (2)
6-9: Good implementation of shortcuts retrieval.The handler correctly uses the shortcuts module to retrieve all shortcuts and returns them to the caller.
24-27: Good event listener implementation.The event listener properly responds to internal shortcut changes and broadcasts them to all listeners. This ensures all UI components stay in sync with the latest shortcuts.
src/renderer/src/components/settings/sections/shortcuts/components/search-header.tsx (2)
5-10: Well-defined component props interface.The props interface clearly defines the expected inputs and callbacks for the component, making it easy to understand how to use it.
26-29: Great user feedback with hover animation.The rotating icon on hover provides excellent visual feedback to the user, enhancing the interactive experience.
src/renderer/src/components/browser-ui/sidebar/header/address-bar/copy-link-button.tsx (1)
3-3: Good refactor to use centralized actions.The component now uses the
useActionshook to access the centralizedcopyLinkaction, which aligns with the architectural goals of the PR to centralize action handling.Also applies to: 54-55
src/renderer/src/components/settings/settings-layout.tsx (3)
2-3: Good organization of settings layout with clear section definitionsThe new sidebar-based settings UI is well structured with clear icons and labels for each section. The addition of the Shortcuts section with the KeyboardIcon is consistent with the overall design pattern.
Also applies to: 13-14, 21-29
57-58: Proper integration of ShortcutsSettings componentThe ShortcutsSettings component is correctly integrated into the switch statement with the appropriate section ID.
66-80: Good component hierarchy with appropriate context providersThe component hierarchy with ShortcutsProvider wrapping SettingsProvider ensures that shortcut context is available to all settings components. The layout structure with a separate sidebar and content area follows modern UI patterns.
src/renderer/src/components/settings/sections/shortcuts/components/empty-state.tsx (4)
4-7: Well-defined props interface with appropriate typesThe EmptyState component has a clear and concise props interface with appropriate type definitions.
9-22: Loading state implementation with proper animationThe loading state provides good user feedback with a spinner and text. The motion animations provide a smooth transition experience.
24-39: No-results state with dynamic search term displayThe no-results state appropriately handles both cases - with and without a search term - providing helpful guidance to users.
41-53: Consistent styling across all state variationsThe default no-shortcuts state maintains consistent styling with the other states, creating a unified user experience. The component follows good React patterns with conditional rendering and proper motion animations.
src/renderer/src/components/settings/sections/general/set-as-default-browser-setting.tsx (3)
11-12: Variable rename improves code clarityRenaming the variable from
defaultBrowserResulttoisDefaultResultmakes the code more readable and consistent with the state variable name.
25-31: Enhanced UI with improved layout and stylingThe refactored layout uses Flexbox effectively with appropriate spacing, hover effects, and text styling. The descriptive text helps users understand the purpose of this setting.
32-56: Improved button states with appropriate visual feedbackThe component now provides clear visual differentiation between loading, not-default, and default states. The button styling is consistent and the use of icons enhances the user experience.
src/main/modules/logs.ts (3)
7-15: Platform-specific log path determinationThe function correctly determines the appropriate log directory based on the platform, following OS conventions for log storage.
19-21: Ensures log directory exists before writingGood practice to create the log directory if it doesn't exist, preventing potential file write errors.
23-28: Well-structured log filename generationThe log filename includes both version and timestamp, making it easy to identify logs from specific application sessions.
src/renderer/src/components/settings/sections/shortcuts/hooks/use-keyboard-normalizer.tsx (1)
52-63: Returning single non-modifier keys bypasses modifier filterThe branch in lines 60-62 returns
normalizedKeyeven when it is itself a modifier alias (Spaceis fine, but e.g.Escapeis filtered earlier).
Consider explicitly filtering‐out any residual modifiers or reserved keys to avoid accidental acceptance of pure modifier keys when users release modifiers in quick succession.src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx (6)
3-3: Good use of utils import for favicon URL generation.The import of
craftActiveFaviconURLfrom utils is a good practice that centralizes the URL generation logic, making it reusable across components.
6-6: AnimatePresence improves UI transitions.Adding
AnimatePresencefrom framer-motion enables smooth exit animations for elements that are removed from the DOM, which will provide better visual feedback for users.
79-80: Good pattern for conditional icon selection.Creating a variable to dynamically select the appropriate icon based on state is cleaner than using inline conditionals in JSX, improving readability.
113-128: Improved favicon rendering with proper fallback.The updated favicon container implementation with proper error handling, rounded corners, and the use of
craftActiveFaviconURLimproves the user experience by ensuring favicons are properly displayed with cache-busting capabilities.
130-155: Well-implemented animation for audio controls.The animation for audio controls using
AnimatePresenceandmotion.divis well-implemented with proper spring physics and transitions. The event propagation stopping in the button'sonMouseDownhandler prevents unintended parent interactions.
157-160: Improved layout with better spacing.The updated layout for the title and right-side container provides better spacing and visual consistency, which improves the overall user interface.
src/main/browser/window.ts (5)
41-41: Good addition of state tracking property.Adding a private property to track window button visibility state is a good approach for maintaining the component's state.
77-79: Improved initialization in constructor.Using the new
setWindowButtonVisibilitymethod in the constructor instead of directly calling native methods improves consistency and centralizes the visibility logic.
91-91: Fixed window controls in fullscreen mode.Adding calls to
_updateWindowButtonVisibility()in both fullscreen event handlers ensures window controls remain visible in fullscreen mode, fixing the issue mentioned in the PR objectives.Also applies to: 96-96
256-264: Well-implemented window button visibility logic.The private method centralizes the visibility logic with proper conditional checks. It ensures window buttons are always visible in fullscreen mode, addressing the issue described in the PR objectives.
266-273: Good API design for button visibility control.The public methods provide a clean API for controlling window button visibility, making it easier to manage visibility state from other components or modules.
src/renderer/src/components/settings/sections/shortcuts/components/category-section.tsx (6)
1-5: Clean imports with proper organization.The imports are well-organized, including only what's necessary for the component to function properly.
6-20: Well-defined props interface with clear typing.The props interface is comprehensive and well-typed, making it clear what data and callbacks the component expects.
37-38: Good implementation of modified shortcuts detection.The component efficiently calculates the number of modified shortcuts by comparing current values with original values, which helps provide useful feedback to users.
40-44: Smooth entry animation for improved UX.The use of framer-motion to animate the component's entry with proper timing and delay creates a pleasant user experience.
45-52: Informative heading with modification indicator.The category heading with a badge showing the number of modified shortcuts provides valuable information to users at a glance.
53-76: Well-structured shortcut items rendering with animations.The component properly maps over shortcuts and renders
ShortcutItemcomponents with appropriate props, including animation delays for a staggered effect. The use ofAnimatePresenceensures smooth exit animations.src/renderer/src/components/providers/actions-provider.tsx (4)
8-20: Well-defined context interface with clear documentation.The context interface is well-documented with JSDoc comments that explain the purpose of each method.
24-30: Good error handling in custom hook.The custom hook properly checks for context existence and throws a helpful error message if used outside the provider.
55-58: TODO comment needs implementation.There's a TODO comment for handling generic incoming actions that should be addressed before finalizing this feature.
Do you have plans for implementing the generic incoming actions handler mentioned in the TODO comment?
60-61: Good use of callback refs for stable references.Using refs to maintain stable references for callbacks in event listeners is a good practice to avoid unnecessary re-subscriptions.
Also applies to: 51-52
src/renderer/src/components/settings/sections/general/basic-settings-cards.tsx (1)
100-104: Avoid using array index as ReactkeyUsing the loop index (
key={index}) prevents React from reliably tracking re-ordering or insertion of cards at runtime, which can cause incorrect DOM mutations and state leakage between cards.If
cardhas a stable identifier (card.idorcard.title), prefer that:- {cards.map((card, index) => ( - <BasicSettingsCard key={index} card={card} /> + {cards.map((card) => ( + <BasicSettingsCard key={card.title} card={card} /> ))}[ suggest_optional_refactor ]
src/renderer/src/components/settings/sections/shortcuts/shortcut-item.tsx (2)
82-110: Icon buttons need accessible labelsThe three ghost buttons rely solely on iconography. Screen-reader users will not detect their purpose because
titleis not consistently announced by all assistive tech.Supply
aria-labeloraria-describedbyattributes:-<Button … title="Save"> +<Button … aria-label="Save shortcut" title="Save"> … -<Button … title="Cancel Edit"> +<Button … aria-label="Cancel editing" title="Cancel Edit"> … -<Button … title="Reset to Default"> +<Button … aria-label="Reset shortcut to default" title="Reset to Default">This small addition markedly improves accessibility compliance.
[ suggest_nitpick ]
114-125: Guard against empty shortcuts in display
formatShortcutForDisplay(shortcut.shortcut)may return an empty string for unassigned shortcuts, producing a visually empty badge.
You already style the “unassigned” case in editing mode; mirror that here for consistency:-{formatShortcutForDisplay(shortcut.shortcut)} +{formatShortcutForDisplay(shortcut.shortcut) || "—"}Optionally add a dedicated style (e.g.,
italic text-muted-foreground) to align with the placeholder used during recording.
[ suggest_nitpick ]src/main/modules/shortcuts.ts (1)
81-88:newShortcutis un-typed – tighten the contract with a dedicated interface
modifiedShortcutData?.newShortcutis accessed blindly. IfgetAllModifiedShortcuts()ever returns a record without that field (or with a typo), the merge silently falls back to the default shortcut and the bug is hard to track.Create a small interface for the persisted shape so TypeScript can help:
// saving/shortcuts.ts export interface ModifiedShortcut { newShortcut: string; } // shortcuts.ts const modifiedShortcutsData = getAllModifiedShortcuts() as Array< - { id: string } & { newShortcut: string } + { id: ShortcutId } & ModifiedShortcut >;Type-safety now guarantees
newShortcutexists and is a string.src/renderer/src/components/settings/sections/external-apps/section.tsx (1)
2-2:motion/reactimport path may be wrongMost projects use
framer-motion(import { motion, AnimatePresence } from "framer-motion").
motion/reactbelongs to Motion-One; unless you deliberately migrated, this will cause a bundler error.-import { motion, AnimatePresence } from "motion/react"; +import { motion, AnimatePresence } from "framer-motion";src/renderer/src/components/ui/alert-dialog.tsx (2)
32-46: Note the coupling of Content, Portal, and Overlay componentsThe AlertDialogContent component includes both the AlertDialogPortal and AlertDialogOverlay components, which means they're tightly coupled. This is a design choice that's worth noting.
This design means that every time AlertDialogContent is used, a new portal and overlay are created. While this simplifies usage, it's important to verify this is the intended behavior across the application.
Consider documenting this relationship in a comment to make it clear to other developers:
function AlertDialogContent({ className, ...props }: React.ComponentProps<typeof AlertDialogPrimitive.Content>) { + // AlertDialogContent automatically includes Portal and Overlay components return ( <AlertDialogPortal> <AlertDialogOverlay />
91-97: Consider handling missing buttonVariantsThe AlertDialogAction and AlertDialogCancel components rely on the buttonVariants utility, but there's no error handling if the utility is unavailable or changed.
The implementation is clean and properly uses the button variants. The code correctly styles action buttons as primary and cancel buttons as outline variant.
src/renderer/src/components/browser-ui/sidebar/header/address-bar/copy-link-button.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/renderer/src/components/providers/shortcuts-provider.tsx (3)
32-41: Consider enhancing the shortcut formatting functionThe formatting function handles some common keys, but could be more comprehensive to support all possible keyboard keys that might be used in shortcuts.
const formatShortcutForDisplay = useCallback((shortcut: string | null): string => { if (!shortcut) return "None"; return shortcut .replace(/\+/g, " + ") .replace("CommandOrControl", "⌘/Ctrl") .replace("ArrowUp", "↑") .replace("ArrowDown", "↓") .replace("ArrowLeft", "←") .replace("ArrowRight", "→") + .replace("Alt", "⌥/Alt") + .replace("Shift", "⇧/Shift") + .replace("Escape", "Esc") + .replace("Delete", "Del") + .replace("Backspace", "⌫") + .replace("Enter", "↵") + .replace("Tab", "⇥"); }, []);
98-109: Optimize loading state management in resetAllShortcutsThe function sets
isLoadingto true but then callsfetchShortcutswhich also setsisLoadingto true, potentially causing unnecessary state updates.const resetAllShortcuts = useCallback(async (): Promise<void> => { setIsLoading(true); try { const resetPromises = shortcuts.map((shortcut) => flow.shortcuts.resetShortcut(shortcut.id)); await Promise.all(resetPromises); - fetchShortcuts(); + // Directly fetch shortcuts without setting isLoading again + try { + const fetchedShortcuts = await flow.shortcuts.getShortcuts(); + setShortcuts(fetchedShortcuts); + } catch (error) { + console.error("Failed to fetch shortcuts:", error); + } } catch (error) { console.error("Failed to reset all shortcuts:", error); } finally { setIsLoading(false); } }, [shortcuts, fetchShortcuts]);
66-80: Consider adding user feedback for errorsThe error handling in async functions only logs to the console but doesn't provide user feedback.
Consider integrating with a toast notification system to inform users when shortcuts cannot be set or reset.
// Example integration with a toast notification system const setShortcut = useCallback( async (actionId: string, shortcut: string): Promise<boolean> => { try { const success = await flow.shortcuts.setShortcut(actionId, shortcut); if (success) { fetchShortcuts(); // Refresh shortcut data } return success; } catch (error) { console.error("Error setting shortcut:", error); // Add user notification // toast.error("Failed to set shortcut. Please try again."); return false; } }, [fetchShortcuts] );Also applies to: 82-96
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/settings/main.ts(1 hunks)src/renderer/src/components/providers/shortcuts-provider.tsx(1 hunks)src/renderer/src/components/settings/sections/shortcuts/components/category-section.tsx(1 hunks)src/renderer/src/components/settings/sections/shortcuts/components/modified-shortcuts-section.tsx(1 hunks)src/renderer/src/components/settings/sections/shortcuts/hooks/use-keyboard-normalizer.tsx(1 hunks)src/renderer/src/components/settings/sections/shortcuts/section.tsx(1 hunks)src/renderer/src/components/settings/sections/shortcuts/shortcut-item.tsx(1 hunks)src/shared/flow/interfaces/app/shortcuts.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/renderer/src/components/settings/sections/shortcuts/components/modified-shortcuts-section.tsx
- src/shared/flow/interfaces/app/shortcuts.ts
- src/renderer/src/components/settings/sections/shortcuts/section.tsx
- src/renderer/src/components/settings/sections/shortcuts/hooks/use-keyboard-normalizer.tsx
- src/renderer/src/components/settings/sections/shortcuts/components/category-section.tsx
- src/renderer/src/components/settings/sections/shortcuts/shortcut-item.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/renderer/src/components/providers/shortcuts-provider.tsx (1)
src/shared/types/shortcuts.ts (1)
ShortcutAction(1-7)
🔇 Additional comments (5)
src/renderer/src/components/providers/shortcuts-provider.tsx (5)
1-13: Well-structured shortcuts context and value interfaceThe interfaces and context setup follow React best practices, providing a clear contract for the shortcut management system.
15-21: Good implementation of context hook with error boundaryThe custom hook implementation correctly ensures the context is used within a provider and provides a helpful error message.
43-53: Good implementation of data fetching with proper loading state managementThe
fetchShortcutsfunction correctly manages loading state and includes robust error handling.
55-64: Well-implemented effect hook with proper cleanupThe useEffect hook correctly fetches data on mount and sets up a subscription with proper cleanup.
111-125: Well-structured context provider implementationThe provider correctly passes all necessary functions and state to children components.
1 participant
Summary:
Add Shortcuts Editor which allows you to customize the shortcuts.
Random Little Changes:
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Chores
Refactor