fix: 替换原生 alert/confirm 解决 Electron 焦点劫持 bug#117
Merged
AmintaCCCP merged 7 commits intomainfrom Apr 27, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a unified dialog/toast notification system by creating new UI components ( Changes
Sequence DiagramsequenceDiagram
actor User
participant Component
participant DialogProvider as DialogProvider<br/>(Context)
participant Toast
participant ConfirmDialog
User->>Component: Trigger action (e.g., delete)
Component->>DialogProvider: await confirm(title, message, options)
DialogProvider->>ConfirmDialog: Open with state
ConfirmDialog->>User: Display modal with Cancel/Confirm buttons
User->>ConfirmDialog: Click Confirm or Cancel
ConfirmDialog->>DialogProvider: Resolve promise (true/false)
DialogProvider->>Component: Return boolean result
Component->>Component: Perform action if confirmed
Component->>DialogProvider: toast(message, 'success'|'error')
DialogProvider->>Toast: Render with type styling
Toast->>User: Display notification
User->>Toast: (auto-dismiss after duration)
Toast->>DialogProvider: onClose()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/settings/WebDAVPanel.tsx (1)
48-62:⚠️ Potential issue | 🟡 MinorMulti-line validation errors collapse to a single line in the toast.
Native
alert(translated.join('\n'))rendered each error on its own line. The newToastrenders the message inside<p>{message}</p>(Toast.tsx Line 57), so\nis collapsed to a space and several validation errors run together as one wall of text. Either join with a visible separator here, or letToasthonor whitespace.🐛 Quick fix in this caller
- toast(translated.join('\n'), 'error'); + toast(translated.join('；'), 'error');Alternatively, in
src/components/ui/Toast.tsxLine 57, usewhitespace-pre-lineon the<p>to preserve newlines for all callers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/WebDAVPanel.tsx` around lines 48 - 62, The toast message collapses newline characters because Toast.tsx renders the error string inside a plain <p>, so in WebDAVPanel.handleSave where you call toast(translated.join('\n'), 'error') the newlines are lost; fix it either by joining errors with a visible separator (e.g., translated.join(' • ') or translated.join('\n\n') combined with changing Toast) or, better, update Toast.tsx (around the <p> rendering on Line 57) to preserve newlines by applying CSS white-space: pre-line (or className "whitespace-pre-line") to the <p>, so callers like WebDAVPanel.handleSave and others retain multiline formatting without changing callers.
🧹 Nitpick comments (6)
src/components/settings/BackupPanel.tsx (1)
116-120: RedundantsetIsRestoring(false)—finallyalready handles it.The
finallyblock at Lines 254-256 always resetsisRestoring. The explicit reset before this early return is unnecessary.if (!backupContent) { - setIsRestoring(false); toast(t('备份文件内容为空，无法恢复。', 'Backup file is empty, cannot restore.'), 'error'); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/BackupPanel.tsx` around lines 116 - 120, Remove the redundant setIsRestoring(false) call in the early-return branch that checks backupContent; keep the toast(t('备份文件内容为空，无法恢复。', 'Backup file is empty, cannot restore.')) and return, since the existing finally block in the restore handler already resets isRestoring. Update the function (the handler that references backupContent, setIsRestoring, toast and contains the finally) to rely on the finally cleanup and delete the extra setIsRestoring(false).src/components/ui/Toast.tsx (1)
36-49: Effect re-runs ononCloseidentity change can reset the saved focus.Including
onClosein the dependency array means a non-memoized callback from a future caller would re-run this effect, re-capturingpreviousActiveElement.currentfromdocument.activeElementat that moment (potentially the toast itself) and restarting the auto-dismiss timer. It works today only becauseDialogProvider'scloseToastisuseCallback(..., []). Consider stashingonClosein a ref so the effect runs once on mount, decoupling correctness from caller memoization.♻️ Proposed refactor
- useEffect(() => { - previousActiveElement.current = document.activeElement as HTMLElement; - - timeoutRef.current = setTimeout(() => { - onClose(); - }, duration); - - return () => { - if (timeoutRef.current) { - clearTimeout(timeoutRef.current); - } - previousActiveElement.current?.focus(); - }; - }, [duration, onClose]); + const onCloseRef = useRef(onClose); + useEffect(() => { onCloseRef.current = onClose; }, [onClose]); + + useEffect(() => { + previousActiveElement.current = document.activeElement as HTMLElement; + timeoutRef.current = setTimeout(() => onCloseRef.current(), duration); + return () => { + if (timeoutRef.current) clearTimeout(timeoutRef.current); + previousActiveElement.current?.focus(); + }; + }, [duration]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/Toast.tsx` around lines 36 - 49, The effect currently includes onClose in its dependency array which can cause re-runs that overwrite previousActiveElement and restart timeout; fix this by stashing the onClose callback into a ref (e.g., onCloseRef.current = onClose in a small effect) and then use onCloseRef.current inside the timeout handler, remove onClose from the main useEffect dependencies so the mount behavior (saving previousActiveElement.current and starting timeoutRef) is stable; keep duration in dependencies only if you intentionally want timer reset when duration changes, otherwise remove it so the effect runs once on mount; update references to previousActiveElement, timeoutRef, and onClose accordingly.src/hooks/useDialog.tsx (1)
53-59: Toast overwrite drops the previous instance'sonClosesemantics.Calling
toast()while another toast is showing replacestoastState; the previous Toast unmounts (via key change) without itsonClosebeing invoked. Today this is harmless becausecloseToastonly doessetToastState(null). IfonCloseever gains behavior (analytics, callback to caller), this silently regresses. Optional: queue toasts or expose a stable API guarantee.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDialog.tsx` around lines 53 - 59, The current toast() replaces toastState causing the previous Toast to unmount without invoking its onClose; update the implementation so that when toast() is called and an existing toastState exists you first invoke its onClose (if present) before calling setToastState with the new value (or implement a simple FIFO queue inside the hook to enqueue subsequent messages and only unmount after calling onClose). Target the useDialog hook's toast, closeToast and setToastState logic: detect existing toastState in toast(), call existing toastState.onClose() safely before replacing, and ensure closeToast still clears state and calls onClose consistently.src/components/settings/AIConfigPanel.tsx (1)
45-62: Two parallel notification systems remain in this file.The migration introduces
toast()for most flows, but the legacynotification/showNotification/notificationTimerRefstate is still used at Lines 279, 288, 296, and 310 (and rendered inline at Lines 476-489). This creates inconsistent UX: some messages appear top-right via the new Toast portal, others appear inline within the form panel. Consider routing the remainingshowNotification(...)calls throughtoast(...)and removing the inline notification block, or—if the inline placement is intentional for prompt-editor feedback—document that distinction. Otherwise the legacy timer/state is just additional code paths to maintain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/AIConfigPanel.tsx` around lines 45 - 62, The file still maintains the legacy inline notification state (notification, showNotification, notificationTimerRef) while new flows use toast(); remove the legacy system by replacing the remaining calls to showNotification(...) with toast(...) (convert the three types 'success'|'error'|'info' to the appropriate toast variant), delete the notification state, notificationTimerRef and showNotification function, and remove the inline notification render block; if inline placement is desired instead, document that choice and convert toast calls to use the inline state consistently — otherwise eliminate the old state and timer to keep all messages routed through toast.src/components/ui/ConfirmDialog.tsx (2)
98-111: Staticid="confirm-dialog-title"may collide.
aria-labelledbyand the heading both use the literal idconfirm-dialog-title. Today only oneConfirmDialogis mounted at a time viaDialogProvider, so this is not actively broken, but it's a footgun if another consumer ever renders the component directly or stacks dialogs. Consider generating a unique id per instance via React'suseId().♻️ Proposed change
-import React, { useEffect, useRef } from 'react'; +import React, { useEffect, useId, useRef } from 'react'; @@ const cancelButtonRef = useRef<HTMLButtonElement>(null); + const titleId = useId(); @@ - aria-labelledby="confirm-dialog-title" + aria-labelledby={titleId} @@ - <h3 id="confirm-dialog-title" className="text-lg font-semibold text-gray-900 dark:text-text-primary"> + <h3 id={titleId} className="text-lg font-semibold text-gray-900 dark:text-text-primary">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/ConfirmDialog.tsx` around lines 98 - 111, The ConfirmDialog currently uses a static id "confirm-dialog-title" for both aria-labelledby and the h3 which can collide when multiple dialogs are mounted; update the ConfirmDialog component to call React's useId() (import useId) to generate a unique id per instance and replace the literal "confirm-dialog-title" with that generated id in both the aria-labelledby attribute on the Dialog wrapper and the id prop on the h3 heading so each instance has a unique accessible label.
49-63: Add Tab focus trapping foraria-modal="true"dialogs.Both
ConfirmDialog(line 97) andReadmeModal(line 346) declarearia-modal="true"and handle Escape-to-close, but neither traps Tab focus. Keyboard users can Tab out of the modal into background controls while it's open. Since these are critical interaction dialogs (especially the danger-type confirmation), focus should cycle between focusable descendants without escaping the modal. Implement a Tab/Shift+Tab keydown listener that cycles through buttons, inputs, and focusable elements within the dialog, or extract a reusableuseFocusTraphook to share between these components. This aligns with WAI-ARIA modal dialog patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/ConfirmDialog.tsx` around lines 49 - 63, ConfirmDialog and ReadmeModal declare aria-modal="true" but only handle Escape; add focus trapping so Tab/Shift+Tab cycles inside the dialog and cannot reach background controls. Implement a reusable hook (e.g., useFocusTrap) or extend the existing useEffect in ConfirmDialog/ReadmeModal: when isOpen is true, query focusable elements inside the dialog root, set initial focus to the first/primary control, and add a keydown listener that intercepts Tab/Shift+Tab to move focus among those elements (wrapping from last→first and first→last), and remove the listener on close; reference the existing handleEscape listener, isOpen, and onCancel to place/remove the trap consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ui/ConfirmDialog.tsx`:
- Around line 75-91: The backdrop click handler is attached to the outer
container so clicks land on the fixed backdrop sibling and event.target !==
event.currentTarget, preventing onCancel from firing; move the onClick from the
outer container (where dialogContent is rendered) onto the backdrop div (the
element with class "fixed inset-0 bg-black/50"), remove the handleBackdropClick
function and its event.target === event.currentTarget check, and ensure onCancel
is invoked directly from the backdrop's onClick handler (leave the dialog's
markup unchanged so clicks inside the dialog do not trigger the backdrop
handler).
In `@src/components/ui/Toast.tsx`:
- Around line 53-66: The toast lacks screen-reader semantics and the icon-only
close button has no accessible name; update the toastContent wrapper div (the
outer <div> containing the toast) to include an appropriate ARIA live region and
role (e.g., aria-live="polite" or "assertive" and role="status" or "alert"
depending on severity) so screen readers announce messages, and add an
accessible name to the close button (the button with onClick={onClose} and the X
icon) by adding aria-label="Close notification" (or similar) or include
visually-hidden text for the icon; ensure Icon and message remain unchanged but
that the container and close button have these ARIA attributes for proper
accessibility.
In `@src/hooks/useDialog.tsx`:
- Around line 61-91: confirm currently overwrites confirmState.resolve causing
earlier promises to hang; modify the confirm function so before calling
setConfirmState it checks if confirmState.resolve exists and if so calls
it(false) to auto-cancel the pending promise, then sets the new state with the
new resolve; ensure confirm uses the existing confirmState reference (or a
functional updater) to avoid stale closures and that handleConfirm/handleCancel
continue to clear resolve and close the dialog via setConfirmState({ ... ,
isOpen: false, resolve: null }).
---
Outside diff comments:
In `@src/components/settings/WebDAVPanel.tsx`:
- Around line 48-62: The toast message collapses newline characters because
Toast.tsx renders the error string inside a plain <p>, so in
WebDAVPanel.handleSave where you call toast(translated.join('\n'), 'error') the
newlines are lost; fix it either by joining errors with a visible separator
(e.g., translated.join(' • ') or translated.join('\n\n') combined with changing
Toast) or, better, update Toast.tsx (around the <p> rendering on Line 57) to
preserve newlines by applying CSS white-space: pre-line (or className
"whitespace-pre-line") to the <p>, so callers like WebDAVPanel.handleSave and
others retain multiline formatting without changing callers.
---
Nitpick comments:
In `@src/components/settings/AIConfigPanel.tsx`:
- Around line 45-62: The file still maintains the legacy inline notification
state (notification, showNotification, notificationTimerRef) while new flows use
toast(); remove the legacy system by replacing the remaining calls to
showNotification(...) with toast(...) (convert the three types
'success'|'error'|'info' to the appropriate toast variant), delete the
notification state, notificationTimerRef and showNotification function, and
remove the inline notification render block; if inline placement is desired
instead, document that choice and convert toast calls to use the inline state
consistently — otherwise eliminate the old state and timer to keep all messages
routed through toast.
In `@src/components/settings/BackupPanel.tsx`:
- Around line 116-120: Remove the redundant setIsRestoring(false) call in the
early-return branch that checks backupContent; keep the
toast(t('备份文件内容为空，无法恢复。', 'Backup file is empty, cannot restore.')) and return,
since the existing finally block in the restore handler already resets
isRestoring. Update the function (the handler that references backupContent,
setIsRestoring, toast and contains the finally) to rely on the finally cleanup
and delete the extra setIsRestoring(false).
In `@src/components/ui/ConfirmDialog.tsx`:
- Around line 98-111: The ConfirmDialog currently uses a static id
"confirm-dialog-title" for both aria-labelledby and the h3 which can collide
when multiple dialogs are mounted; update the ConfirmDialog component to call
React's useId() (import useId) to generate a unique id per instance and replace
the literal "confirm-dialog-title" with that generated id in both the
aria-labelledby attribute on the Dialog wrapper and the id prop on the h3
heading so each instance has a unique accessible label.
- Around line 49-63: ConfirmDialog and ReadmeModal declare aria-modal="true" but
only handle Escape; add focus trapping so Tab/Shift+Tab cycles inside the dialog
and cannot reach background controls. Implement a reusable hook (e.g.,
useFocusTrap) or extend the existing useEffect in ConfirmDialog/ReadmeModal:
when isOpen is true, query focusable elements inside the dialog root, set
initial focus to the first/primary control, and add a keydown listener that
intercepts Tab/Shift+Tab to move focus among those elements (wrapping from
last→first and first→last), and remove the listener on close; reference the
existing handleEscape listener, isOpen, and onCancel to place/remove the trap
consistently.
In `@src/components/ui/Toast.tsx`:
- Around line 36-49: The effect currently includes onClose in its dependency
array which can cause re-runs that overwrite previousActiveElement and restart
timeout; fix this by stashing the onClose callback into a ref (e.g.,
onCloseRef.current = onClose in a small effect) and then use onCloseRef.current
inside the timeout handler, remove onClose from the main useEffect dependencies
so the mount behavior (saving previousActiveElement.current and starting
timeoutRef) is stable; keep duration in dependencies only if you intentionally
want timer reset when duration changes, otherwise remove it so the effect runs
once on mount; update references to previousActiveElement, timeoutRef, and
onClose accordingly.
In `@src/hooks/useDialog.tsx`:
- Around line 53-59: The current toast() replaces toastState causing the
previous Toast to unmount without invoking its onClose; update the
implementation so that when toast() is called and an existing toastState exists
you first invoke its onClose (if present) before calling setToastState with the
new value (or implement a simple FIFO queue inside the hook to enqueue
subsequent messages and only unmount after calling onClose). Target the
useDialog hook's toast, closeToast and setToastState logic: detect existing
toastState in toast(), call existing toastState.onClose() safely before
replacing, and ensure closeToast still clears state and calls onClose
consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5fba592-0b72-4030-ba61-fc7d7f5d9bde
📒 Files selected for processing (9)
src/App.tsxsrc/components/settings/AIConfigPanel.tsxsrc/components/settings/BackendPanel.tsxsrc/components/settings/BackupPanel.tsxsrc/components/settings/CategoryPanel.tsxsrc/components/settings/WebDAVPanel.tsxsrc/components/ui/ConfirmDialog.tsxsrc/components/ui/Toast.tsxsrc/hooks/useDialog.tsx
问题：window.alert() / confirm() 在 Electron 中会破坏 renderer 进 程的焦点状态。用户点击确定关闭对话框后，之前聚焦的 input 元素丢失 焦点且无法重新获得，必须 Alt+Tab 切换窗口才能恢复。 解决方案： - 创建 Toast 组件替代 alert()，使用 React Portal 渲染 - 创建 ConfirmDialog 组件替代 confirm()，返回 Promise 支持 async/await - 创建 useDialog hook 提供 toast() 和 confirm() API - 在 App.tsx 添加 DialogProvider - 替换所有设置面板中的 alert/confirm 调用 新文件： - src/components/ui/Toast.tsx - src/components/ui/ConfirmDialog.tsx - src/hooks/useDialog.tsx 修改面板：AIConfigPanel, BackendPanel, WebDAVPanel, BackupPanel, CategoryPanel Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AmintaCCCP
force-pushed
the
fix/electron-dialog-focus-bug
branch
from
99b3286 to
6e8d011
Compare
AmintaCCCP
added a commit
that referenced
this pull request
Apr 27, 2026
Closed
AmintaCCCP
added a commit
that referenced
this pull request
Apr 27, 2026
问题：window.alert() / confirm() 在 Electron 中会破坏 renderer 进 程的焦点状态。用户点击确定关闭对话框后，之前聚焦的 input 元素丢失 焦点且无法重新获得，必须 Alt+Tab 切换窗口才能恢复。 解决方案： - 创建 Toast 组件替代 alert()，使用 React Portal 渲染 - 创建 ConfirmDialog 组件替代 confirm()，返回 Promise 支持 async/await - 创建 useDialog hook 提供 toast() 和 confirm() API - 在 App.tsx 添加 DialogProvider - 替换所有设置面板中的 alert/confirm 调用 新文件： - src/components/ui/Toast.tsx - src/components/ui/ConfirmDialog.tsx - src/hooks/useDialog.tsx 修改面板：AIConfigPanel, BackendPanel, WebDAVPanel, BackupPanel, CategoryPanel Refs: #109, #113, #117 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AmintaCCCP
force-pushed
the
fix/electron-dialog-focus-bug
branch
from
6a86197 to
976bec5
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/settings/WebDAVPanel.tsx (1)
50-62:⚠️ Potential issue | 🟠 MajorToast does not preserve newlines—multiple validation errors will render as a single line.
The Toast component renders the message in a
<p>tag withoutwhite-space: pre-lineor similar CSS. Whentoast(translated.join('\n'), 'error')is called, the newline characters collapse into whitespace, creating a run-on string instead of separate lines. Either applywhite-space: pre-lineto the Toast's message element, or refactor to display multiple errors as separate toasts or a list.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/WebDAVPanel.tsx` around lines 50 - 62, Validation messages in WebDAVPanel.tsx are joined with '\n' and passed to toast(translated.join('\n'), 'error'), but the Toast component collapses newlines so errors render on one line; fix by either updating the toast usage to emit separate messages (e.g., iterate over translated and call toast(msg, 'error') for each) or adjust the Toast component's message container to preserve line breaks (add CSS like white-space: pre-line to the Toast message element); locate the translated variable and the toast(...) call in WebDAVPanel and implement one of these two fixes so multiple validation errors display as separate lines or separate toasts.
🧹 Nitpick comments (4)
src/components/SubscriptionRepoCard.tsx (1)
470-512: Consider migrating the inline unstar Modal to the newconfirm()API for consistency.The PR centralizes confirmation flows through
useDialog().confirm(...), but this component keeps a bespokeModal-based unstar confirmation (withunstarConfirmOpen,pendingUnstarAction, etc.). Migrating toconfirm({ type: 'danger', confirmText: ... })would remove the dedicated state, the inline modal JSX, and the deferred-callback indirection inhandleStar(Lines 140-145).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SubscriptionRepoCard.tsx` around lines 470 - 512, Replace the inline Modal-based unstar flow (states unstarConfirmOpen and pendingUnstarAction and the Modal JSX) by calling the centralized dialog API: useDialog().confirm(...) from the point where you currently open the modal (see handleStar usage). Remove the unstarConfirmOpen and pendingUnstarAction state and the Modal block, and instead invoke confirm({ type: 'danger', confirmText: t('确认取消','Confirm Unstar'), title: ..., description: ..., onConfirm: () => { /* perform the unstar logic currently in pendingUnstarAction */ }}) so the unstar logic runs directly in onConfirm and the component no longer needs the deferred-callback indirection.src/components/UpdateChecker.tsx (1)
47-51: Error feedback duplicated between toast and inline banner.
setError(errorMessage)still drives the inline error block at Lines 95-99, whiletoast(errorMessage, 'error')now also surfaces the same message. Users will see two simultaneous notifications. Consider removing one source of truth — either drop theerrorstate and inline block, or skip the toast for non-silent failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UpdateChecker.tsx` around lines 47 - 51, The error message is currently duplicated because setError(errorMessage) drives the inline error banner while toast(errorMessage, 'error') shows the same message; fix this by removing the toast call in the error path of UpdateChecker (keep setError to populate the inline banner) so only the inline block driven by the error state is shown on failures; ensure any existing silent logic remains intact and no other code paths still call toast with the same error.src/components/CategorySidebar.tsx (1)
399-402: Redundant conditional: both branches return the same value.
isSidebarCollapsed ? 'calc(100vh - 8rem)' : 'calc(100vh - 8rem)'evaluates to the same string regardless of state. Simplify to a constant string.♻️ Proposed cleanup
style={{ - maxHeight: isSidebarCollapsed ? 'calc(100vh - 8rem)' : 'calc(100vh - 8rem)', + maxHeight: 'calc(100vh - 8rem)', transitionProperty: 'width, padding, max-height', }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/CategorySidebar.tsx` around lines 399 - 402, The inline style for the sidebar sets maxHeight via a redundant ternary using isSidebarCollapsed that returns the same string in both branches; simplify by replacing the ternary with the constant string 'calc(100vh - 8rem)' directly in the style object used in CategorySidebar (look for the style prop where maxHeight is set and the identifier isSidebarCollapsed is referenced) so the code no longer performs an unnecessary conditional.src/hooks/useDialog.tsx (1)
93-96: Memoize the context value to avoid re-rendering all consumers each render.
valueis rebuilt every render, which causes every component readinguseDialog()to re-render wheneverDialogProviderre-renders (e.g., on each toast/confirm state change). Wrapping inuseMemokeeps the identity stable sincetoast/confirmare already memoized withuseCallback.♻️ Suggested refactor
- const value: DialogContextValue = { - toast, - confirm, - }; + const value = useMemo<DialogContextValue>(() => ({ toast, confirm }), [toast, confirm]);Also add
useMemoto the import on line 1.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDialog.tsx` around lines 93 - 96, The context `value` (of type DialogContextValue) is being re-created each render which forces all `useDialog()` consumers to re-render; wrap the object { toast, confirm } in `useMemo` (and add `useMemo` to the imports) so its identity is stable and depends only on `toast` and `confirm` (both already memoized), then return/use that memoized `value` in the provider.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/AssetFilterManager.tsx`:
- Around line 131-152: The catch block rollback uses the stale assetFilters
closure; change it to read live state via useAppStore.getState().assetFilters
(or equivalent store getter) inside the catch before performing existence
checks, or simpler: unconditionally call deleteAssetFilter for every id in
addedFilterIds and unconditionally call addAssetFilter for every entry in
previousFilters (Zustand actions should be idempotent), then restore selections
via onFilterToggle for selectedFilters and previousSelected as before; update
references to assetFilters.find(...) to use the live state variable or remove
the conditional checks entirely.
- Around line 116-130: The reset success path currently toggles off selected
preset filters (presetFilters loop using deleteAssetFilter and onFilterToggle)
but never re-applies the user's previously selected presets on success; update
the success branch after DEFAULT_PRESET_FILTERS/addAssetFilter/addedFilterIds
processing to replay the saved previousSelected selections (e.g., iterate
previousSelected and call onFilterToggle for each id still present in
assetFilters) so user selections are restored on successful reset; ensure you
reference presetFilters, assetFilters, deleteAssetFilter, onFilterToggle,
DEFAULT_PRESET_FILTERS, addAssetFilter, addedFilterIds and previousSelected when
making the change.
In `@src/components/DiscoveryView.tsx`:
- Line 416: The destructuring of useDialog in DiscoveryView.tsx includes an
unused confirm variable causing an ESLint warning; update the statement in the
component (where const { toast, confirm } = useDialog(); appears) to only
extract the used value (toast) — e.g., change to destructure just toast from
useDialog — and run lint to verify the warning is resolved.
In `@src/components/Header.tsx`:
- Around line 403-410: The dialog title is inconsistent between locales; update
the confirm(...) call inside the onClick handler to use a matching noun-phrase
title for both languages (e.g., change the first argument currently '确定要退出登录吗？'
to a Chinese noun phrase like '退出登录确认' while keeping the English 'Logout
Confirmation'), i.e., modify the t(...) invocation used in the onClick handler
so both localized strings convey the same noun-phrase header.
- Around line 113-123: The toast messages in handleSync are hard-coded Chinese;
update the success/info/error to use the existing localization helper (t or the
language === 'zh' ? ... : ...) so English users see English. Locate the toast
calls inside handleSync (the branches that currently call toast(`同步完成！发现
${newRepoCount} 个新仓库。`, 'success'); toast('同步完成！所有仓库都是最新的。', 'info');
toast('GitHub token 已过期或无效，请重新登录。', 'error'); and toast('同步失败，请检查网络连接。',
'error');) and replace each literal Chinese string with the localized variant
using t(zhString, enString) or the language conditional; keep the same toast
type and ensure the token-expired branch still calls logout() after showing the
localized error.
In `@src/components/RepositoryCard.tsx`:
- Around line 273-277: The toast message in handleAIAnalyze uses the English
string for both t() arguments so Chinese users see English; update the second
argument of t(...) in RepositoryCard.tsx (inside the handleAIAnalyze function
where toast is called) to the Chinese translation used elsewhere, e.g. 'GitHub
token 未找到，请重新登录。', so the call becomes t('GitHub token not found. Please login
again.', 'GitHub token 未找到，请重新登录。') before passing it to toast.
---
Outside diff comments:
In `@src/components/settings/WebDAVPanel.tsx`:
- Around line 50-62: Validation messages in WebDAVPanel.tsx are joined with '\n'
and passed to toast(translated.join('\n'), 'error'), but the Toast component
collapses newlines so errors render on one line; fix by either updating the
toast usage to emit separate messages (e.g., iterate over translated and call
toast(msg, 'error') for each) or adjust the Toast component's message container
to preserve line breaks (add CSS like white-space: pre-line to the Toast message
element); locate the translated variable and the toast(...) call in WebDAVPanel
and implement one of these two fixes so multiple validation errors display as
separate lines or separate toasts.
---
Nitpick comments:
In `@src/components/CategorySidebar.tsx`:
- Around line 399-402: The inline style for the sidebar sets maxHeight via a
redundant ternary using isSidebarCollapsed that returns the same string in both
branches; simplify by replacing the ternary with the constant string 'calc(100vh
- 8rem)' directly in the style object used in CategorySidebar (look for the
style prop where maxHeight is set and the identifier isSidebarCollapsed is
referenced) so the code no longer performs an unnecessary conditional.
In `@src/components/SubscriptionRepoCard.tsx`:
- Around line 470-512: Replace the inline Modal-based unstar flow (states
unstarConfirmOpen and pendingUnstarAction and the Modal JSX) by calling the
centralized dialog API: useDialog().confirm(...) from the point where you
currently open the modal (see handleStar usage). Remove the unstarConfirmOpen
and pendingUnstarAction state and the Modal block, and instead invoke confirm({
type: 'danger', confirmText: t('确认取消','Confirm Unstar'), title: ...,
description: ..., onConfirm: () => { /* perform the unstar logic currently in
pendingUnstarAction */ }}) so the unstar logic runs directly in onConfirm and
the component no longer needs the deferred-callback indirection.
In `@src/components/UpdateChecker.tsx`:
- Around line 47-51: The error message is currently duplicated because
setError(errorMessage) drives the inline error banner while toast(errorMessage,
'error') shows the same message; fix this by removing the toast call in the
error path of UpdateChecker (keep setError to populate the inline banner) so
only the inline block driven by the error state is shown on failures; ensure any
existing silent logic remains intact and no other code paths still call toast
with the same error.
In `@src/hooks/useDialog.tsx`:
- Around line 93-96: The context `value` (of type DialogContextValue) is being
re-created each render which forces all `useDialog()` consumers to re-render;
wrap the object { toast, confirm } in `useMemo` (and add `useMemo` to the
imports) so its identity is stable and depends only on `toast` and `confirm`
(both already memoized), then return/use that memoized `value` in the provider.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: af074983-f66b-4cb3-8a84-e57904f153f8
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
src/components/AssetFilterManager.tsxsrc/components/CategoryEditModal.tsxsrc/components/CategorySidebar.tsxsrc/components/DiscoveryView.tsxsrc/components/Header.tsxsrc/components/ReleaseTimeline.tsxsrc/components/RepositoryCard.tsxsrc/components/RepositoryList.tsxsrc/components/SubscriptionRepoCard.tsxsrc/components/UpdateChecker.tsxsrc/components/settings/AIConfigPanel.tsxsrc/components/settings/BackendPanel.tsxsrc/components/settings/BackupPanel.tsxsrc/components/settings/CategoryPanel.tsxsrc/components/settings/WebDAVPanel.tsxsrc/components/ui/ConfirmDialog.tsxsrc/components/ui/Toast.tsxsrc/hooks/useDialog.tsxsrc/main.tsx
✅ Files skipped from review due to trivial changes (2)
- src/components/ui/Toast.tsx
- src/components/settings/AIConfigPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/settings/BackupPanel.tsx
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/RepositoryCard.tsx (1)
275-275:⚠️ Potential issue | 🟡 MinorChinese localization missing — both
t()arguments are English.
t('GitHub token not found. Please login again.', 'GitHub token not found. Please login again.')passes the English text as the Chinese argument. Other localized calls in this file (e.g., line 534t('未找到 GitHub Token，请重新登录。', ...)) use the proper Chinese string; this one should match.✏️ Suggested fix
const handleAIAnalyze = async () => { if (!githubToken) { - toast(t('GitHub token not found. Please login again.', 'GitHub token not found. Please login again.'), 'error'); + toast(t('未找到 GitHub Token，请重新登录。', 'GitHub token not found. Please login again.'), 'error'); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/RepositoryCard.tsx` at line 275, In RepositoryCard.tsx update the incorrect i18n call used in the toast: replace the current t('GitHub token not found. Please login again.', 'GitHub token not found. Please login again.') with the proper Chinese-first localization (e.g., t('未找到 GitHub Token，请重新登录。', 'GitHub token not found. Please login again.')) so the t(...) arguments match other calls in this file; this change involves the t(...) call passed into toast(...) near the existing RepositoryCard component logic.
🧹 Nitpick comments (3)
src/components/CategorySidebar.tsx (2)
247-256: Nit: reuset()for consistency.The rest of the component routes localized strings through the
t(zh, en)helper.handleSyncErrorre-implements thelanguage === 'zh'check inline — minor inconsistency.♻️ Proposed tweak
toast( - language === 'zh' - ? `同步到后端失败，已恢复分类更改。` - : `Failed to sync to backend. Category change has been reverted.`, + t('同步到后端失败，已恢复分类更改。', 'Failed to sync to backend. Category change has been reverted.'), 'error' );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/CategorySidebar.tsx` around lines 247 - 256, handleSyncError currently builds the localized toast message with an inline language check instead of using the existing t() helper; replace the conditional string with a call to t() so the message is consistent with the rest of the component (updateRepository and setDragOverCategoryId stay the same, call toast(t('同步到后端失败，已恢复分类更改。', 'Failed to sync to backend. Category change has been reverted.'), 'error')).
399-402: Redundant ternary — both branches are identical.
maxHeightresolves to'calc(100vh - 8rem)'regardless ofisSidebarCollapsed, so the conditional is dead andtransitionProperty: 'max-height'no longer animates anything. Either drop the ternary or restore distinct collapsed/expanded heights if the animation was intended.♻️ Proposed simplification
style={{ - maxHeight: isSidebarCollapsed ? 'calc(100vh - 8rem)' : 'calc(100vh - 8rem)', - transitionProperty: 'width, padding, max-height', + maxHeight: 'calc(100vh - 8rem)', + transitionProperty: 'width, padding', }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/CategorySidebar.tsx` around lines 399 - 402, The inline style in CategorySidebar.tsx uses a redundant ternary for maxHeight (both branches set 'calc(100vh - 8rem') and sets transitionProperty: 'width, padding, max-height' even though max-height doesn't change; update the style in the component (where isSidebarCollapsed is referenced) to either remove the ternary and use a single maxHeight plus remove 'max-height' from transitionProperty, or restore distinct values for the collapsed and expanded states (e.g., different maxHeight when isSidebarCollapsed is true) and keep 'max-height' in transitionProperty so the collapse/expand animates correctly.src/components/RepositoryCard.tsx (1)
577-585: Drag image: comment is slightly misleading.The comment on line 582 says "offsetX/Y 使拖拽图片中心对准鼠标位置" (center the drag image on the cursor), but the computed
event.clientX - rect.left/event.clientY - rect.topactually preserves the cursor's offset relative to where the user grabbed the card (which is the desired behavior). Consider updating the comment to reflect this, e.g., "保持光标相对于卡片的抓取位置".Functionally, the code is correct:
event.currentTargetis the drag handle and.closest('.repository-card')reliably resolves to the card container declared on line 726.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/RepositoryCard.tsx` around lines 577 - 585, Update the misleading comment above the setDragImage call to state that the computed offsets (event.clientX - rect.left, event.clientY - rect.top) preserve the cursor's grab position relative to the card rather than centering the image; locate the block using event.currentTarget.closest('.repository-card') and event.dataTransfer.setDragImage(cardElement, ...) and replace the Chinese comment "offsetX/Y 使拖拽图片中心对准鼠标位置" with a clearer phrasing such as "保持光标相对于卡片的抓取位置".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/RepositoryCard.tsx`:
- Line 275: In RepositoryCard.tsx update the incorrect i18n call used in the
toast: replace the current t('GitHub token not found. Please login again.',
'GitHub token not found. Please login again.') with the proper Chinese-first
localization (e.g., t('未找到 GitHub Token，请重新登录。', 'GitHub token not found. Please
login again.')) so the t(...) arguments match other calls in this file; this
change involves the t(...) call passed into toast(...) near the existing
RepositoryCard component logic.
---
Nitpick comments:
In `@src/components/CategorySidebar.tsx`:
- Around line 247-256: handleSyncError currently builds the localized toast
message with an inline language check instead of using the existing t() helper;
replace the conditional string with a call to t() so the message is consistent
with the rest of the component (updateRepository and setDragOverCategoryId stay
the same, call toast(t('同步到后端失败，已恢复分类更改。', 'Failed to sync to backend. Category
change has been reverted.'), 'error')).
- Around line 399-402: The inline style in CategorySidebar.tsx uses a redundant
ternary for maxHeight (both branches set 'calc(100vh - 8rem') and sets
transitionProperty: 'width, padding, max-height' even though max-height doesn't
change; update the style in the component (where isSidebarCollapsed is
referenced) to either remove the ternary and use a single maxHeight plus remove
'max-height' from transitionProperty, or restore distinct values for the
collapsed and expanded states (e.g., different maxHeight when isSidebarCollapsed
is true) and keep 'max-height' in transitionProperty so the collapse/expand
animates correctly.
In `@src/components/RepositoryCard.tsx`:
- Around line 577-585: Update the misleading comment above the setDragImage call
to state that the computed offsets (event.clientX - rect.left, event.clientY -
rect.top) preserve the cursor's grab position relative to the card rather than
centering the image; locate the block using
event.currentTarget.closest('.repository-card') and
event.dataTransfer.setDragImage(cardElement, ...) and replace the Chinese
comment "offsetX/Y 使拖拽图片中心对准鼠标位置" with a clearer phrasing such as
"保持光标相对于卡片的抓取位置".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2abd76cc-8f21-4559-8038-13d1185e8846
📒 Files selected for processing (2)
src/components/CategorySidebar.tsxsrc/components/RepositoryCard.tsx
AmintaCCCP
added a commit
that referenced
this pull request
Apr 27, 2026
问题：window.alert() / confirm() 在 Electron 中会破坏 renderer 进 程的焦点状态。用户点击确定关闭对话框后，之前聚焦的 input 元素丢失 焦点且无法重新获得，必须 Alt+Tab 切换窗口才能恢复。 解决方案： - 创建 Toast 组件替代 alert()，使用 React Portal 渲染 - 创建 ConfirmDialog 组件替代 confirm()，返回 Promise 支持 async/await - 创建 useDialog hook 提供 toast() 和 confirm() API - 在 App.tsx 添加 DialogProvider - 替换所有设置面板中的 alert/confirm 调用 新文件： - src/components/ui/Toast.tsx - src/components/ui/ConfirmDialog.tsx - src/hooks/useDialog.tsx 修改面板：AIConfigPanel, BackendPanel, WebDAVPanel, BackupPanel, CategoryPanel Refs: #109, #113, #117 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AmintaCCCP
force-pushed
the
fix/electron-dialog-focus-bug
branch
from
976bec5 to
5948f22
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
LandingPage/robots.txt (1)
1-61:⚠️ Potential issue | 🔴 CriticalMove
robots.txttopublic/directory so it is published todist/during build.Currently
LandingPage/robots.txtis not in thepublic/directory, so Vite's build process will not copy it todist/robots.txt. Since the Dockerfile serves only/app/distvia nginx, this file will never be published at/robots.txt. As a result, crawlers will not receive these Content-Signal directives (including theai-train=nopolicy), rendering them ineffective. MoveLandingPage/robots.txt→public/robots.txtto ensure it is included in the build output.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LandingPage/robots.txt` around lines 1 - 61, The robots.txt file is currently not in the static assets directory used by the Vite build, so it never gets copied to the build output and won't be served; move the robots.txt file into the project's public static directory (so Vite will include it in dist during build), confirm the file name remains robots.txt, and re-run the build to verify dist contains robots.txt and that the container/webserver will serve it from the site root.
♻️ Duplicate comments (1)
src/hooks/useDialog.tsx (1)
61-81:⚠️ Potential issue | 🔴 CriticalConcurrent
confirm()calls still cause earlier promises to hang forever.
confirmStatecarries a singleresolveslot. Ifconfirm()is invoked while a previous dialog is still pending (two delete confirmations fromWebDAVPanel/CategoryPanel/AIConfigPanel, a re-entrantawait confirm, etc.), the newsetConfirmState({ ..., resolve })replaces the prior resolver and the first awaiter never settles — itsawait confirm(...)hangs and anyfinally/setIsRestoring(false)cleanup leaks. Resolve the prior pending promise (withfalse, treating it as cancelled) before opening the new dialog, or queue the requests.🔒 Proposed fix — auto-cancel any pending confirm when a new one is opened
const confirm = useCallback(( title: string, message: string, options?: { confirmText?: string; cancelText?: string; type?: 'danger' | 'warning' | 'info'; } ): Promise<boolean> => { return new Promise((resolve) => { - setConfirmState({ - isOpen: true, - title, - message, - confirmText: options?.confirmText, - cancelText: options?.cancelText, - type: options?.type || 'warning', - resolve, - }); + setConfirmState((prev) => { + // Cancel any pending confirm so its awaiter does not hang. + prev.resolve?.(false); + return { + isOpen: true, + title, + message, + confirmText: options?.confirmText, + cancelText: options?.cancelText, + type: options?.type || 'warning', + resolve, + }; + }); }); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDialog.tsx` around lines 61 - 81, The confirm function in useDialog creates a Promise and stores its resolver in confirmState, but new calls overwrite the resolver causing previous promises to hang; before calling setConfirmState in confirm(...)/useCallback, detect if confirmState?.resolve exists and call it(false) to auto-cancel the prior pending confirm, then proceed to setConfirmState with the new resolver; ensure you access the existing resolver via the stored state (confirmState) and nullify/replace it only after calling it so you don't double-resolve, preserving Promise semantics for components awaiting confirm().
🧹 Nitpick comments (6)
LandingPage/main.js (1)
62-62: Inner loop should start atj = i + 1(avoid self-pair).With
j = i, the first iteration computes a distance of0betweenparticles[i]and itself, takes the< 120branch, and draws a zero-length line every frame. Cheap but wasteful, and this nested O(n²) runs every frame for up to 150 particles.♻️ Proposed micro-fix
- for (let j = i; j < particles.length; j++) { + for (let j = i + 1; j < particles.length; j++) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LandingPage/main.js` at line 62, The inner loop currently starts with j = i causing each particle to be compared to itself and draw zero-length lines; change the inner loop initialization in the double-loop that iterates over particles (the loop using variables i and j over the particles array) to start at j = i + 1 so self-pairs are skipped and you only process unique pairs, reducing wasted work in that O(n²) per-frame routine.src/components/CategorySidebar.tsx (2)
399-402: Nit: ternary picks the same value in both branches.
maxHeightiscalc(100vh - 8rem)regardless ofisSidebarCollapsed. The conditional can be dropped.♻️ Proposed cleanup
style={{ - maxHeight: isSidebarCollapsed ? 'calc(100vh - 8rem)' : 'calc(100vh - 8rem)', + maxHeight: 'calc(100vh - 8rem)', transitionProperty: 'width, padding, max-height', }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/CategorySidebar.tsx` around lines 399 - 402, In CategorySidebar update the inline style where maxHeight is set with a ternary (currently "maxHeight: isSidebarCollapsed ? 'calc(100vh - 8rem)' : 'calc(100vh - 8rem)'") — replace the ternary with a single constant value "calc(100vh - 8rem)" so maxHeight is set directly and remove the redundant conditional expression; keep transitionProperty as-is.
213-217: Nit: error message for delete sync failure is slightly misleading.
deleteCustomCategory(category.id)already mutated local state, so by the time the catch block runs, the category has been deleted client-side; only the backend sync failed. Compare withhandleHideDefaultCategory(Lines 233–238), which rolls back viashowDefaultCategory(...)and surfaces the same kind of failure. Either:
- restore the deleted category on sync failure to keep semantics symmetric, or
- reword the toast to match
handleSyncError("Failed to sync to backend …") so users aren't told a delete failed when it actually persisted locally.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/CategorySidebar.tsx` around lines 213 - 217, The catch block after calling forceSyncToBackend is misleading because deleteCustomCategory(category.id) has already removed the category locally; either restore the local state on sync failure (call the appropriate restore function—e.g., add or show the deleted custom category back using the inverse of deleteCustomCategory, or reuse an existing restore helper) and then call toast with the existing handleSyncError-style message, or simply change the toast text to match handleSyncError ("Failed to sync to backend …") so it correctly reports a backend sync failure; update the catch in the delete flow to reference deleteCustomCategory, forceSyncToBackend, and the restore helper or handleSyncError wording accordingly.LandingPage/style.css (1)
218-247: Optional: align with stylelint conventions (kebab-case keyframe names, no quotes around single-token font names).Stylelint flagged
traversePath1..4,fadeUp,pulseGlow,rotateReverse(keyframes-name-pattern) and the quoted'Inter'/'SFMono-Regular'(font-family-name-quotes). Renames are mechanical (rename the@keyframesand theanimationreferences together), and quotes can simply be dropped for single-token names. Defer if the team has chosen to suppress these rules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LandingPage/style.css` around lines 218 - 247, Rename the keyframes traversePath1, traversePath2, traversePath3, traversePath4, fadeUp, pulseGlow, and rotateReverse to kebab-case (e.g., traverse-path-1, traverse-path-2, traverse-path-3, traverse-path-4, fade-up, pulse-glow, rotate-reverse) and update every corresponding animation/animation-name usage to match those new identifiers (search for "traversePath", "fadeUp", "pulseGlow", "rotateReverse" to update references); also remove unnecessary quotes around single-token font-family names like 'Inter' and 'SFMono-Regular' in any font-family declarations so they become Inter and SFMono-Regular to satisfy stylelint rules.src/hooks/useDialog.tsx (1)
22-28: Nit: exposedurationon thetoast()API.
Toastaccepts an optionalduration(default 3s), but the hook signaturetoast(message, type?)doesn't surface it. Long error messages or copy-on-failure flows would benefit from a longer dwell time. Threadingdurationthrough the context type,ToastState, and theToastrender at Lines 101–107 is a cheap, additive change.Also applies to: 53-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDialog.tsx` around lines 22 - 28, Update the toast API to accept an optional duration parameter and thread it through the types and render so callers can extend how long to show toasts. Change the toast signature (function named toast) to toast(message: string, type?: ToastType, durationMs?: number) => void (or similar), add duration?: number to the ToastState shape and to the Context/type declarations where ToastState is referenced, and pass that duration into the Toast renderer (the component/logic at the Toast render block around lines 101–107) using a default of 3000ms when duration is undefined; also update the other toast signatures referenced around lines 53–55 to match.src/components/ui/Toast.tsx (1)
1-1: Drop the unuseduseIdimport and call.ESLint flags
'toastId' is assigned a value but never used. The variable is computed on every render and never referenced.♻️ Proposed cleanup
-import React, { useEffect, useRef, useId } from 'react'; +import React, { useEffect, useRef } from 'react'; @@ - const onCloseRef = useRef(onClose); - onCloseRef.current = onClose; - const toastId = useId(); + const onCloseRef = useRef(onClose); + onCloseRef.current = onClose;Also applies to: 37-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/Toast.tsx` at line 1, Remove the unused React useId import and the unused toastId generation in Toast.tsx: delete useId from the import list and remove the toastId = useId() call (and any unused variable declaration) so ESLint no longer flags "assigned a value but never used"; keep all other toast logic (useEffect, useRef, etc.) unchanged and ensure no other references to toastId remain in the component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/settings.local.json:
- Around line 1-23: The committed personal config .claude/settings.local.json
should be ignored to avoid machine-specific churn: add an ignore rule for
".claude/settings.local.json" (or the entire ".claude/" directory, if you prefer
to allow only .claude/settings.json) to the repository .gitignore, verify that
.claude/settings.local.json is removed from the index (git rm --cached) if still
tracked, and commit the updated .gitignore so only the shared
.claude/settings.json remains tracked.
In `@LandingPage/style.css`:
- Line 213: The CSS rule for .path-beam uses an invalid paint-server reference
("stroke: url(style.css)") so the SVG gradient stroke doesn't render; update the
.path-beam rule to reference a valid SVG paint-server id (e.g., stroke:
url(`#grad1`)) or replace url(...) with a valid CSS color/gradient, ensuring the
id matches the <linearGradient> or <radialGradient> defined inside the SVG
markup; verify .line-art-container and the SVG include the corresponding
<defs><linearGradient id="grad1">...</linearGradient></defs> (or change the
stroke to a plain color) so the animated beam displays correctly on desktop.
In `@src/components/ui/Toast.tsx`:
- Around line 39-52: The Toast component currently restores focus on unmount by
always calling previousActiveElement.current?.focus() in the useEffect cleanup,
which can steal focus from user interactions; change the cleanup in the
useEffect inside the Toast component so it only restores focus when the active
element hasn't changed (e.g., when document.activeElement === document.body) or
remove focus restoration entirely for Toasts; locate the useEffect that sets
previousActiveElement.current and timeoutRef and update its return handler to
conditionally call previousActiveElement.current?.focus() (or skip it), keeping
onCloseRef behavior unchanged.
---
Outside diff comments:
In `@LandingPage/robots.txt`:
- Around line 1-61: The robots.txt file is currently not in the static assets
directory used by the Vite build, so it never gets copied to the build output
and won't be served; move the robots.txt file into the project's public static
directory (so Vite will include it in dist during build), confirm the file name
remains robots.txt, and re-run the build to verify dist contains robots.txt and
that the container/webserver will serve it from the site root.
---
Duplicate comments:
In `@src/hooks/useDialog.tsx`:
- Around line 61-81: The confirm function in useDialog creates a Promise and
stores its resolver in confirmState, but new calls overwrite the resolver
causing previous promises to hang; before calling setConfirmState in
confirm(...)/useCallback, detect if confirmState?.resolve exists and call
it(false) to auto-cancel the prior pending confirm, then proceed to
setConfirmState with the new resolver; ensure you access the existing resolver
via the stored state (confirmState) and nullify/replace it only after calling it
so you don't double-resolve, preserving Promise semantics for components
awaiting confirm().
---
Nitpick comments:
In `@LandingPage/main.js`:
- Line 62: The inner loop currently starts with j = i causing each particle to
be compared to itself and draw zero-length lines; change the inner loop
initialization in the double-loop that iterates over particles (the loop using
variables i and j over the particles array) to start at j = i + 1 so self-pairs
are skipped and you only process unique pairs, reducing wasted work in that
O(n²) per-frame routine.
In `@LandingPage/style.css`:
- Around line 218-247: Rename the keyframes traversePath1, traversePath2,
traversePath3, traversePath4, fadeUp, pulseGlow, and rotateReverse to kebab-case
(e.g., traverse-path-1, traverse-path-2, traverse-path-3, traverse-path-4,
fade-up, pulse-glow, rotate-reverse) and update every corresponding
animation/animation-name usage to match those new identifiers (search for
"traversePath", "fadeUp", "pulseGlow", "rotateReverse" to update references);
also remove unnecessary quotes around single-token font-family names like
'Inter' and 'SFMono-Regular' in any font-family declarations so they become
Inter and SFMono-Regular to satisfy stylelint rules.
In `@src/components/CategorySidebar.tsx`:
- Around line 399-402: In CategorySidebar update the inline style where
maxHeight is set with a ternary (currently "maxHeight: isSidebarCollapsed ?
'calc(100vh - 8rem)' : 'calc(100vh - 8rem)'") — replace the ternary with a
single constant value "calc(100vh - 8rem)" so maxHeight is set directly and
remove the redundant conditional expression; keep transitionProperty as-is.
- Around line 213-217: The catch block after calling forceSyncToBackend is
misleading because deleteCustomCategory(category.id) has already removed the
category locally; either restore the local state on sync failure (call the
appropriate restore function—e.g., add or show the deleted custom category back
using the inverse of deleteCustomCategory, or reuse an existing restore helper)
and then call toast with the existing handleSyncError-style message, or simply
change the toast text to match handleSyncError ("Failed to sync to backend …")
so it correctly reports a backend sync failure; update the catch in the delete
flow to reference deleteCustomCategory, forceSyncToBackend, and the restore
helper or handleSyncError wording accordingly.
In `@src/components/ui/Toast.tsx`:
- Line 1: Remove the unused React useId import and the unused toastId generation
in Toast.tsx: delete useId from the import list and remove the toastId = useId()
call (and any unused variable declaration) so ESLint no longer flags "assigned a
value but never used"; keep all other toast logic (useEffect, useRef, etc.)
unchanged and ensure no other references to toastId remain in the component.
In `@src/hooks/useDialog.tsx`:
- Around line 22-28: Update the toast API to accept an optional duration
parameter and thread it through the types and render so callers can extend how
long to show toasts. Change the toast signature (function named toast) to
toast(message: string, type?: ToastType, durationMs?: number) => void (or
similar), add duration?: number to the ToastState shape and to the Context/type
declarations where ToastState is referenced, and pass that duration into the
Toast renderer (the component/logic at the Toast render block around lines
101–107) using a default of 3000ms when duration is undefined; also update the
other toast signatures referenced around lines 53–55 to match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b9fae820-2e0f-4e76-a1c7-bd9029a2b85e
⛔ Files ignored due to path filters (1)
LandingPage/assets/logo.pngis excluded by!**/*.png
📒 Files selected for processing (14)
.claude/settings.local.jsonLandingPage/.claude/settings.local.jsonLandingPage/index.htmlLandingPage/main.jsLandingPage/robots.txtLandingPage/style.csssrc/components/CategorySidebar.tsxsrc/components/DiscoveryView.tsxsrc/components/RepositoryCard.tsxsrc/components/UpdateChecker.tsxsrc/components/settings/BackupPanel.tsxsrc/components/ui/ConfirmDialog.tsxsrc/components/ui/Toast.tsxsrc/hooks/useDialog.tsx
✅ Files skipped from review due to trivial changes (4)
- LandingPage/.claude/settings.local.json
- LandingPage/index.html
- src/components/settings/BackupPanel.tsx
- src/components/DiscoveryView.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/ui/ConfirmDialog.tsx
- src/components/UpdateChecker.tsx
- src/components/RepositoryCard.tsx
AmintaCCCP
added a commit
that referenced
this pull request
Apr 27, 2026
问题：window.alert() / confirm() 在 Electron 中会破坏 renderer 进 程的焦点状态。用户点击确定关闭对话框后，之前聚焦的 input 元素丢失 焦点且无法重新获得，必须 Alt+Tab 切换窗口才能恢复。 解决方案： - 创建 Toast 组件替代 alert()，使用 React Portal 渲染 - 创建 ConfirmDialog 组件替代 confirm()，返回 Promise 支持 async/await - 创建 useDialog hook 提供 toast() 和 confirm() API - 在 App.tsx 添加 DialogProvider - 替换所有设置面板中的 alert/confirm 调用 新文件： - src/components/ui/Toast.tsx - src/components/ui/ConfirmDialog.tsx - src/hooks/useDialog.tsx 修改面板：AIConfigPanel, BackendPanel, WebDAVPanel, BackupPanel, CategoryPanel Refs: #109, #113, #117 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AmintaCCCP
force-pushed
the
fix/electron-dialog-focus-bug
branch
from
5948f22 to
d058803
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Header.tsx (1)
69-123:⚠️ Potential issue | 🟡 MinorToast messages are not language-aware.
Line 69 hardcodes English while lines 113/115/120/123 hardcode Chinese, so users in the opposite locale will see the wrong language. The rest of this file uses the
t(zh, en)helper for UI strings — these toasts should follow the same pattern for consistency with the existing localization scheme used throughout the component.🌐 Proposed fix
const handleSync = async () => { if (!githubToken) { - toast('GitHub token not found. Please login again.', 'error'); + toast(t('未找到 GitHub Token，请重新登录。', 'GitHub token not found. Please login again.'), 'error'); return; } @@ if (newRepoCount > 0) { - toast(`同步完成！发现 ${newRepoCount} 个新仓库。`, 'success'); + toast(t(`同步完成！发现 ${newRepoCount} 个新仓库。`, `Sync completed! Found ${newRepoCount} new repositories.`), 'success'); } else { - toast('同步完成！所有仓库都是最新的。', 'info'); + toast(t('同步完成！所有仓库都是最新的。', 'Sync completed! All repositories are up to date.'), 'info'); } } catch (error) { console.error('Sync failed:', error); if (error instanceof Error && error.message.includes('token')) { - toast('GitHub token 已过期或无效，请重新登录。', 'error'); + toast(t('GitHub token 已过期或无效，请重新登录。', 'GitHub token has expired or is invalid. Please login again.'), 'error'); logout(); } else { - toast('同步失败，请检查网络连接。', 'error'); + toast(t('同步失败，请检查网络连接。', 'Sync failed. Please check your network connection.'), 'error'); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Header.tsx` around lines 69 - 123, The toast strings in the sync flow are hardcoded in different languages; update all toast(...) calls in the sync method in Header.tsx to use the existing t(zh, en) localization helper (the same pattern used elsewhere in this component) so messages are language-aware: replace the initial "GitHub token not found. Please login again.", the success/info messages after computing newRepoCount, the token-expired message inside the catch that calls logout(), and the generic "sync failed" toast with t(...) variants; keep the current control flow (setLoading, GitHubApiService, getAllStarredRepositories, setRepositories, getMultipleRepositoryReleases, setReleases, setLastSync) unchanged and only change the toast string arguments to t(zh, en).
🧹 Nitpick comments (1)
src/components/RepositoryList.tsx (1)
299-307: Consider'info'severity for "nothing to do" cases.For "no repositories to analyze" (lines 299–307) and "none of the selected repositories are subscribed" (lines 797–806), no error actually occurred — the user's selection is simply a no-op. Using
'error'styling here can read as alarming. Other panels in this PR use'error'only for genuine failures.♻️ Proposed change
- toast(message, 'error'); + toast(message, 'info'); return; } @@ if (subscribedRepos.length === 0) { toast(language === 'zh' ? '选中的仓库中没有被订阅的' : 'None of the selected repositories are subscribed', - 'error' + 'info' ); return; }Also applies to: 797-806
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/RepositoryList.tsx` around lines 299 - 307, The toast calls for "nothing to do" cases use 'error' severity but should be informational; update the toast invocation in the targetRepos.length === 0 branch (the block referencing targetRepos, analyzeFailedOnly, analyzeUnanalyzedOnly and toast) to use 'info' instead of 'error', and make the same change in the "none of the selected repositories are subscribed" block (the second branch around the selected-repos subscription check that also calls toast) so both no-op messages use 'info' styling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/DiscoveryView.tsx`:
- Line 416: The destructuring in DiscoveryView.tsx pulls `confirm` from
useDialog() but never uses it, causing a lint error; update the destructure in
the component (where `const { toast, confirm } = useDialog()` appears) to only
extract the used symbol (e.g., `toast`) or rename to a prefixed unused variable
if you intend to keep it, ensuring `confirm` is removed so the linter stops
flagging an unused variable.
In `@src/components/RepositoryCard.tsx`:
- Line 275: The toast call in RepositoryCard currently passes the English string
as the first argument to t(...) causing Chinese users to see English; update the
toast invocation that shows "GitHub token not found. Please login again." to
call t with the Chinese string as the first arg and the English string as the
second (i.e., t('GitHub 令牌未找到。请重新登录。', 'GitHub token not found. Please login
again.')), so it matches the other validation toasts and uses the t function
correctly; modify the specific toast(...) inside the RepositoryCard component
where t is used for this error.
---
Outside diff comments:
In `@src/components/Header.tsx`:
- Around line 69-123: The toast strings in the sync flow are hardcoded in
different languages; update all toast(...) calls in the sync method in
Header.tsx to use the existing t(zh, en) localization helper (the same pattern
used elsewhere in this component) so messages are language-aware: replace the
initial "GitHub token not found. Please login again.", the success/info messages
after computing newRepoCount, the token-expired message inside the catch that
calls logout(), and the generic "sync failed" toast with t(...) variants; keep
the current control flow (setLoading, GitHubApiService,
getAllStarredRepositories, setRepositories, getMultipleRepositoryReleases,
setReleases, setLastSync) unchanged and only change the toast string arguments
to t(zh, en).
---
Nitpick comments:
In `@src/components/RepositoryList.tsx`:
- Around line 299-307: The toast calls for "nothing to do" cases use 'error'
severity but should be informational; update the toast invocation in the
targetRepos.length === 0 branch (the block referencing targetRepos,
analyzeFailedOnly, analyzeUnanalyzedOnly and toast) to use 'info' instead of
'error', and make the same change in the "none of the selected repositories are
subscribed" block (the second branch around the selected-repos subscription
check that also calls toast) so both no-op messages use 'info' styling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e5124fd-0f21-46e4-9a9d-09fc33f5d699
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
src/components/AssetFilterManager.tsxsrc/components/CategoryEditModal.tsxsrc/components/CategorySidebar.tsxsrc/components/DiscoveryView.tsxsrc/components/Header.tsxsrc/components/ReleaseTimeline.tsxsrc/components/RepositoryCard.tsxsrc/components/RepositoryList.tsxsrc/components/SubscriptionRepoCard.tsxsrc/components/UpdateChecker.tsxsrc/components/settings/AIConfigPanel.tsxsrc/components/settings/BackendPanel.tsxsrc/components/settings/BackupPanel.tsxsrc/components/settings/CategoryPanel.tsxsrc/components/settings/WebDAVPanel.tsxsrc/components/ui/ConfirmDialog.tsxsrc/components/ui/Toast.tsxsrc/hooks/useDialog.tsxsrc/main.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/ui/Toast.tsx
- src/components/settings/AIConfigPanel.tsx
- src/components/settings/BackupPanel.tsx
- src/components/settings/BackendPanel.tsx
AmintaCCCP
added a commit
that referenced
this pull request
Apr 27, 2026
问题：window.alert() / confirm() 在 Electron 中会破坏 renderer 进 程的焦点状态。用户点击确定关闭对话框后，之前聚焦的 input 元素丢失 焦点且无法重新获得，必须 Alt+Tab 切换窗口才能恢复。 解决方案： - 创建 Toast 组件替代 alert()，使用 React Portal 渲染 - 创建 ConfirmDialog 组件替代 confirm()，返回 Promise 支持 async/await - 创建 useDialog hook 提供 toast() 和 confirm() API - 在 App.tsx 添加 DialogProvider - 替换所有设置面板中的 alert/confirm 调用 新文件： - src/components/ui/Toast.tsx - src/components/ui/ConfirmDialog.tsx - src/hooks/useDialog.tsx 修改面板：AIConfigPanel, BackendPanel, WebDAVPanel, BackupPanel, CategoryPanel Refs: #109, #113, #117 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AmintaCCCP
force-pushed
the
fix/electron-dialog-focus-bug
branch
from
d058803 to
51057d4
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/settings/BackupPanel.tsx (1)
102-256:⚠️ Potential issue | 🟡 MinorOuter
try/finallyswallows WebDAV errors silently.
webdavService.listFiles()(line 105) andwebdavService.downloadFile(latestBackup)(line 114) sit outside the innertry/catchon lines 121–252, and the outer block has only afinally. If either call rejects (network failure, auth error, etc.), the rejection propagates out ofhandleRestorewith no user-facing toast — the user just sees the spinner stop. Consider wrapping the whole restore body or adding a siblingcatchto surface a localized error toast, mirroring the backup path's behavior.🛡️ Suggested fix
try { const webdavService = new WebDAVService(activeConfig); const files = await webdavService.listFiles(); ... try { const backupData = JSON.parse(backupContent); ... } catch (error) { console.error('Restore failed:', error); toast(`${t('恢复失败', 'Restore failed')}: ${(error as Error).message}`, 'error'); } + } catch (error) { + console.error('Restore failed:', error); + toast(`${t('恢复失败', 'Restore failed')}: ${(error as Error).message}`, 'error'); } finally { setIsRestoring(false); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/BackupPanel.tsx` around lines 102 - 256, The outer try/finally around the restore flow (which calls WebDAVService.listFiles() and downloadFile()) currently lacks a catch, so network/auth errors are swallowed; modify handleRestore to catch errors from webdavService.listFiles() and webdavService.downloadFile(latestBackup) (either by wrapping the entire restore body in a try/catch or adding a sibling catch after the outer try) and in that catch call toast(...) with a localized error message and console.error, ensuring setIsRestoring(false) stays in the finally; reference the symbols webdavService.listFiles, webdavService.downloadFile, setIsRestoring, and toast when implementing the change.src/components/RepositoryCard.tsx (1)
295-303:⚠️ Potential issue | 🟡 MinorConfirm multi-line separators render correctly in ConfirmDialog.
The
confirmMessagestrings contain literal\n\nseparators (lines 297 and 539) that will collapse to single spaces in the ConfirmDialog's<p>element, since it has no CSS for whitespace preservation. The message will appear visually concatenated. Either addwhite-space: pre-wrapto the dialog message container or replace\n\nwith explicit semantic markup (e.g., separate<div>elements or proper paragraph structure).[Applies to lines 297–298 and 539–540]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/RepositoryCard.tsx` around lines 295 - 303, The confirmMessage uses literal "\n\n" which gets collapsed in the ConfirmDialog's <p>, so update the UI to preserve line breaks: either modify the ConfirmDialog component (the message container element rendered by confirm(...) / ConfirmDialog) to apply CSS "white-space: pre-wrap" to its paragraph/container, or change the call site (the repository.analyzed_at branch where confirmMessage is built and passed to confirm(t(...), confirmMessage, { type: 'warning' })) to pass semantic markup (e.g., two <p> elements or an array of React nodes) instead of a plain string; adjust ConfirmDialog to accept ReactNode if needed and ensure the confirm(...) consumer still works.
♻️ Duplicate comments (1)
src/components/ui/Toast.tsx (1)
60-62:⚠️ Potential issue | 🟡 Minor
role="alert"+aria-live="polite"are contradictory; derive both fromtype.
role="alert"has an implicitaria-live="assertive"+aria-atomic="true", which overrides the explicitaria-live="polite". This creates conflicting guidance to assistive technology. It's also semantically incorrect forsuccessandinfotoasts, which should not interrupt users. Map the role and aria-live from thetypeprop so errors announce assertively and success/info announce politely.Suggested fix
- <div - role="alert" - aria-live="polite" + <div + role={type === 'error' ? 'alert' : 'status'} + aria-live={type === 'error' ? 'assertive' : 'polite'}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/Toast.tsx` around lines 60 - 62, The Toast component currently hardcodes role="alert" and aria-live="polite", which conflicts and is semantically wrong; update the Toast rendering to derive role, aria-live and aria-atomic from the type prop (e.g., in the Toast component: if type === 'error' set role="alert", aria-live="assertive", aria-atomic="true"; for 'success'/'info' set role="status" (or no role), aria-live="polite", and aria-atomic="false" or omit it), and remove the fixed attributes so screen readers get the correct, non-conflicting announcement behavior.
🧹 Nitpick comments (6)
src/components/DiscoveryView.tsx (1)
781-789: Mixed-result analyses show as a red error toast even when most succeeded.
failCount > 0 ? 'error' : 'success'flips the toast to'error'whenever even a single repository fails, despite the message reading like a partial-success summary (e.g., "成功 19 个，失败 1 个"). Users will see a red error notification for an otherwise successful batch. Consider'info'(or'success'with the failure count appended) whensuccessCount > 0 && failCount > 0, reserving'error'for the all-failed case.♻️ Suggested change
toast( t( `AI分析完成！成功 ${successCount} 个${failCount > 0 ? `，失败 ${failCount} 个` : ''}`, `AI analysis complete! ${successCount} succeeded${failCount > 0 ? `, ${failCount} failed` : ''}` ), - failCount > 0 ? 'error' : 'success' + successCount === 0 ? 'error' : failCount > 0 ? 'info' : 'success' );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DiscoveryView.tsx` around lines 781 - 789, The toast severity should not be 'error' whenever failCount > 0; update the toast call in DiscoveryView (around successCount/failCount and the toast invocation) to use 'error' only when all analyses failed (successCount === 0 && failCount > 0), use 'info' when there are mixed results (successCount > 0 && failCount > 0), and keep 'success' when failCount === 0; adjust the third argument to toast accordingly so the notification color matches the overall outcome.LandingPage/main.js (1)
58-76: Off-by-one in particle-pair loop:jshould start ati + 1.
for (let j = i; j < particles.length; j++)runs the inner body once withj === i, wheredx = dy = 0,distance = 0, and the code happily strokes a degenerate zero-length line from each particle to itself every frame. It's harmless visually but pure waste at 60 fps. Startjati + 1to skip the self-pair.♻️ Suggested fix
- for (let j = i; j < particles.length; j++) { + for (let j = i + 1; j < particles.length; j++) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LandingPage/main.js` around lines 58 - 76, The inner particle-pair loop currently starts with j = i which causes each particle to be compared with itself (wasting work and drawing zero-length lines); change the inner loop initialization in the block that iterates over particles (the for loop referencing particles[i] and particles[j]) so j starts at i + 1 instead of i, i.e. update the loop that declares "for (let j = i; j < particles.length; j++)" to begin at "i + 1" to skip self-pairs and avoid redundant distance calculations and drawing.src/hooks/useDialog.tsx (2)
87-95:handleConfirm/handleCancelclose over a stale resolver after re-renders duringconfirm()reentry.Both callbacks depend on
confirmState.resolve(deps array on lines 90, 95). Between the timeconfirm()resolves the previous awaiter (via the updater on line 73) and the new state commits, a synchronous follow-upconfirm()call from the same handler will use a new resolver while the buttons inConfirmDialogmay still be wired to the previoushandleConfirmsnapshot until the re-render flushes. In practice this is rare, but combined with the impurity insetConfirmState's updater the resolver bookkeeping is fragile. The ref-based pattern suggested for line 70–85 also fixes this —handleConfirm/handleCancelwould readpendingResolveRef.currentand not needconfirmState.resolvein their deps.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDialog.tsx` around lines 87 - 95, The handlers handleConfirm and handleCancel close over confirmState.resolve making them use a stale resolver on re-entry; change them to read the current resolver from a ref (e.g. pendingResolveRef.current created alongside the ref-based pattern used for the confirm() logic) instead of from confirmState.resolve, update the resolver via that ref whenever confirm() sets a new resolver, and remove confirmState.resolve from the handlers' dependency arrays so handleConfirm/handleCancel become stable and use pendingResolveRef.current to call resolve(true)/resolve(false) before calling setConfirmState to close the dialog.
1-1: ImportReactas default to align with repo convention and avoid relying on global namespace.Line 44 uses
React.FC<DialogProviderProps>, but the import statement on line 1 only includes named exports—no default React import. This works because@types/reactdeclares a globalReactnamespace, but it's inconsistent with every other component in this repository (all useimport React, { ... }). Additionally, withisolatedModules: truein tsconfig, explicitly importing React is the safer, future-proof approach.Consider either adding the default import or replacing
React.FCwith a plain function signature:♻️ Suggested change
-import { useState, useCallback, createContext, useContext, ReactNode, useMemo } from 'react'; +import React, { useState, useCallback, createContext, useContext, ReactNode, useMemo } from 'react';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDialog.tsx` at line 1, The file imports only named React exports but uses the default React namespace in the DialogProvider component signature (React.FC<DialogProviderProps>); update the top import to include the default React import (i.e., import React, { useState, useCallback, createContext, useContext, ReactNode, useMemo } from 'react') or alternatively change the component type to an explicit function signature (remove React.FC and type props directly) so that the usage around DialogProvider and DialogProviderProps is consistent with the repo and compatible with isolatedModules.LandingPage/style.css (1)
19-19: Optional: align with project Stylelint config (font-family quotes & kebab-case keyframes).Stylelint is reporting:
font-family-name-quoteson'Inter'(line 19) and'SFMono-Regular'(line 91) — generic-or-keyword family names should be unquoted.keyframes-name-patternontraversePath1..4,fadeUp,pulseGlow,rotateReverse— project lint expects kebab-case.These don't affect rendering, but will fail CI if Stylelint runs in
--max-warnings 0mode.♻️ Suggested renames (also update animation references on lines 68–69, 72, 81–83, 165–172, 217, 244)
-@keyframes traversePath1 { ... } -@keyframes traversePath2 { ... } -@keyframes traversePath3 { ... } -@keyframes traversePath4 { ... } -@keyframes fadeUp { ... } -@keyframes pulseGlow { ... } -@keyframes rotateReverse { ... } +@keyframes traverse-path-1 { ... } +@keyframes traverse-path-2 { ... } +@keyframes traverse-path-3 { ... } +@keyframes traverse-path-4 { ... } +@keyframes fade-up { ... } +@keyframes pulse-glow { ... } +@keyframes rotate-reverse { ... }Also applies to: 91-91, 218-221, 242-242, 245-245, 247-247
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LandingPage/style.css` at line 19, Remove quotes from generic-or-keyword font family names in the CSS (e.g., replace 'Inter' and 'SFMono-Regular' with Inter and SFMono-Regular) to satisfy font-family-name-quotes; rename the keyframes to kebab-case (e.g., traverse-path-1, traverse-path-2, traverse-path-3, traverse-path-4, fade-up, pulse-glow, rotate-reverse) and update every usage/reference of those animation names (animation, animation-name, and shorthand properties) to the new kebab-case identifiers so the selectors and keyframes remain consistent.src/components/ui/ConfirmDialog.tsx (1)
27-27:dialogRefis dead code — drop it or use it for the focus trap.
dialogRefis created and attached to the dialog<div>on line 93, but never read anywhere in the component. Either remove it, or use it as the container for a focus trap (which would also resolve the long-standing "Tab can leave the modal" feedback).♻️ Minimal removal
const previousActiveElement = useRef<HTMLElement | null>(null); - const dialogRef = useRef<HTMLDivElement>(null); const cancelButtonRef = useRef<HTMLButtonElement>(null); @@ <div - ref={dialogRef} role="dialog"Also applies to: 93-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/ConfirmDialog.tsx` at line 27, The dialogRef variable is declared but unused; either remove it or wire it up as the modal focus trap to prevent Tab from leaving the dialog. To fix, keep dialogRef (useRef<HTMLDivElement>(null)) on the dialog container and add a useEffect inside ConfirmDialog that, when isOpen is true, captures all focusable elements within dialogRef.current, focuses the first element, and installs a keydown handler that intercepts Tab/Shift+Tab to loop focus within those elements (and removes handlers on cleanup); alternatively, delete dialogRef and any ref={dialogRef} usage if you prefer the minimal removal. Ensure you reference dialogRef, ConfirmDialog, and the useEffect/keyDown handler when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@LandingPage/index.html`:
- Around line 86-94: Several external anchors (the DeepWiki/Zread links, the
GitHub link with class "github-link", the hero CTA anchors, the Product Hunt
embed anchor, and footer doc/social anchors) open with target="_blank" but lack
rel="noopener noreferrer", which risks window.opener/referrer leaks; update each
<a ... target="_blank"> (e.g., the anchors with hrefs
"https://deepwiki.com/AmintaCCCP/GithubStarsManager",
"https://zread.ai/AmintaCCCP/GithubStarsManager", the GitHub anchor with class
"github-link", the hero CTA anchors, the Product Hunt embed anchor, and the
footer anchors) to include rel="noopener noreferrer" (or append it if a rel
already exists) so every external blank-target link uses rel="noopener
noreferrer".
- Line 39: Replace the open-ended CDN import of lucide by pinning to the known
UMD version and add SRI/crossorigin attributes: change the <script
src="https://unpkg.com/lucide@latest"> usage to reference the specific version
(e.g., lucide@0.577.0 UMD bundle), compute and include the correct integrity
hash in an integrity="..." attribute and add crossorigin="anonymous"; update the
script tag that currently uses lucide@latest so it points to the pinned URL and
includes integrity and crossorigin for supply-chain safety.
In `@LandingPage/main.js`:
- Around line 1-3: The code calls document.getElementById('bg-canvas') and
immediately uses canvas.getContext('2d') which throws if the element is missing;
add a null-check after obtaining canvas and only call getContext when canvas is
truthy, then gate any canvas-dependent initialization (e.g., initMain,
createParticles, animateParticles and any code that uses ctx) behind that check
so the rest of main.js can run when bg-canvas is absent or not yet parsed;
ensure any references to ctx are similarly guarded or early-return when
canvas/ctx are unavailable.
In `@src/components/ui/ConfirmDialog.tsx`:
- Around line 33-49: The effect in ConfirmDialog's useEffect improperly clears
body overflow on initial mount, fails to restore focus if the component unmounts
while isOpen is true, and leaks the setTimeout handle; update the useEffect (the
effect that reads isOpen, previousActiveElement, cancelButtonRef) to: 1) only
clear document.body.style.overflow in the "else" branch when
previousActiveElement.current is set (or track a "hasOpened" flag) so initial
mount doesn't clobber other scroll locks; 2) on cleanup detect if
previousActiveElement.current exists and call
previousActiveElement.current?.focus() when unmounting while dialog was open so
focus is restored; and 3) store the timer id for the setTimeout used to focus
cancelButtonRef and clear it in the cleanup to avoid delayed focus after
unmount.
In `@src/components/ui/Toast.tsx`:
- Line 37: The local variable toastId (created via useId() in the Toast
component) is unused; either remove the const toastId = useId() line or wire it
so the alert container references the message paragraph for accessibility:
assign id={toastId} to the message <p> (or whatever element renders the message
text) and add aria-labelledby={toastId} to the alert/container element (the
element that has role="alert" or the root of the Toast component) so screen
readers announce the message; if you prefer the simpler change, just delete the
unused toastId declaration.
In `@src/hooks/useDialog.tsx`:
- Around line 70-85: The state updater currently calls prev.resolve?.(false)
which is an impure side-effect; instead introduce a ref (e.g.,
pendingConfirmResolveRef) to track the active resolver, call
pendingConfirmResolveRef.current?.(false) before invoking setConfirmState to
cancel the prior promise, assign pendingConfirmResolveRef.current = resolve, and
then call setConfirmState without performing any side-effects inside the updater
(retain the same fields: isOpen, title, message, confirmText, cancelText, type,
and pass resolve if needed). Ensure any cleanup (clearing
pendingConfirmResolveRef.current) happens when the dialog is settled so
prev.resolve is never invoked inside the state setter.
---
Outside diff comments:
In `@src/components/RepositoryCard.tsx`:
- Around line 295-303: The confirmMessage uses literal "\n\n" which gets
collapsed in the ConfirmDialog's <p>, so update the UI to preserve line breaks:
either modify the ConfirmDialog component (the message container element
rendered by confirm(...) / ConfirmDialog) to apply CSS "white-space: pre-wrap"
to its paragraph/container, or change the call site (the repository.analyzed_at
branch where confirmMessage is built and passed to confirm(t(...),
confirmMessage, { type: 'warning' })) to pass semantic markup (e.g., two <p>
elements or an array of React nodes) instead of a plain string; adjust
ConfirmDialog to accept ReactNode if needed and ensure the confirm(...) consumer
still works.
In `@src/components/settings/BackupPanel.tsx`:
- Around line 102-256: The outer try/finally around the restore flow (which
calls WebDAVService.listFiles() and downloadFile()) currently lacks a catch, so
network/auth errors are swallowed; modify handleRestore to catch errors from
webdavService.listFiles() and webdavService.downloadFile(latestBackup) (either
by wrapping the entire restore body in a try/catch or adding a sibling catch
after the outer try) and in that catch call toast(...) with a localized error
message and console.error, ensuring setIsRestoring(false) stays in the finally;
reference the symbols webdavService.listFiles, webdavService.downloadFile,
setIsRestoring, and toast when implementing the change.
---
Duplicate comments:
In `@src/components/ui/Toast.tsx`:
- Around line 60-62: The Toast component currently hardcodes role="alert" and
aria-live="polite", which conflicts and is semantically wrong; update the Toast
rendering to derive role, aria-live and aria-atomic from the type prop (e.g., in
the Toast component: if type === 'error' set role="alert",
aria-live="assertive", aria-atomic="true"; for 'success'/'info' set
role="status" (or no role), aria-live="polite", and aria-atomic="false" or omit
it), and remove the fixed attributes so screen readers get the correct,
non-conflicting announcement behavior.
---
Nitpick comments:
In `@LandingPage/main.js`:
- Around line 58-76: The inner particle-pair loop currently starts with j = i
which causes each particle to be compared with itself (wasting work and drawing
zero-length lines); change the inner loop initialization in the block that
iterates over particles (the for loop referencing particles[i] and particles[j])
so j starts at i + 1 instead of i, i.e. update the loop that declares "for (let
j = i; j < particles.length; j++)" to begin at "i + 1" to skip self-pairs and
avoid redundant distance calculations and drawing.
In `@LandingPage/style.css`:
- Line 19: Remove quotes from generic-or-keyword font family names in the CSS
(e.g., replace 'Inter' and 'SFMono-Regular' with Inter and SFMono-Regular) to
satisfy font-family-name-quotes; rename the keyframes to kebab-case (e.g.,
traverse-path-1, traverse-path-2, traverse-path-3, traverse-path-4, fade-up,
pulse-glow, rotate-reverse) and update every usage/reference of those animation
names (animation, animation-name, and shorthand properties) to the new
kebab-case identifiers so the selectors and keyframes remain consistent.
In `@src/components/DiscoveryView.tsx`:
- Around line 781-789: The toast severity should not be 'error' whenever
failCount > 0; update the toast call in DiscoveryView (around
successCount/failCount and the toast invocation) to use 'error' only when all
analyses failed (successCount === 0 && failCount > 0), use 'info' when there are
mixed results (successCount > 0 && failCount > 0), and keep 'success' when
failCount === 0; adjust the third argument to toast accordingly so the
notification color matches the overall outcome.
In `@src/components/ui/ConfirmDialog.tsx`:
- Line 27: The dialogRef variable is declared but unused; either remove it or
wire it up as the modal focus trap to prevent Tab from leaving the dialog. To
fix, keep dialogRef (useRef<HTMLDivElement>(null)) on the dialog container and
add a useEffect inside ConfirmDialog that, when isOpen is true, captures all
focusable elements within dialogRef.current, focuses the first element, and
installs a keydown handler that intercepts Tab/Shift+Tab to loop focus within
those elements (and removes handlers on cleanup); alternatively, delete
dialogRef and any ref={dialogRef} usage if you prefer the minimal removal.
Ensure you reference dialogRef, ConfirmDialog, and the useEffect/keyDown handler
when making the change.
In `@src/hooks/useDialog.tsx`:
- Around line 87-95: The handlers handleConfirm and handleCancel close over
confirmState.resolve making them use a stale resolver on re-entry; change them
to read the current resolver from a ref (e.g. pendingResolveRef.current created
alongside the ref-based pattern used for the confirm() logic) instead of from
confirmState.resolve, update the resolver via that ref whenever confirm() sets a
new resolver, and remove confirmState.resolve from the handlers' dependency
arrays so handleConfirm/handleCancel become stable and use
pendingResolveRef.current to call resolve(true)/resolve(false) before calling
setConfirmState to close the dialog.
- Line 1: The file imports only named React exports but uses the default React
namespace in the DialogProvider component signature
(React.FC<DialogProviderProps>); update the top import to include the default
React import (i.e., import React, { useState, useCallback, createContext,
useContext, ReactNode, useMemo } from 'react') or alternatively change the
component type to an explicit function signature (remove React.FC and type props
directly) so that the usage around DialogProvider and DialogProviderProps is
consistent with the repo and compatible with isolatedModules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 24c2828f-7305-47cc-baf5-8acb83ce0986
⛔ Files ignored due to path filters (1)
LandingPage/assets/logo.pngis excluded by!**/*.png
📒 Files selected for processing (15)
.gitignoreLandingPage/index.htmlLandingPage/main.jsLandingPage/robots.txtLandingPage/style.csssrc/components/AssetFilterManager.tsxsrc/components/CategorySidebar.tsxsrc/components/DiscoveryView.tsxsrc/components/Header.tsxsrc/components/RepositoryCard.tsxsrc/components/UpdateChecker.tsxsrc/components/settings/BackupPanel.tsxsrc/components/ui/ConfirmDialog.tsxsrc/components/ui/Toast.tsxsrc/hooks/useDialog.tsx
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/UpdateChecker.tsx
- src/components/AssetFilterManager.tsx
- src/components/CategorySidebar.tsx
| <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin> | ||
| <link href="https://fonts.googleapis.com/css2?family=Clash+Display:wght@400;500;600;700&family=Inter:wght@300;400;500;600;700&display=swap" rel="stylesheet"> | ||
| <!-- Icons --> | ||
| <script src="https://unpkg.com/lucide@latest"></script> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the latest stable version of the lucide UMD bundle on unpkg, and what's the recommended way to load it with subresource integrity?
💡 Result:
The latest stable version of the lucide package on npm (and thus unpkg) is 1.11.0, published on April 24, 2026. However, Lucide v1 removed the UMD build (except possibly for the core lucide package), providing only ESM and CJS formats instead. No UMD bundle is available at paths like https://unpkg.com/lucide@1.11.0/dist/umd/lucide.js based on unpkg listings for v1 versions. The UMD bundle is available in the last v0 versions (e.g., 0.577.0), at https://unpkg.com/lucide@0.577.0/dist/umd/lucide.min.js. Recommended way to load (for v0 UMD): Use a pinned version with SRI for security. Official docs suggest: <script src="https://unpkg.com/lucide@0.577.0/dist/umd/lucide.min.js" integrity="sha384-[hash]" crossorigin="anonymous"></script> Then call lucide.createIcons; To get the exact SRI hash, fetch the file from unpkg and compute sha384 (or use tools like SRI hash generators). Docs strongly recommend pinning to specific versions over @latest. For v1+, use ESM via <script type="module"> or bundlers.
Citations:
- 1: https://www.npmjs.com/package/lucide
- 2: https://v0.lucide.dev/guide/packages/lucide
- 3: https://lucide.dev/guide/version-1
- 4: https://app.unpkg.com/lucide@0.552.0/files/dist/umd
- 5: https://app.unpkg.com/lucide@1.7.0
Avoid lucide@latest from a public CDN.
<script src="https://unpkg.com/lucide@latest"> resolves to the most recently published version, which creates a breakage risk (a future major release could silently land in production) and a supply-chain hazard. Pin to a specific version instead and include subresource integrity.
Note: The latest v0 UMD bundle is 0.577.0; newer versions (v1+) dropped UMD in favor of ESM.
🔒 Suggested change
- <script src="https://unpkg.com/lucide@latest"></script>
+ <script src="https://unpkg.com/lucide@0.577.0/dist/umd/lucide.min.js"
+ integrity="sha384-..." crossorigin="anonymous"></script>To get the exact integrity hash, fetch the file from unpkg or use an SRI hash generator.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@LandingPage/index.html` at line 39, Replace the open-ended CDN import of
lucide by pinning to the known UMD version and add SRI/crossorigin attributes:
change the <script src="https://unpkg.com/lucide@latest"> usage to reference the
specific version (e.g., lucide@0.577.0 UMD bundle), compute and include the
correct integrity hash in an integrity="..." attribute and add
crossorigin="anonymous"; update the script tag that currently uses lucide@latest
so it points to the pinned URL and includes integrity and crossorigin for
supply-chain safety.
Comment on lines
+86
to
+94
| <a href="index.html#features">Features</a> | ||
| <a href="index.html#showcase">Showcase</a> | ||
| <a href="https://deepwiki.com/AmintaCCCP/GithubStarsManager" target="_blank"><i data-lucide="book" style="width:16px;height:16px;"></i> DeepWiki</a> | ||
| <a href="https://zread.ai/AmintaCCCP/GithubStarsManager" target="_blank"><i data-lucide="book-open" style="width:16px;height:16px;"></i> Zread</a> | ||
| <a href="https://github.com/AmintaCCCP/GithubStarsManager" target="_blank" class="github-link"> | ||
| <svg width="16" height="16" viewBox="0 0 24 24" fill="currentColor"><path d="M12 0c-6.626 0-12 5.373-12 12 0 5.302 3.438 9.8 8.207 11.387.599.111.793-.261.793-.577v-2.234c-3.338.726-4.033-1.416-4.033-1.416-.546-1.387-1.333-1.756-1.333-1.756-1.089-.745.083-.729.083-.729 1.205.084 1.839 1.237 1.839 1.237 1.07 1.834 2.807 1.304 3.492.997.107-.775.418-1.305.762-1.604-2.665-.305-5.467-1.334-5.467-5.931 0-1.311.469-2.381 1.236-3.221-.124-.303-.535-1.524.117-3.176 0 0 1.008-.322 3.301 1.23.957-.266 1.983-.399 3.003-.404 1.02.005 2.047.138 3.006.404 2.291-1.552 3.297-1.23 3.297-1.23.653 1.653.242 2.874.118 3.176.77.84 1.235 1.911 1.235 3.221 0 4.609-2.807 5.624-5.479 5.921.43.372.823 1.102.823 2.222v3.293c0 .319.192.694.801.576 4.765-1.589 8.199-6.086 8.199-11.386 0-6.627-5.373-12-12-12z"/></svg> | ||
| GitHub | ||
| </a> | ||
| </div> |
Contributor
There was a problem hiding this comment.
External target="_blank" links should set rel="noopener noreferrer".
The DeepWiki/Zread anchors (lines 88–89), the bare GitHub anchor (line 90 — only class is set, no rel), the hero CTAs (lines 126, 129), the Product Hunt embed (line 134), and the footer doc/social anchors (lines 514, 515, 519) all use target="_blank" without rel="noopener noreferrer". This leaks window.opener and Referer to third parties, and on older Safari/Edge enables reverse-tabnabbing. Apply rel="noopener noreferrer" consistently (you already do it on the in-line GitHub SVG anchor at line 90 — wait, that one is missing too).
Also applies to: 126-136, 514-520
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@LandingPage/index.html` around lines 86 - 94, Several external anchors (the
DeepWiki/Zread links, the GitHub link with class "github-link", the hero CTA
anchors, the Product Hunt embed anchor, and footer doc/social anchors) open with
target="_blank" but lack rel="noopener noreferrer", which risks
window.opener/referrer leaks; update each <a ... target="_blank"> (e.g., the
anchors with hrefs "https://deepwiki.com/AmintaCCCP/GithubStarsManager",
"https://zread.ai/AmintaCCCP/GithubStarsManager", the GitHub anchor with class
"github-link", the hero CTA anchors, the Product Hunt embed anchor, and the
footer anchors) to include rel="noopener noreferrer" (or append it if a rel
already exists) so every external blank-target link uses rel="noopener
noreferrer".
Comment on lines
+1
to
+3
| // --- Interactive Canvas Background --- | ||
| const canvas = document.getElementById('bg-canvas'); | ||
| const ctx = canvas.getContext('2d'); |
Contributor
There was a problem hiding this comment.
No null-check before calling canvas.getContext('2d').
If bg-canvas is missing from the DOM (e.g., page variant, future refactor, or if this script is loaded before the body parses), document.getElementById('bg-canvas') returns null and line 3 throws TypeError: Cannot read properties of null (reading 'getContext'), which then aborts the rest of main.js — no navbar effects, no terminal animation, no stars fetch. Guard the canvas-specific code so an isolated DOM change can't take down the whole landing page.
🛡️ Suggested fix
const canvas = document.getElementById('bg-canvas');
-const ctx = canvas.getContext('2d');
+const ctx = canvas ? canvas.getContext('2d') : null;…and gate initMain, createParticles, and animateParticles on canvas && ctx.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@LandingPage/main.js` around lines 1 - 3, The code calls
document.getElementById('bg-canvas') and immediately uses
canvas.getContext('2d') which throws if the element is missing; add a null-check
after obtaining canvas and only call getContext when canvas is truthy, then gate
any canvas-dependent initialization (e.g., initMain, createParticles,
animateParticles and any code that uses ctx) behind that check so the rest of
main.js can run when bg-canvas is absent or not yet parsed; ensure any
references to ctx are similarly guarded or early-return when canvas/ctx are
unavailable.
Comment on lines
+33
to
+49
| useEffect(() => { | ||
| if (isOpen) { | ||
| previousActiveElement.current = document.activeElement as HTMLElement; | ||
| document.body.style.overflow = 'hidden'; | ||
|
|
||
| setTimeout(() => { | ||
| cancelButtonRef.current?.focus(); | ||
| }, 0); | ||
| } else { | ||
| document.body.style.overflow = ''; | ||
| previousActiveElement.current?.focus(); | ||
| } | ||
|
|
||
| return () => { | ||
| document.body.style.overflow = ''; | ||
| }; | ||
| }, [isOpen]); |
Contributor
There was a problem hiding this comment.
Effect has two small edge cases: initial mount and unmount-while-open.
Two issues in this effect:
- On the very first render with
isOpen={false}, theelsebranch still runs and unconditionally writesdocument.body.style.overflow = ''. If anything else on the page later relies on body scroll lock and this dialog re-mounts elsewhere, that other lock can be clobbered. Guard the else branch onpreviousActiveElement.current(or use a "did open at least once" flag). - If the parent unmounts the dialog while
isOpenis stilltrue(e.g., route change during a confirmation), the cleanup only resetsoverflowand never callspreviousActiveElement.current?.focus(), so focus is lost and the original Electron focus-bug regression returns. - The
setTimeout(..., 0)for focusing the cancel button is not cleared on cleanup; ifisOpenflips quickly it can fire after unmount.
🛠️ Suggested adjustment
useEffect(() => {
+ let focusTimer: ReturnType<typeof setTimeout> | undefined;
if (isOpen) {
previousActiveElement.current = document.activeElement as HTMLElement;
document.body.style.overflow = 'hidden';
- setTimeout(() => {
+ focusTimer = setTimeout(() => {
cancelButtonRef.current?.focus();
}, 0);
- } else {
+ } else if (previousActiveElement.current) {
document.body.style.overflow = '';
previousActiveElement.current?.focus();
+ previousActiveElement.current = null;
}
return () => {
+ if (focusTimer) clearTimeout(focusTimer);
document.body.style.overflow = '';
+ // Restore focus if we unmount while still open
+ previousActiveElement.current?.focus();
};
}, [isOpen]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/ConfirmDialog.tsx` around lines 33 - 49, The effect in
ConfirmDialog's useEffect improperly clears body overflow on initial mount,
fails to restore focus if the component unmounts while isOpen is true, and leaks
the setTimeout handle; update the useEffect (the effect that reads isOpen,
previousActiveElement, cancelButtonRef) to: 1) only clear
document.body.style.overflow in the "else" branch when
previousActiveElement.current is set (or track a "hasOpened" flag) so initial
mount doesn't clobber other scroll locks; 2) on cleanup detect if
previousActiveElement.current exists and call
previousActiveElement.current?.focus() when unmounting while dialog was open so
focus is restored; and 3) store the timer id for the setTimeout used to focus
cancelButtonRef and clear it in the cleanup to avoid delayed focus after
unmount.
AmintaCCCP
added a commit
that referenced
this pull request
Apr 27, 2026
问题：window.alert() / confirm() 在 Electron 中会破坏 renderer 进 程的焦点状态。用户点击确定关闭对话框后，之前聚焦的 input 元素丢失 焦点且无法重新获得，必须 Alt+Tab 切换窗口才能恢复。 解决方案： - 创建 Toast 组件替代 alert()，使用 React Portal 渲染 - 创建 ConfirmDialog 组件替代 confirm()，返回 Promise 支持 async/await - 创建 useDialog hook 提供 toast() 和 confirm() API - 在 App.tsx 添加 DialogProvider - 替换所有设置面板中的 alert/confirm 调用 新文件： - src/components/ui/Toast.tsx - src/components/ui/ConfirmDialog.tsx - src/hooks/useDialog.tsx 修改面板：AIConfigPanel, BackendPanel, WebDAVPanel, BackupPanel, CategoryPanel Refs: #109, #113, #117 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AmintaCCCP
force-pushed
the
fix/electron-dialog-focus-bug
branch
from
51057d4 to
4a9b8b6
Compare
问题：window.alert() / confirm() 在 Electron 中会破坏 renderer 进 程的焦点状态。用户点击确定关闭对话框后，之前聚焦的 input 元素丢失 焦点且无法重新获得，必须 Alt+Tab 切换窗口才能恢复。 解决方案： - 创建 Toast 组件替代 alert()，使用 React Portal 渲染 - 创建 ConfirmDialog 组件替代 confirm()，返回 Promise 支持 async/await - 创建 useDialog hook 提供 toast() 和 confirm() API - 在 App.tsx 添加 DialogProvider - 替换所有设置面板中的 alert/confirm 调用 新文件： - src/components/ui/Toast.tsx - src/components/ui/ConfirmDialog.tsx - src/hooks/useDialog.tsx 修改面板：AIConfigPanel, BackendPanel, WebDAVPanel, BackupPanel, CategoryPanel Refs: #109, #113, #117 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AmintaCCCP
force-pushed
the
fix/electron-dialog-focus-bug
branch
from
4a9b8b6 to
e47e531
Compare
…sedown PlatformFilter's mousedown listener was closing the dropdown before the native select's click handler could open it. Changing to mouseup fixes the first-click issue and ensures clicking outside properly dismisses focus. Fixes #117
Native <select> elements have uncontrollable focus styling that persists after closing. Replace Sort by and Sort Order selects with custom dropdown components matching PlatformFilter style, eliminating yellow border issue on blur. Also remove duplicate discoveryPlatform select (already handled by PlatformFilter).
Repository page "Sort by" dropdown was using native <select> which has uncontrollable yellow focus ring. Replaced with SortByDropdown custom component using mousedown-based click-outside detection. Fixes #117
overflow-x-auto creates a new scroll context that interferes with the absolutely-positioned SortByDropdown panel. This caused the dropdown to not expand properly and added an extra scrollbar. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Toast和ConfirmDialog组件替换原生alert()/confirm()useDialoghook 提供toast()和confirm()APIProblem
window.alert()/window.confirm()在 Electron 中会破坏 renderer 进程的焦点状态。用户点击"确定"关闭对话框后，之前聚焦的 input 元素丢失焦点且无法重新获得，必须 Alt+Tab 切换窗口才能恢复。Solution
使用 React Portal + 非阻塞对话框，参考 ReadmeModal.tsx 的焦点恢复模式：
document.activeElement，对话框关闭后.focus()恢复Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements