fix: support token paste on first login#54
Conversation
📝 WalkthroughWalkthroughAdds clipboard paste handling (Ctrl/Cmd+V) to the login input; registers global search keyboard shortcuts to focus/clear search and toggle filters; and exposes the Edit menu plus disables auto-hide of the menu bar in the Electron production build configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginScreen as LoginScreen/Input
participant Clipboard as SystemClipboard
participant AppState as TokenState
User->>LoginScreen: Press Ctrl/Cmd+V
LoginScreen->>Clipboard: navigator.clipboard.readText()
Clipboard-->>LoginScreen: clipboardText
LoginScreen->>AppState: setToken(trim(clipboardText))
LoginScreen->>LoginScreen: clearErrors()
LoginScreen-->>User: token populated in input
sequenceDiagram
participant User
participant ShortcutHook as GlobalShortcutHook
participant SearchBar as SearchBar/Input
participant UI as Filters/HistoryPanel
User->>ShortcutHook: Press Ctrl/Cmd+K or /
ShortcutHook->>SearchBar: focusInput()
ShortcutHook->>UI: showHistory(true)
User->>ShortcutHook: Press Ctrl/Cmd+Shift+F
ShortcutHook->>UI: toggleFilters()
User->>ShortcutHook: Press Escape
ShortcutHook->>SearchBar: clearAndRefocus()
ShortcutHook->>UI: hideHistory()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/LoginScreen.tsx (2)
52-52: Add a feature check fornavigator.clipboardavailability.
navigator.clipboardmay be undefined in non-secure contexts (non-HTTPS) or older browsers. While thetry-catchhandles errors, accessingreadText()onundefinedthrows aTypeErrorbefore entering thetryblock's async operation.🛡️ Proposed defensive check
if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'v' && !isLoading) { + if (!navigator.clipboard?.readText) { + return; // Let default paste behavior handle it + } + e.preventDefault(); try { const text = await navigator.clipboard.readText();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/LoginScreen.tsx` at line 52, Wrap the clipboard access in a defensive feature check inside the LoginScreen component: before calling await navigator.clipboard.readText() (the line declaring const text), verify that window.navigator && navigator.clipboard && typeof navigator.clipboard.readText === 'function'; if the check fails, handle the fallback (show user-facing message or return early) otherwise proceed to call readText inside the existing try/catch so errors are still caught; update the code path that references const text accordingly (look for the const text = await navigator.clipboard.readText() usage in LoginScreen.tsx).
43-43: ReplaceonKeyPresswithonKeyDownfor reliable keyboard event handling.
onKeyPressis deprecated in web standards and doesn't reliably capture modifier key combinations (Ctrl/Cmd+key) across browsers.onKeyDownis the recommended event for handling keyboard shortcuts including your Ctrl/Cmd+V paste functionality.Update the handler at line 43 and its binding at line 122:
♻️ Changes needed
- const handleKeyPress = async (e: React.KeyboardEvent<HTMLInputElement>) => { + const handleKeyDown = async (e: React.KeyboardEvent<HTMLInputElement>) => {- onKeyPress={handleKeyPress} + onKeyDown={handleKeyDown}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/LoginScreen.tsx` at line 43, Replace the deprecated onKeyPress usage with onKeyDown by renaming the handler function handleKeyPress to handleKeyDown (or update its usage) and attach it to the input's onKeyDown prop; ensure the function signature remains (e: React.KeyboardEvent<HTMLInputElement>) => Promise<void> and that any logic checking e.ctrlKey / e.metaKey and e.key === 'v' (the Ctrl/Cmd+V paste shortcut) continues to run in the new handler so keyboard shortcuts work reliably across browsers.
🤖 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/LoginScreen.tsx`:
- Around line 50-56: In the LoginScreen key handler where you detect Ctrl/Cmd+V
and call navigator.clipboard.readText(), add e.preventDefault() immediately when
the paste hotkey is detected (before the await) to stop the browser's default
paste from also firing; keep the existing logic that awaits
navigator.clipboard.readText(), trims and calls setToken(...) and clears
setError(...), but ensure e.preventDefault() is invoked first to avoid the race
with the onChange handler.
---
Nitpick comments:
In `@src/components/LoginScreen.tsx`:
- Line 52: Wrap the clipboard access in a defensive feature check inside the
LoginScreen component: before calling await navigator.clipboard.readText() (the
line declaring const text), verify that window.navigator && navigator.clipboard
&& typeof navigator.clipboard.readText === 'function'; if the check fails,
handle the fallback (show user-facing message or return early) otherwise proceed
to call readText inside the existing try/catch so errors are still caught;
update the code path that references const text accordingly (look for the const
text = await navigator.clipboard.readText() usage in LoginScreen.tsx).
- Line 43: Replace the deprecated onKeyPress usage with onKeyDown by renaming
the handler function handleKeyPress to handleKeyDown (or update its usage) and
attach it to the input's onKeyDown prop; ensure the function signature remains
(e: React.KeyboardEvent<HTMLInputElement>) => Promise<void> and that any logic
checking e.ctrlKey / e.metaKey and e.key === 'v' (the Ctrl/Cmd+V paste shortcut)
continues to run in the new handler so keyboard shortcuts work reliably across
browsers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 002dd6e0-5856-43ed-8e77-11e594f7695e
📒 Files selected for processing (1)
src/components/LoginScreen.tsx
| if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'v' && !isLoading) { | ||
| try { | ||
| const text = await navigator.clipboard.readText(); | ||
| if (text) { | ||
| setToken(text.trim()); | ||
| setError(''); | ||
| } |
There was a problem hiding this comment.
Missing e.preventDefault() may cause double paste behavior.
When the manual clipboard read succeeds, the browser's default paste also fires (via onChange), potentially causing a race condition between the async setToken(text.trim()) and the synchronous setToken(e.target.value) from onChange. This can lead to inconsistent state or the untrimmed value winning.
Call e.preventDefault() before the async clipboard read to suppress the default paste when handling it manually.
🐛 Proposed fix
// 兼容桌面端首次登录场景下 Ctrl/Cmd + V 无法触发默认粘贴的问题
if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'v' && !isLoading) {
+ e.preventDefault();
try {
const text = await navigator.clipboard.readText();
if (text) {
setToken(text.trim());
setError('');
}
} catch (error) {
// 忽略读取剪贴板失败，让浏览器/系统默认行为继续兜底
console.warn('Clipboard read failed:', error);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/LoginScreen.tsx` around lines 50 - 56, In the LoginScreen key
handler where you detect Ctrl/Cmd+V and call navigator.clipboard.readText(), add
e.preventDefault() immediately when the paste hotkey is detected (before the
await) to stop the browser's default paste from also firing; keep the existing
logic that awaits navigator.clipboard.readText(), trims and calls setToken(...)
and clears setError(...), but ensure e.preventDefault() is invoked first to
avoid the race with the onChange handler.
|
补充 Root Cause 分析（针对 #52 评论中“快捷键失效”）： 我对比了 v0.2.0 ~ v0.2.2 的提交链，确认不是 devtools 集成导致。 触发点在 v0.2.0 期间对
本 PR 已一并修复：
这样 #52 反馈的“首次登录无法粘贴 token + 快捷键失效”两个问题都覆盖到了。 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/SearchBar.tsx (1)
513-528: Consider memoizing callbacks withuseCallbackto reduce listener churn.The inline arrow functions are recreated on every render, causing the hook's
useEffectto re-register thekeydownlistener each time. While the cleanup works correctly (no listener stacking), this results in unnecessary DOM operations.♻️ Proposed optimization
+ const focusSearch = useCallback(() => { + searchInputRef.current?.focus(); + if (!searchQuery && searchHistory.length > 0) { + setShowSearchHistory(true); + } + }, [searchQuery, searchHistory.length]); + + const clearSearchAndFocus = useCallback(() => { + handleClearSearch(); + searchInputRef.current?.focus(); + }, [handleClearSearch]); + + const toggleFilters = useCallback(() => { + setShowFilters(prev => !prev); + }, []); + // 全局快捷键支持（Ctrl/Cmd+K、Ctrl/Cmd+Shift+F、/、Escape） useSearchShortcuts({ - onFocusSearch: () => { - searchInputRef.current?.focus(); - if (!searchQuery && searchHistory.length > 0) { - setShowSearchHistory(true); - } - }, - onClearSearch: () => { - handleClearSearch(); - searchInputRef.current?.focus(); - }, - onToggleFilters: () => { - setShowFilters(prev => !prev); - }, + onFocusSearch: focusSearch, + onClearSearch: clearSearchAndFocus, + onToggleFilters: toggleFilters, });Note: You'll also need to wrap
handleClearSearchinuseCallbackfor this to be fully effective.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SearchBar.tsx` around lines 513 - 528, The inline callbacks passed to useSearchShortcuts (onFocusSearch, onClearSearch, onToggleFilters) are recreated each render and cause the hook to re-register its keydown listener; wrap these handlers in React.useCallback (e.g., memoize onFocusSearch, onClearSearch, onToggleFilters) and update useSearchShortcuts to receive the memoized functions, and also memoize handleClearSearch itself so onClearSearch remains stable; ensure dependencies include searchInputRef, searchQuery, searchHistory, setShowSearchHistory, handleClearSearch, setShowFilters as appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/SearchBar.tsx`:
- Around line 513-528: The inline callbacks passed to useSearchShortcuts
(onFocusSearch, onClearSearch, onToggleFilters) are recreated each render and
cause the hook to re-register its keydown listener; wrap these handlers in
React.useCallback (e.g., memoize onFocusSearch, onClearSearch, onToggleFilters)
and update useSearchShortcuts to receive the memoized functions, and also
memoize handleClearSearch itself so onClearSearch remains stable; ensure
dependencies include searchInputRef, searchQuery, searchHistory,
setShowSearchHistory, handleClearSearch, setShowFilters as appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99bfb0ca-301d-4373-b4c7-1612a2171160
📒 Files selected for processing (1)
src/components/SearchBar.tsx
|
收到最新反馈后做了二次 root cause 定位： 这次不是前端输入框逻辑，而是 Electron 桌面构建脚本生成的应用菜单配置 导致。 在 v0.2.x 的桌面构建里：
这会导致在部分平台/打包场景下，系统级编辑快捷键（Ctrl/Cmd + C/V）无法正确工作。 已在本 PR 追加修复：
现在 #52 对应的三类问题都覆盖：
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-desktop.yml:
- Line 212: Revert the unrelated UX change: remove or restore the
autoHideMenuBar configuration change (the autoHideMenuBar: false setting) so
menu visibility is not tied to the clipboard shortcut fix, and keep the actual
paste shortcut restoration code (the block that re-enables native clipboard
accelerators via Menu.setApplicationMenu / the paste role) intact; ensure the
paste-fix remains limited to the code that manipulates Menu.setApplicationMenu
or accelerator handling and does not modify the autoHideMenuBar UI setting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7f19c87-90b2-4ab1-b87c-8988b321f1d8
📒 Files selected for processing (1)
.github/workflows/build-desktop.yml
| ' titleBarStyle: \\'default\\', // 使用默认标题栏，避免重叠问题\\n' + | ||
| ' show: false,\\n' + | ||
| ' autoHideMenuBar: true, // 隐藏菜单栏\\n' + | ||
| ' autoHideMenuBar: false, // 显示菜单栏，确保编辑快捷键行为一致\\n' + |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Electron, do menu-item accelerators from Menu.setApplicationMenu()—especially the built-in pasterole—still work whenBrowserWindow.autoHideMenuBaristrue on Windows and Linux?
💡 Result:
Yes. On Windows and Linux, setting BrowserWindow.autoHideMenuBar: true only auto-hides the visible menu bar (it reappears with Alt) and does not disable the application menu’s registered accelerators. (electronjs.org)
Electron’s own MenuItem docs note that the “accelerator works when hidden” switch is macOS-only because accelerators always work when items are hidden on Windows and Linux—so menu accelerators (including role-based ones) keep firing even if the menu bar isn’t visible. (electronjs.org)
That includes the built-in paste role (as long as you’ve set an application menu via Menu.setApplicationMenu() and you haven’t disabled registration via registerAccelerator: false). (electronjs.org)
Citations:
- 1: https://www.electronjs.org/docs/api/browser-window
- 2: https://www.electronjs.org/fr/docs/latest/api/menu-item
- 3: https://www.electronjs.org/docs/latest/api/menu-item?utm_source=openai
Decouple the menu-bar visibility change from the paste-fix.
On Windows and Linux, menu-item accelerators from Menu.setApplicationMenu()—including the built-in paste role—remain active even when autoHideMenuBar: true; the menu bar only hides visually but does not disable accelerators. Lines 287-297 alone restore native clipboard shortcuts; line 212 is an unrelated UX change and should be reconsidered.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-desktop.yml at line 212, Revert the unrelated UX
change: remove or restore the autoHideMenuBar configuration change (the
autoHideMenuBar: false setting) so menu visibility is not tied to the clipboard
shortcut fix, and keep the actual paste shortcut restoration code (the block
that re-enables native clipboard accelerators via Menu.setApplicationMenu / the
paste role) intact; ensure the paste-fix remains limited to the code that
manipulates Menu.setApplicationMenu or accelerator handling and does not modify
the autoHideMenuBar UI setting.
1 participant
Summary\nFixes the issue where users on v2.0-v2.2 cannot paste GitHub token during first login.\n\n## Changes\n- Add keyboard fallback for Ctrl/Cmd + V in login token input\n- Read clipboard text via and populate token field\n- Keep default behavior unchanged for normal typing and Enter submit\n- No functional logic changes outside login input handling\n\n## Testing\n-
vite v5.4.8 building for production...
transforming...
✓ 1810 modules transformed.
rendering chunks...
computing gzip size...
dist/index.html 1.11 kB │ gzip: 0.65 kB
dist/assets/index-DW9FpI6u.css 39.40 kB │ gzip: 6.68 kB
dist/assets/index-Djqhd4g9.js 387.01 kB │ gzip: 111.39 kB
✓ built in 2.63s passes\n- Verified no type/build errors\n\nFixes #52
Summary by CodeRabbit
New Features
Bug Fixes