feat(desktop): enhance browser bookmarks#55
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughデスクトップ版でブックマークを階層ツリー化し、フォルダ編集・DnD・インポート/エクスポートとElectronベースのファイルI/O（openTextFile/saveTextFile）を追加しました。webview相互作用の多重ロック制御も導入されています。 Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as BrowserOverflowMenu
participant TRPC as electronTrpc
participant Electron
participant Store as BookmarkStore
User->>UI: Click "Import Bookmarks"
UI->>TRPC: openTextFile(request filters)
TRPC->>Electron: showOpenDialog()
Electron-->>TRPC: path (or cancel)
TRPC-->>UI: { path, content } or null
UI->>Store: importBrowserBookmarksFromHtml(content)
Store-->>UI: { bookmarksAdded, foldersAdded }
UI-->>User: show success/error toast
sequenceDiagram
actor User
participant Bar as BookmarkBar
participant Dnd as DndContext
participant Lock as usePersistentWebview
participant Store as BookmarkStore
User->>Bar: Drag bookmark
Bar->>Dnd: start drag
Dnd->>Lock: setPersistentWebviewInteractionLock("drag", true)
User->>Bar: Drop into folder
Dnd->>Bar: onDragEnd(active, over)
Bar->>Store: moveNode(activeId, overId)
Store-->>Bar: updated tree
Bar->>Lock: setPersistentWebviewInteractionLock("drag", false)
Bar-->>User: update UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 3
🧹 Nitpick comments (12)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkBar/components/BookmarkBarItem/components/EditBookmarkDialog/EditBookmarkDialog.tsx (1)
14-17:FolderOption型のエクスポートを検討してください
FolderOptionインターフェースはこのファイル内でのみ定義されていますが、親コンポーネント（BookmarkBarItem）でも同じ構造を使用しています。型の再利用性を高めるために、この型をエクスポートすることを検討してください。♻️ 提案する修正
-interface FolderOption { +export interface FolderOption { id: string; label: string; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkBar/components/BookmarkBarItem/components/EditBookmarkDialog/EditBookmarkDialog.tsx` around lines 14 - 17, Export the FolderOption interface from EditBookmarkDialog (add "export" to the interface declaration for FolderOption) so it can be reused; then update the parent component BookmarkBarItem to import and use this exported FolderOption type instead of duplicating the structure, ensuring both EditBookmarkDialog and BookmarkBarItem reference the same FolderOption symbol.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserToolbar/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx (2)
117-132: エクスポート処理のエラーハンドリングを追加してください
saveTextFileMutation.mutateAsyncが失敗した場合のエラーハンドリングがありません。ファイル書き込みエラー時にユーザーに通知できるようにしてください。♻️ 提案する修正
const handleExportBookmarks = async () => { - const content = exportBrowserBookmarksToHtml(bookmarks); - const saved = await saveTextFileMutation.mutateAsync({ - title: "Export Bookmarks", - defaultPath: "bookmarks.html", - buttonLabel: "Export", - filters: [{ name: "Bookmarks HTML", extensions: ["html"] }], - content, - }); - - if (!saved) { - return; - } - - toast.success("Bookmarks exported"); + try { + const content = exportBrowserBookmarksToHtml(bookmarks); + const saved = await saveTextFileMutation.mutateAsync({ + title: "Export Bookmarks", + defaultPath: "bookmarks.html", + buttonLabel: "Export", + filters: [{ name: "Bookmarks HTML", extensions: ["html"] }], + content, + }); + + if (!saved) { + return; + } + + toast.success("Bookmarks exported"); + } catch (error) { + toast.error("Failed to export bookmarks"); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserToolbar/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx` around lines 117 - 132, The export handler handleExportBookmarks should catch errors from saveTextFileMutation.mutateAsync (and any thrown while generating content via exportBrowserBookmarksToHtml) and notify the user on failure; wrap the mutateAsync call in try/catch, call toast.error with a clear message on failure (and optionally include error.message), and log the error (e.g., console.error or processLogger) before returning so the success toast is only shown on true success.
93-115: インポート処理のエラーハンドリングを追加してください
openTextFileMutation.mutateAsyncやimportBrowserBookmarksFromHtmlが失敗した場合、例外がキャッチされずコンソールにのみエラーが出力されます。ユーザーに適切なフィードバックを提供するために、try-catch を追加してください。♻️ 提案する修正
const handleImportBookmarks = async () => { + try { const file = await openTextFileMutation.mutateAsync({ title: "Import Bookmarks", buttonLabel: "Import", filters: [{ name: "Bookmarks HTML", extensions: ["html", "htm"] }], }); if (!file) { return; } const importedNodes = importBrowserBookmarksFromHtml(file.content); const result = importBookmarks(importedNodes); if (result.bookmarksAdded === 0 && result.foldersAdded === 0) { toast.error("No bookmarks were imported"); return; } toast.success( `Imported ${result.bookmarksAdded} bookmarks and ${result.foldersAdded} folders`, ); + } catch (error) { + toast.error("Failed to import bookmarks"); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserToolbar/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx` around lines 93 - 115, Wrap the entire body of handleImportBookmarks in a try-catch so any exceptions from openTextFileMutation.mutateAsync, importBrowserBookmarksFromHtml, or importBookmarks are caught; in the catch block call toast.error with a user-friendly message (include error.message for debugging) and optionally log the error to console/diagnostics, then return to avoid further processing — update references in handleImportBookmarks, openTextFileMutation.mutateAsync, importBrowserBookmarksFromHtml, importBookmarks, and toast.error accordingly.apps/desktop/src/lib/trpc/routers/external/index.ts (2)
263-271: ファイル書き込み時のエラーハンドリングを追加してください
writeFileもディスク容量不足や書き込み権限がない場合に失敗する可能性があります。openTextFileと同様に、明示的なエラーハンドリングを追加することを推奨します。♻️ 提案する修正
if (result.canceled || !result.filePath) { return null; } - await writeFile(result.filePath, input.content, "utf-8"); - return { - path: result.filePath, - }; + try { + await writeFile(result.filePath, input.content, "utf-8"); + return { + path: result.filePath, + }; + } catch (error) { + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: error instanceof Error ? error.message : "Failed to write file", + }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/external/index.ts` around lines 263 - 271, The resolver currently calls writeFile without error handling (see writeFile and the surrounding code handling result.canceled/result.filePath); wrap the await writeFile(...) in a try/catch, and on failure either throw a descriptive TRPCError (including the original error message) or return a null/structured error consistent with openTextFile's behavior so callers get meaningful feedback; ensure the catch includes context (e.g., "failed to write file" + file path) and preserves the original error for logging/inspection.
229-239: ファイル読み込み時のエラーハンドリングを追加してください
readFileはファイルが存在しない、読み取り権限がない、エンコーディングエラーなど様々な理由で失敗する可能性があります。現在の実装では例外がそのまま伝播しますが、ユーザーフレンドリーなエラーメッセージを返すために明示的なエラーハンドリングを検討してください。♻️ 提案する修正
const filePath = result.filePaths[0]; if (!filePath) { return null; } - const content = await readFile(filePath, "utf-8"); - return { - path: filePath, - content, - }; + try { + const content = await readFile(filePath, "utf-8"); + return { + path: filePath, + content, + }; + } catch (error) { + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: error instanceof Error ? error.message : "Failed to read file", + }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/external/index.ts` around lines 229 - 239, Wrap the await readFile(...) call in a try/catch and convert filesystem errors into a user-friendly error response: catch exceptions thrown by readFile (when reading result.filePaths[0]) and either throw a trpc-compatible error (e.g., new TRPCError with an appropriate code and a concise message) or return a structured object containing a clear message and non-sensitive detail; also log or attach the original error (as cause) for debugging. Ensure you keep the existing null check for filePath and only perform the try/catch around the readFile and subsequent return of { path: filePath, content }.apps/desktop/src/renderer/stores/browser-bookmarks-html.ts (2)
41-45:parseTimestampのDate.now()フォールバックが重複呼び出しになる可能性があります。
add_date属性がない場合、毎回Date.now()が呼ばれますが、大量のブックマークをインポートする際に微妙に異なるタイムスタンプが設定されます。意図した動作であれば問題ありませんが、一貫性が必要な場合はインポート開始時に一度だけ取得することを検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/browser-bookmarks-html.ts` around lines 41 - 45, The parseTimestamp function currently calls Date.now() each time it needs a fallback, causing slightly different timestamps across many imports; change parseTimestamp to accept an optional fallback timestamp parameter (e.g., parseTimestamp(value: string | null, fallbackMs?: number)) and use that when value is missing or invalid, and ensure the import routine that iterates bookmarks captures a single fallback timestamp once at start (e.g., const fallbackMs = Date.now()) and passes it into parseTimestamp for each call so all fallbacks are identical.
7-13: 手動HTMLエスケープは、このユースケースでは許容範囲です。静的解析ツールが手動HTML sanitizationを警告していますが、このコンテキストでは以下の理由で問題ありません：
escapeHtmlはエクスポート専用で、内部データをNetscape形式のHTMLファイルに出力するために使用- 出力はファイルに保存されるため、DOMインジェクションのリスクはない
- エスケープ順序（
&が最初）は正しいただし、将来的にファイルがブラウザで直接開かれる可能性を考慮すると、シングルクォートのエスケープ追加を検討してください。
♻️ シングルクォートエスケープの追加（オプション）
function escapeHtml(value: string): string { return value .replaceAll("&", "&") .replaceAll("<", "<") .replaceAll(">", ">") - .replaceAll('"', """); + .replaceAll('"', """) + .replaceAll("'", "'"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/browser-bookmarks-html.ts` around lines 7 - 13, The static analyzer warning can be addressed by extending the existing escapeHtml function to also escape single quotes; update the escapeHtml function to include a .replaceAll("'", "'") step (keeping the existing order that replaces "&" first) so values output to the Netscape HTML file also have single quotes escaped, while leaving the rest of the logic in escapeHtml unchanged.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkBar/components/BookmarkFolderItem/BookmarkFolderItem.tsx (1)
192-206: インタラクションロックの管理が2つのuseEffectに分かれています。Lines 192-199と201-206で同じ
menuLockIdを管理していますが、責務が明確に分離されています：
- 最初のuseEffectはアンマウント時のクリーンアップ（タイマー含む）
- 2番目は
isMenuOpen状態との同期ただし、201-206のクリーンアップ（203-205）が毎回
falseを設定するため、メニューが開いている状態でコンポーネントが再レンダリングされると、一瞬ロックが解除される可能性があります。♻️ クリーンアップの統合（オプション）
useEffect(() => { setPersistentWebviewInteractionLock(menuLockId, isMenuOpen); - return () => { - setPersistentWebviewInteractionLock(menuLockId, false); - }; }, [isMenuOpen, menuLockId]);アンマウント時のクリーンアップは最初のuseEffectで既に処理されているため、2番目のクリーンアップは不要な可能性があります。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkBar/components/BookmarkFolderItem/BookmarkFolderItem.tsx` around lines 192 - 206, The two useEffect hooks both manage menuLockId causing a transient unlock during re-renders; remove the cleanup that sets setPersistentWebviewInteractionLock(menuLockId, false) from the second useEffect (the one depending on [isMenuOpen, menuLockId]) so it only syncs the lock to isMenuOpen, and keep the unmount/timer cleanup (pendingOpenTimerRef and the false reset) in the first useEffect; alternatively merge the logic into a single useEffect that clears pendingOpenTimerRef on unmount and syncs setPersistentWebviewInteractionLock(menuLockId, isMenuOpen) during updates to avoid momentary unlocks.apps/desktop/src/renderer/stores/browser-bookmarks.ts (3)
700-709: バージョン1から3へのマイグレーションについて。
version: 3に直接ジャンプしていますが、migrate関数は任意の古いデータをsanitizeLegacyNodesで処理するため、バージョン2のデータも適切に処理されます。ただし、
persistedStateがisRecordでない場合に空のbookmarks配列を返すのは正しいですが、バージョン情報のロギングがないため、マイグレーション問題のデバッグが困難になる可能性があります。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/browser-bookmarks.ts` around lines 700 - 709, The migration currently jumps to version: 3 and sanitizes legacy data via migrate(persistedState) calling sanitizeLegacyNodes, but it lacks logging to help debug migration issues; update the migrate function (referencing migrate, isRecord, sanitizeLegacyNodes, and bookmarks) to log the incoming persistedState version when present and to emit a warning/error (using the module's logger or console.warn) when persistedState is not an object and you return { bookmarks: [] }, so you can trace which version/data caused the fallback and confirm migrations ran for version 2→3 cases.
173-191:findBookmarkParentFolderIdの戻り値の曖昧さについて。この関数は以下の2つのケースで
nullを返します：
- ブックマークがルートレベルにある場合
- ブックマークが見つからない場合
呼び出し側でこれらのケースを区別できない可能性があります。ただし、現在の使用箇所（
BookmarkBarItem）では、対象ブックマークが必ず存在するコンテキストで呼ばれるため、実用上は問題ありません。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/browser-bookmarks.ts` around lines 173 - 191, findBookmarkParentFolderId currently returns null both when a bookmark is at root and when it's not found, causing ambiguity; change its signature to return a discriminated result like { found: boolean; parentFolderId: string | null } (or similar) so callers can distinguish "found-but-root" (found: true, parentFolderId: null) vs "not found" (found: false). Update the implementation (function findBookmarkParentFolderId) to return { found: true, parentFolderId: ... } on match and { found: false, parentFolderId: null } at the end, and update all callers (e.g., BookmarkBarItem) to check the found flag before using parentFolderId.
560-578: ブックマーク更新時の「削除→再挿入」パターンについて。
updateBookmarkは対象ブックマークを一度ツリーから削除し、指定されたfolderIdに再挿入します。folderIdがfalsyの場合はルートに追加されます（line 574）。この設計は明確ですが、ユーザーが意図せずブックマークをルートに移動してしまう可能性があります。
folderIdが明示的にnullの場合のみルート移動とし、undefinedの場合は元の位置を維持する方が安全かもしれません。♻️ folderId未指定時は元の位置を維持（推奨）
set((state) => { + const currentFolderId = findBookmarkParentFolderId(state.bookmarks, bookmarkId); const removed = removeNodeFromTree(state.bookmarks, bookmarkId); let nextNodes = removed.nodes; - if (bookmark.folderId) { + const targetFolderId = bookmark.folderId === undefined ? currentFolderId : bookmark.folderId; + if (targetFolderId) { const inserted = insertNodeIntoFolder( nextNodes, updatedBookmark, - bookmark.folderId, + targetFolderId, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/browser-bookmarks.ts` around lines 560 - 578, 現在の実装はbookmark.folderIdがfalsyだと常にルートに移動してしまうため、bookmark.folderIdがundefinedの場合は元の位置を維持し、nullのみルート移動とするよう修正してください: removeNodeFromTreeの戻り値（削除したノードの元の親情報）から元のフォルダIDを取得し、bookmark.folderId === undefinedならその元フォルダIDを使ってinsertNodeIntoFolderに再挿入、bookmark.folderId === nullならルートへ、それ以外はbookmark.folderIdを使って再挿入するロジックに変更してください（参照シンボル: removeNodeFromTree, insertNodeIntoFolder, bookmark.folderId, updatedBookmark）。apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkFolderDialog/BookmarkFolderDialog.tsx (1)
71-77: フォーム送信時のバリデーションが不足しています。空のタイトルでもそのまま
onSaveに渡されます。ストア側で"Untitled Folder"にフォールバックしていますが、UIレベルでの最小限のバリデーション（空白のみのタイトル禁止など）を追加することを検討してください。♻️ 空タイトルの防止（オプション）
<form className="space-y-4" onSubmit={(event) => { event.preventDefault(); - onSave({ title, iconKey, color }); + onSave({ title: title.trim() || "Untitled Folder", iconKey, color }); }} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkFolderDialog/BookmarkFolderDialog.tsx` around lines 71 - 77, The form's onSubmit in BookmarkFolderDialog currently calls onSave with title even if it's empty/whitespace; update the onSubmit handler in the BookmarkFolderDialog component to trim title and validate it (e.g., const cleaned = title.trim()) and if cleaned is empty, prevent calling onSave and set a local validation state (e.g., titleError or isTitleInvalid) so the UI can show an inline error and keep the dialog open; only call onSave({ title: cleaned, iconKey, color }) when the title passes validation. Ensure the new validation state is declared in the component and used to render a brief error message near the title input.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkBar/components/BookmarkFolderItem/BookmarkFolderItem.tsx`:
- Around line 121-144: Folder folder nodes are rendered as plain <div> in
BookmarkFolderItem/FolderTreeSection so they aren't draggable; wrap the folder
node rendering with the same sortable wiring used by BookmarkBarItem: call
useSortable (or the sortable HOC) for the folder node, apply the returned
attributes/listeners/style to the folder container and expose a drag handle UI,
and pass the sortable state into FolderTreeSection props so onReorder calls
(which should invoke the store function reorderFolderChildren) receive the
correct source/target indexes and folderId; update
FolderTreeSection/BookmarkFolderItem to accept and forward a sortable prop for
folder nodes and ensure onReorder triggers reorderFolderChildren with the
folderId and new children order.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkFolderDialog/BookmarkFolderDialog.tsx`:
- Around line 58-63: The current useEffect in the BookmarkFolderDialog component
resets state whenever initialTitle/initialIconKey/initialColor change while open
is true; change it so the state is initialized only when the dialog opens (open
transitions false→true). Implement this by tracking previous open (e.g., a
useRef or usePrevious) and in the effect run setTitle(initialTitle),
setIconKey(initialIconKey), setColor(initialColor ?? "#64748b") only when open
is true and prevOpen was false; keep the effect's dependency array minimal
(include open and the initial values if you prefer but gate execution on the
rising edge) and reference the existing identifiers: BookmarkFolderDialog, open,
initialTitle, initialIconKey, initialColor, setTitle, setIconKey, setColor.
In `@apps/desktop/src/renderer/stores/browser-bookmarks.ts`:
- Around line 387-439: cloneImportedNodes currently overwrites original
createdAt values with Date.now(); update it to preserve any existing createdAt
from input nodes for both bookmarks and folders: when handling
isBrowserBookmark(node) use node.createdAt if present (fallback to Date.now()),
and when creating folder nodes use nested child node.createdAt if node.createdAt
exists (fallback to Date.now()); keep existing id generation, title
normalization, and children assignment unchanged so imported add_date timestamps
from parseBookmarkList are retained.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/external/index.ts`:
- Around line 263-271: The resolver currently calls writeFile without error
handling (see writeFile and the surrounding code handling
result.canceled/result.filePath); wrap the await writeFile(...) in a try/catch,
and on failure either throw a descriptive TRPCError (including the original
error message) or return a null/structured error consistent with openTextFile's
behavior so callers get meaningful feedback; ensure the catch includes context
(e.g., "failed to write file" + file path) and preserves the original error for
logging/inspection.
- Around line 229-239: Wrap the await readFile(...) call in a try/catch and
convert filesystem errors into a user-friendly error response: catch exceptions
thrown by readFile (when reading result.filePaths[0]) and either throw a
trpc-compatible error (e.g., new TRPCError with an appropriate code and a
concise message) or return a structured object containing a clear message and
non-sensitive detail; also log or attach the original error (as cause) for
debugging. Ensure you keep the existing null check for filePath and only perform
the try/catch around the readFile and subsequent return of { path: filePath,
content }.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkBar/components/BookmarkBarItem/components/EditBookmarkDialog/EditBookmarkDialog.tsx`:
- Around line 14-17: Export the FolderOption interface from EditBookmarkDialog
(add "export" to the interface declaration for FolderOption) so it can be
reused; then update the parent component BookmarkBarItem to import and use this
exported FolderOption type instead of duplicating the structure, ensuring both
EditBookmarkDialog and BookmarkBarItem reference the same FolderOption symbol.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkBar/components/BookmarkFolderItem/BookmarkFolderItem.tsx`:
- Around line 192-206: The two useEffect hooks both manage menuLockId causing a
transient unlock during re-renders; remove the cleanup that sets
setPersistentWebviewInteractionLock(menuLockId, false) from the second useEffect
(the one depending on [isMenuOpen, menuLockId]) so it only syncs the lock to
isMenuOpen, and keep the unmount/timer cleanup (pendingOpenTimerRef and the
false reset) in the first useEffect; alternatively merge the logic into a single
useEffect that clears pendingOpenTimerRef on unmount and syncs
setPersistentWebviewInteractionLock(menuLockId, isMenuOpen) during updates to
avoid momentary unlocks.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkFolderDialog/BookmarkFolderDialog.tsx`:
- Around line 71-77: The form's onSubmit in BookmarkFolderDialog currently calls
onSave with title even if it's empty/whitespace; update the onSubmit handler in
the BookmarkFolderDialog component to trim title and validate it (e.g., const
cleaned = title.trim()) and if cleaned is empty, prevent calling onSave and set
a local validation state (e.g., titleError or isTitleInvalid) so the UI can show
an inline error and keep the dialog open; only call onSave({ title: cleaned,
iconKey, color }) when the title passes validation. Ensure the new validation
state is declared in the component and used to render a brief error message near
the title input.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserToolbar/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx`:
- Around line 117-132: The export handler handleExportBookmarks should catch
errors from saveTextFileMutation.mutateAsync (and any thrown while generating
content via exportBrowserBookmarksToHtml) and notify the user on failure; wrap
the mutateAsync call in try/catch, call toast.error with a clear message on
failure (and optionally include error.message), and log the error (e.g.,
console.error or processLogger) before returning so the success toast is only
shown on true success.
- Around line 93-115: Wrap the entire body of handleImportBookmarks in a
try-catch so any exceptions from openTextFileMutation.mutateAsync,
importBrowserBookmarksFromHtml, or importBookmarks are caught; in the catch
block call toast.error with a user-friendly message (include error.message for
debugging) and optionally log the error to console/diagnostics, then return to
avoid further processing — update references in handleImportBookmarks,
openTextFileMutation.mutateAsync, importBrowserBookmarksFromHtml,
importBookmarks, and toast.error accordingly.
In `@apps/desktop/src/renderer/stores/browser-bookmarks-html.ts`:
- Around line 41-45: The parseTimestamp function currently calls Date.now() each
time it needs a fallback, causing slightly different timestamps across many
imports; change parseTimestamp to accept an optional fallback timestamp
parameter (e.g., parseTimestamp(value: string | null, fallbackMs?: number)) and
use that when value is missing or invalid, and ensure the import routine that
iterates bookmarks captures a single fallback timestamp once at start (e.g.,
const fallbackMs = Date.now()) and passes it into parseTimestamp for each call
so all fallbacks are identical.
- Around line 7-13: The static analyzer warning can be addressed by extending
the existing escapeHtml function to also escape single quotes; update the
escapeHtml function to include a .replaceAll("'", "'") step (keeping the
existing order that replaces "&" first) so values output to the Netscape HTML
file also have single quotes escaped, while leaving the rest of the logic in
escapeHtml unchanged.
In `@apps/desktop/src/renderer/stores/browser-bookmarks.ts`:
- Around line 700-709: The migration currently jumps to version: 3 and sanitizes
legacy data via migrate(persistedState) calling sanitizeLegacyNodes, but it
lacks logging to help debug migration issues; update the migrate function
(referencing migrate, isRecord, sanitizeLegacyNodes, and bookmarks) to log the
incoming persistedState version when present and to emit a warning/error (using
the module's logger or console.warn) when persistedState is not an object and
you return { bookmarks: [] }, so you can trace which version/data caused the
fallback and confirm migrations ran for version 2→3 cases.
- Around line 173-191: findBookmarkParentFolderId currently returns null both
when a bookmark is at root and when it's not found, causing ambiguity; change
its signature to return a discriminated result like { found: boolean;
parentFolderId: string | null } (or similar) so callers can distinguish
"found-but-root" (found: true, parentFolderId: null) vs "not found" (found:
false). Update the implementation (function findBookmarkParentFolderId) to
return { found: true, parentFolderId: ... } on match and { found: false,
parentFolderId: null } at the end, and update all callers (e.g.,
BookmarkBarItem) to check the found flag before using parentFolderId.
- Around line 560-578:
現在の実装はbookmark.folderIdがfalsyだと常にルートに移動してしまうため、bookmark.folderIdがundefinedの場合は元の位置を維持し、nullのみルート移動とするよう修正してください:
removeNodeFromTreeの戻り値（削除したノードの元の親情報）から元のフォルダIDを取得し、bookmark.folderId ===
undefinedならその元フォルダIDを使ってinsertNodeIntoFolderに再挿入、bookmark.folderId ===
nullならルートへ、それ以外はbookmark.folderIdを使って再挿入するロジックに変更してください（参照シンボル:
removeNodeFromTree, insertNodeIntoFolder, bookmark.folderId, updatedBookmark）。
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1b2a6fe8-0fcc-4414-a8a6-d1ec41f1a7ca
📒 Files selected for processing (16)
apps/desktop/src/lib/trpc/routers/external/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/BrowserPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkBar/BookmarkBar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkBar/components/BookmarkBarItem/BookmarkBarItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkBar/components/BookmarkBarItem/components/EditBookmarkDialog/EditBookmarkDialog.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkBar/components/BookmarkFolderItem/BookmarkFolderItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkBar/components/BookmarkFolderItem/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkFolderDialog/BookmarkFolderDialog.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkFolderDialog/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserToolbar/components/BrowserOverflowMenu/BrowserOverflowMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/hooks/usePersistentWebview/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/stores/browser-bookmark-folder-icons.tsxapps/desktop/src/renderer/stores/browser-bookmarks-html.tsapps/desktop/src/renderer/stores/browser-bookmarks.ts
Comment on lines
+58
to
+63
| useEffect(() => { | ||
| if (!open) return; | ||
| setTitle(initialTitle); | ||
| setIconKey(initialIconKey); | ||
| setColor(initialColor ?? "#64748b"); | ||
| }, [initialColor, initialIconKey, initialTitle, open]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 呼び出し元でinitial*プロパティがダイアログ表示中に変更されるかを確認
rg -n "BookmarkFolderDialog" --type=tsx -A 10 | head -100Repository: MocA-Love/superset
Length of output: 89
🏁 Script executed:
fd "BookmarkFolderDialog.tsx" --type fRepository: MocA-Love/superset
Length of output: 226
🏁 Script executed:
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkFolderDialog/BookmarkFolderDialog.tsx | head -80Repository: MocA-Love/superset
Length of output: 2806
🏁 Script executed:
wc -l apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkFolderDialog/BookmarkFolderDialog.tsxRepository: MocA-Love/superset
Length of output: 230
🏁 Script executed:
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkFolderDialog/BookmarkFolderDialog.tsx | tail -110Repository: MocA-Love/superset
Length of output: 4120
🏁 Script executed:
rg "BookmarkFolderDialog" --type ts -A 5 -B 5 | head -150Repository: MocA-Love/superset
Length of output: 15829
🏁 Script executed:
rg "BookmarkFolderItem" --type ts -A 20 -B 5 | grep -A 20 "isEditOpen\|setIsEditOpen"Repository: MocA-Love/superset
Length of output: 4455
🏁 Script executed:
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkBar/components/BookmarkFolderItem/BookmarkFolderItem.tsx | grep -A 30 "BookmarkFolderDialog"Repository: MocA-Love/superset
Length of output: 1814
🏁 Script executed:
wc -l apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkBar/components/BookmarkFolderItem/BookmarkFolderItem.tsxRepository: MocA-Love/superset
Length of output: 249
useEffectの依存配列の改善：ダイアログ開く時のみ状態を同期するように変更してください。
現在のコードは、initialTitle、initialIconKey、initialColorが変更されるとダイアログが開いている間でも状態がリセットされます。実際の使用パターンではダイアログがすぐに閉じるため問題になっていませんが、親コンポーネントの再レンダリングでfolder参照が変わると意図しないリセットが起こる可能性があります。
ダイアログを開く際だけ状態を初期化するよう改善することで、より堅牢なコードになります。例えば、openがfalseからtrueに変わった時のみリセットする実装が推奨されます。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BookmarkFolderDialog/BookmarkFolderDialog.tsx`
around lines 58 - 63, The current useEffect in the BookmarkFolderDialog
component resets state whenever initialTitle/initialIconKey/initialColor change
while open is true; change it so the state is initialized only when the dialog
opens (open transitions false→true). Implement this by tracking previous open
(e.g., a useRef or usePrevious) and in the effect run setTitle(initialTitle),
setIconKey(initialIconKey), setColor(initialColor ?? "#64748b") only when open
is true and prevOpen was false; keep the effect's dependency array minimal
(include open and the initial values if you prefer but gate execution on the
rising edge) and reference the existing identifiers: BookmarkFolderDialog, open,
initialTitle, initialIconKey, initialColor, setTitle, setIconKey, setColor.
Merged
MocA-Love
added a commit
that referenced
this pull request
Apr 14, 2026
2 tasks
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.
close #52
Summary
Validation
Notes
Summary by CodeRabbit
リリースノート
New Features
Bug Fixes