feat(ui): implement V2 application workspace - #650
Merged
Conversation
📝 WalkthroughWalkthroughV2 UIのルート、アプリシェル、検索・ソース・詳細・管理・設定画面を追加しました。共有UIにはV2バリアント、テーマ、プレビュー選択、コンテナスクロール、破棄確認を導入しました。未対応のJobs機能は無効状態で表示します。 ChangesV2 UI移行
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant V2SearchRoute
participant V2SearchScreen
participant SourceMediaGrid
participant MediaDetailRoute
User->>V2SearchRoute: /v2/searchを開く
V2SearchRoute->>V2SearchScreen: 検索状態とハンドラーを渡す
V2SearchScreen->>SourceMediaGrid: 仮想化結果を表示する
User->>SourceMediaGrid: プレビューを選択する
SourceMediaGrid->>V2SearchScreen: 選択メディアを通知する
User->>MediaDetailRoute: 詳細を開く
MediaDetailRoute-->>User: V2詳細画面を表示する
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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: 15
🧹 Nitpick comments (16)
packages/ui/src/screens/config-screen.tsx (1)
131-159: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Show when={!isV2()}が入れ子で重複しています。外側（Line 131）で既に非 V2 に限定されているため、Line 138 の内側の
Showは常に真です。削除できます。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/screens/config-screen.tsx` around lines 131 - 159, Remove the redundant inner Show wrapper around the form.Subscribe block in the non-V2 settings header; the outer Show already gates this content with !isV2(). Preserve the existing form subscription, button behavior, and layout unchanged.packages/ui/src/screens/v2-manager-screen.tsx (2)
1523-1563: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
EntityTablePanelが2箇所に重複しています。IP 取得失敗時の警告バナーの有無だけが違いなので、バナーを
Showで条件表示し、テーブルは1つに統合するとロジックの分岐が減ります。現状は同一 props の要素が2つあり、条件変化時に別インスタンスとして再マウントされる副作用もあります。♻️ リファクタ案
<Match when={isCrudCategory(activeCategory())}> - <Show - when={ - props.manager.activeTab() !== "characters" || - (props.manager.queryStates().ips.phase !== "error" && - props.manager.queryStates().ips.phase !== "offline") - } - > - <EntityTablePanel ... /> - </Show> - <Show - when={ - props.manager.activeTab() === "characters" && - (props.manager.queryStates().ips.phase === "error" || - props.manager.queryStates().ips.phase === "offline") - } - > + <Show when={hasIpLoadFailure()}> <div class="mb-3 flex flex-wrap items-center justify-between gap-2 rounded-md border border-warning-foreground/30 bg-warning/40 p-3"> ... </div> - <EntityTablePanel ... /> </Show> + <EntityTablePanel + manager={props.manager} + onQueryChange={setQuery} + onSelect={setSelectedId} + query={query()} + selectedId={selectedId()} + /> </Match>
hasIpLoadFailureは次のように定義できます。const hasIpLoadFailure = () => props.manager.activeTab() === "characters" && (props.manager.queryStates().ips.phase === "error" || props.manager.queryStates().ips.phase === "offline");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/screens/v2-manager-screen.tsx` around lines 1523 - 1563, Consolidate the duplicated EntityTablePanel instances in the isCrudCategory(activeCategory()) branch into one shared component. Define a hasIpLoadFailure helper using the active tab and ips query phase, render the warning banner conditionally with Show based on that helper, and keep the existing retry behavior and EntityTablePanel props unchanged.
1060-1063: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
SelectValue<unknown>と型キャストを避けられます。ここでは
stateを使わずselectedSource()を直接参照しているためunknownの型引数は不要です。あわせて 605-613 / 813-821 の(selected as { name: string }).nameも、SelectValue<SafeMediaSource>のように具体型を与えればasキャストを排除できます。コーディングガイドラインでは不要なunknownやキャストによる型安全性の回避を避けるよう定められています。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/screens/v2-manager-screen.tsx` around lines 1060 - 1063, Update the SelectValue usages in the source selectors, including the one rendering selectedSource() and the instances near the other source-selection blocks, to use the concrete SafeMediaSource type instead of unknown. Replace each `(selected as { name: string }).name` access with the typed SelectValue value so the casts are no longer needed.Source: Coding guidelines
packages/ui/src/screens/manager-screen.tsx (1)
111-119: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuevariant 分岐をリアクティブに評価する形へ寄せることを検討してください。
コンポーネント本体での早期リターンは初回セットアップ時にのみ評価されるため、将来
variantを動的に切り替えるようになった場合に再レンダリングされません。現状は/v2/managerから固定値で渡されているだけなので実害はありませんが、<Show>/<Switch>に寄せておくと安全です。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/screens/manager-screen.tsx` around lines 111 - 119, Update the variant selection in the manager screen component to use a reactive conditional such as Show or Switch instead of the component-body early return. Keep the existing V2ManagerScreen props and behavior unchanged, while ensuring changes to props.variant are reflected during rerendering.packages/ui/src/source-form-modal.tsx (1)
289-295: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win破棄確認が
variant === "v2"に限定されており、他モーダルの方針と揃っていません。
packages/ui/src/upload-media-modal.tsx:280-286とpackages/ui/src/import-review-modal.tsx:135-142はvariantに依らず未保存状態を検知して確認ダイアログを出します。本モーダルだけ default variant で無確認クローズになるため、意図的でなければ条件からvariantを外す方が一貫します。♻️ 提案する変更
const requestClose = () => { - if (props.variant === "v2" && form.state.isDirty) { + if (form.state.isDirty) { setShowDiscardDialog(true); return; } props.onClose(); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/source-form-modal.tsx` around lines 289 - 295, Update requestClose to show the discard confirmation whenever form.state.isDirty, regardless of props.variant. Preserve the existing immediate props.onClose() behavior for clean forms.packages/ui/src/v2/collection-inspector.tsx (1)
42-50: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
keyedによりprops.mediaの参照が変わるたびプレビュー全体が再マウントされます。同一メディアでもクエリ再取得で新しいオブジェクトになると再生成されるため、
when={props.media?.id}等でキーを安定させるかkeyedを外す方が無駄な再描画を避けられます。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/v2/collection-inspector.tsx` around lines 42 - 50, Update the Show block in collection-inspector.tsx to avoid remounting the preview whenever props.media receives a new object reference for the same media; remove keyed or key the condition by the stable media identifier such as props.media?.id, while preserving the existing fallback and preview behavior.apps/server/src/tests/e2e/v2-routes.responsive.spec.ts (1)
29-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winresponsive spec ですが、このテストは viewport を明示しておらず既定サイズ依存です。
expectNoHorizontalOverflowの結果が Playwright 設定の既定 viewport に左右されます。3つ目のテストのように検証したい幅を明示するか、モバイル幅・デスクトップ幅の両方でループする方が意図が明確になります。あわせてtest.step(path, ...)で包むと、どのルートで失敗したか特定しやすくなります。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/server/src/tests/e2e/v2-routes.responsive.spec.ts` around lines 29 - 53, Update the “V2 routes survive direct navigation and reload” test to use explicit viewport dimensions instead of Playwright’s default, preferably by looping through the intended mobile and desktop widths. Wrap each route-and-viewport iteration in test.step(path, ...) while preserving the existing navigation, reload, visibility, health, and overflow assertions.packages/ui/src/upload-media-modal.tsx (1)
280-286: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
initialFile起点で開いた場合、ユーザーが未編集でも必ず破棄確認が出ます。250-271 の初期化で
setSelectedFiles([props.initialFile])されるため、selectedFiles().length > 0が開いた直後から真になります。初期ファイルからの変更があるかで判定するとノイズを減らせます。♻️ 提案する変更
const requestClose = () => { - if (form.state.isDirty || selectedFiles().length > 0 || isFetchingUrl()) { + const initialCount = props.initialFile ? 1 : 0; + const hasFileChanges = selectedFiles().length !== initialCount; + if (form.state.isDirty || hasFileChanges || isFetchingUrl()) { setShowDiscardDialog(true); return; } props.onClose(); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/upload-media-modal.tsx` around lines 280 - 286, Update requestClose so an initialFile preloaded into selectedFiles does not by itself trigger the discard dialog. Compare the current selection against props.initialFile and treat the form as changed only when the user has modified the initial selection, while preserving prompts for other selected files, dirty form state, or active URL fetching.packages/ui/src/import-review-modal.tsx (1)
135-147: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift破棄確認フロー（
showDiscardDialogシグナル +requestClose/discardAndClose+AlertDialogJSX）が3モーダルでほぼ同一のまま重複しています。 共有フック（例:useDiscardGuard(isDirty)）と確認ダイアログコンポーネントに抽出すると、文言・v2-themeの適用・条件判定のばらつきも一箇所で揃えられます。
packages/ui/src/import-review-modal.tsx#L135-L147:requestClose/discardAndCloseと 356-374 のAlertDialogを共有フック・共有コンポーネント呼び出しに置き換える。packages/ui/src/source-form-modal.tsx#L289-L300: 同フックを使い、form.reset(defaultValues())を破棄時コールバックとして渡す。packages/ui/src/upload-media-modal.tsx#L280-L290: 同フックを使い、dirty 判定（フォーム・選択ファイル・取得中）を渡す形にする。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/import-review-modal.tsx` around lines 135 - 147, Extract the duplicated discard-confirmation flow into a shared useDiscardGuard hook and confirmation-dialog component, centralizing wording, v2-theme application, and condition handling. In packages/ui/src/import-review-modal.tsx:135-147, replace requestClose/discardAndClose and the related AlertDialog usage with the shared abstractions. In packages/ui/src/source-form-modal.tsx:289-300, use the hook and pass form.reset(defaultValues()) as the discard callback. In packages/ui/src/upload-media-modal.tsx:280-290, use the hook with the combined dirty state from the form, selected file, and loading state.packages/ui/src/screens/v2-search-screen.tsx (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winV2画面とデフォルト画面で派生ステートロジックが重複しています。
page/filterStates/マウントゲート(canRenderContent/shouldRenderGrid)の派生ロジックが、対応するデフォルト実装（search-screen.tsxのL53-77、source-media-screen.tsxのL85-87、いずれも本PRでは未変更）とほぼ同一のまま新規ファイルに複製されています。今後どちらか一方だけ修正され、もう一方が追従し忘れられるリスクがあります。
packages/ui/src/screens/v2-search-screen.tsx#L9-24:page/filterStates/canRenderContentの算出ロジックを共有フック（例:useMountGate(ssrGuard)や共通のfilterStates集約ヘルパー）に抽出し、search-screen.tsxの同等ロジックと共通化する。packages/ui/src/screens/v2-source-media-screen.tsx#L19-28: 同様にpage/filterStates/shouldRenderGridの算出ロジックを共有ヘルパーに抽出し、source-media-screen.tsxの同等ロジックと共通化する。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/screens/v2-search-screen.tsx` at line 1, Extract the duplicated page, filterStates, and mount-gate derivation from v2-search-screen.tsx and search-screen.tsx into shared helpers, preserving canRenderContent behavior. Similarly extract the corresponding logic from v2-source-media-screen.tsx and source-media-screen.tsx, preserving shouldRenderGrid behavior, and update all four screens to use the shared implementations.packages/ui/src/source-media-grid.tsx (2)
144-173: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff未使用側の virtualizer も常に生成・購読されます。
scrollMode === "element"でもcreateWindowVirtualizerが window のスクロール／リサイズを監視し続け、rowCount/scrollMarginシグナルにも反応します。動作上は問題ありませんが、大量アイテム時の無駄な再計算を避けたい場合はモードに応じて片方のみ生成する構成を検討してください。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/source-media-grid.tsx` around lines 144 - 173, Update the virtualizer setup around createWindowVirtualizer, createVirtualizer, and mediaRowVirtualizer so only the virtualizer matching props.scrollMode is created and subscribed. Preserve the existing configuration and selection behavior for both "element" and window modes while preventing the inactive virtualizer from reacting to scroll, resize, rowCount, or scrollMargin changes.
338-340: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value浮動小数の等値比較でスケルトンのアスペクト比を決めています。
props.itemAspectRatio === 4 / 3は現状の呼び出し（itemAspectRatio={4 / 3}）では一致しますが、1.333などが渡されると静かに3/4にフォールバックします。数値ではなく"4/3" | "3/4"のようなトークンを props で受け取る方が堅牢です。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/source-media-grid.tsx` around lines 338 - 340, Update the MediaGridSkeleton aspect-ratio selection in the surrounding source-media grid component to use an explicit `"4/3" | "3/4"` token prop instead of comparing props.itemAspectRatio with `4 / 3`. Adjust the prop definition and current callers, including the existing 4/3 usage, so the intended ratio is passed directly and no numeric fallback comparison remains.apps/server/src/routes/sources/$mediaSourceId/components/source-media-page.tsx (1)
155-164: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
onOpenMediaDetailが variant に関係なく v2 ルートへ遷移します。
props.variant === "default"でこのコールバックが利用されるようになった場合、旧 UI から突然/v2/...へ飛ぶことになります。現状は default のSourceMediaScreenがこのコールバックを使っていないため実害はありませんが、遷移先を variant で分けるか、v2 のときだけ渡す形にしておくと安全です。♻️ 提案
onOpenMediaDetail={(media) => { + if (props.variant !== "v2") { + void navigate({ + params: { + mediaId: media.id, + mediaSourceId: media.mediaSourceId, + }, + to: "/sources/$mediaSourceId/$mediaId", + }); + return; + } sessionStorage.setItem("v2:media-return", location().href);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/server/src/routes/sources/`$mediaSourceId/components/source-media-page.tsx around lines 155 - 164, Update onOpenMediaDetail in SourceMediaPage so navigation respects props.variant instead of always targeting the v2 route. Keep the existing v2 destination for the v2 variant, and route the default variant through the legacy destination or avoid passing this callback to it, ensuring the legacy UI cannot navigate to /v2.apps/server/src/components/media/media-grid-item.tsx (1)
25-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win2つの
Link分岐はtoと sessionStorage 保存だけが違うため統合を検討してください。クリックハンドラのロジックがほぼ完全に重複しており、片方だけ修正される事故が起きやすくなっています（実際に v2 側だけ
returnがあり、default 側にはありません）。♻️ 重複を削減する例
- const detailLink = (linkProps: MediaGridLinkProps) => - props.routeVersion === "v2" ? ( - <Link ... to="/v2/sources/$mediaSourceId/$mediaId">...</Link> - ) : ( - <Link ... to="/sources/$mediaSourceId/$mediaId">...</Link> - ); + const isV2 = () => props.routeVersion === "v2"; + const detailLink = (linkProps: MediaGridLinkProps) => ( + <Link + class={linkProps.class} + data-media-id={linkProps["data-media-id"]} + onClick={(event: MouseEvent) => { + if ( + props.onPreviewSelect && + window.matchMedia("(min-width: 1536px)").matches + ) { + event.preventDefault(); + props.onPreviewSelect(); + return; + } + if ( + isV2() && + event.button === 0 && + !(event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) + ) { + sessionStorage.setItem("v2:media-return", location().href); + } + }} + onContextMenu={linkProps.onContextMenu} + params={{ + mediaId: props.media.id, + mediaSourceId: props.media.mediaSourceId, + }} + to={ + isV2() + ? "/v2/sources/$mediaSourceId/$mediaId" + : "/sources/$mediaSourceId/$mediaId" + } + > + {linkProps.children} + </Link> + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/server/src/components/media/media-grid-item.tsx` around lines 25 - 80, 統合可能な2つのLink分岐で重複しているクリック処理を、detailLink内の共通ハンドラへまとめてください。routeVersionに応じて遷移先だけを切り替え、v2の場合のみ通常クリック時のsessionStorage保存を実行し、プレビュー選択時は両方でpreventDefaultと処理終了を一貫して適用してください。apps/server/src/components/v2/v2-app-shell.tsx (1)
110-120: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win新しいv2コンポーネント群で
outline-noneが広く使われていますが、Tailwind v4 では完全な代替になりません。 提供された Tailwind v4 ドキュメントによると、outline-noneはoutline-style: noneのみを適用するようになり、強制カラーモード（ハイコントラストモード）でフォーカスリングが視認できなくなる可能性があります。カスタムのfocus-visible:ring-*でネイティブのアウトラインを完全に置き換える意図がある箇所は、いずれもoutline-hidden（もしくは該当バリアント版）へ置き換えることを推奨します。根本原因は共通のため、以下の4箇所をまとめて修正することを提案します。
apps/server/src/components/v2/v2-app-shell.tsx#L110-L120:V2NavigationItemのLinkクラスのoutline-noneをoutline-hiddenに変更（同ファイル内のCollapsibleTrigger・V2SidebarのLink・V2SourceActionsのPopoverTriggerも同様）。packages/ui/src/v2/management-layout.tsx#L37-L43:v2CategoryButtonClass内のoutline-noneをoutline-hiddenに変更。packages/ui/src/v2/search-toolbar.tsx#L291-L301: トークン削除ボタンのfocus-visible:outline-noneをfocus-visible:outline-hiddenに変更。packages/ui/src/media-grid-item.tsx#L56-L64: v2バリアントのグリッドアイテムリンククラスのoutline-noneをoutline-hiddenに変更。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/server/src/components/v2/v2-app-shell.tsx` around lines 110 - 120, Replace outline-none with outline-hidden wherever the v2 components intentionally replace the native focus outline with custom focus-visible rings. In apps/server/src/components/v2/v2-app-shell.tsx#L110-L120, update V2NavigationItem and the same-file CollapsibleTrigger, V2Sidebar Link, and V2SourceActions PopoverTrigger; also update v2CategoryButtonClass in packages/ui/src/v2/management-layout.tsx#L37-L43, the token removal button’s focus-visible:outline-none in packages/ui/src/v2/search-toolbar.tsx#L291-L301 to focus-visible:outline-hidden, and the v2 grid-item link class in packages/ui/src/media-grid-item.tsx#L56-L64.packages/ui/src/toast.tsx (1)
50-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
as unknown as ...キャストの重複と未文書化。
error/success/info/warning/messageの各実装で同じas unknown as Parameters<typeof SonnerToast.X>[1]パターンが繰り返し使われており、理由を説明するコメントもありません。ライブラリ境界でのやむを得ない例外は、狭いスコープに限定し理由を明記することが求められています。共通のマッピング関数に集約し、キャストの理由をコメントで残すことを推奨します。♻️ 提案: 共通ヘルパーへの集約例
+// solid-sonner の内部型とローカルの `ToastOptions` は構造的に一致しないため、 +// 呼び出し境界でのみキャストを許容する。 +function toSonnerOptions<T>(opts: ToastOptions | undefined): T { + return opts as unknown as T; +} + error: (msg: string, opts?: ToastOptions) => - toastImpl.error(msg, { duration: 9_000, ...opts } as unknown as Parameters< - typeof SonnerToast.error - >[1]), + toastImpl.error( + msg, + toSonnerOptions<Parameters<typeof SonnerToast.error>[1]>({ + duration: 9_000, + ...opts, + }), + ),Also applies to: 83-86
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/toast.tsx` around lines 50 - 66, Consolidate the repeated option conversions used by the toastImpl error, success, info, warning, and message implementations into one shared mapping helper. Keep the unavoidable SonnerToast parameter cast confined to that helper, and add a concise comment documenting the library-boundary type mismatch; remove the duplicated casts from each toast method.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/server/src/components/media/media-sidebar.tsx`:
- Around line 896-928: AlertDialogAction の意図的な確定操作後に onOpenChange(false) が
navigationBlocker の reset を呼ばないよう、許可済み遷移を示すフラグを追加して管理してください。onClick で説明編集状態を更新して
proceed() する前にフラグを設定し、onOpenChange ではそのフラグを消費して reset
をスキップする一方、Cancel・Esc・オーバーレイクリックによる通常の閉鎖では従来どおり resolver.reset?.() を実行してください。対象は
AlertDialog の onOpenChange と AlertDialogAction の onClick です。
In `@apps/server/src/components/v2/v2-app-shell.tsx`:
- Around line 436-450: モバイルナビゲーションの DialogContent に v2-theme クラスを追加し、既存の p-0
と組み合わせて class="v2-theme p-0"
としてください。placement="left"、V2Sidebar、その他のダイアログ挙動は変更しないでください。
In `@apps/server/src/routes/v2/sources/`$mediaSourceId/$mediaId/index.tsx:
- Around line 49-65: Update returnToCollection so it does not call
window.history.back() solely because a valid v2:media-return value exists. Use
the stored returnPath for explicit navigation, or verify that usable back
history exists before going back and otherwise navigate to the appropriate
collection route; preserve cleanup of v2:media-return.
In `@packages/ui/src/hooks/use-search-page.ts`:
- Around line 250-263:
スクロールコンテナ用ヘルパーを共有モジュールへ切り出し、packages/ui/src/hooks/use-search-page.ts
の250-263行では既存のローカル実装を削除して共有ヘルパーをimportし、セレクタ指定時にコンテナを取得できなければ setScrollY
が0を保存せずスキップするよう更新してください。packages/ui/src/hooks/use-source-media-page.ts
の244-257行も同じ共有ヘルパーへ置き換え、setScrollPosition
でコンテナ未取得時の0保存を回避してください。セレクタ未指定時のみwindowをフォールバックに使用します。
In `@packages/ui/src/import-review-modal.tsx`:
- Around line 356-374: Unify the language used by the discard confirmation
dialog and the surrounding import-review modal. Update the visible labels and
messages associated with AlertDialog, including AlertDialogTitle,
AlertDialogDescription, AlertDialogCancel, and AlertDialogAction, to match the
modal’s existing English text.
- Around line 196-207: Ensure the discard-confirmation AlertDialog preserves the
active theme across its portal by applying v2-theme to its AlertDialogContent
when props.variant is v2, or update the --v2-surface-muted background usage in
the default variant to fall back to the standard background value. Use the
existing AlertDialogContent and variant handling in import-review-modal.tsx.
In `@packages/ui/src/media-grid-item.tsx`:
- Around line 70-81: Update the selection indicator in the media-grid item
render, replacing the aria-hidden-only presentation with an accessible
selection-state semantic such as aria-pressed or aria-selected on the
interactive element represented by LinkComponent. Ensure both bulk-select mode
and the v2 selected state expose props.isSelected to screen readers while
preserving the existing visual checkmark.
In `@packages/ui/src/pending-downloads-indicator.tsx`:
- Around line 94-122: Update the button class in the pending-downloads indicator
so it has position: relative whenever the compact badge uses absolute
positioning. Preserve the existing variant and compact styling while ensuring
the badge is positioned relative to its own button.
In `@packages/ui/src/screens/config-screen.tsx`:
- Around line 326-338: Replace the undefined --v2-muted color reference with
--v2-text-muted in the V2 config-screen text styles, including the connection
status text and settings-unsaved text, while preserving the existing styling and
layout.
- Around line 179-197: Update the TabsTrigger class selection in the
SETTINGS_CATEGORIES rendering to branch on isV2(), applying
V2_CATEGORY_TABS_CLASS only for the V2 variant and preserving the existing
legacy Settings tab classes for variant="default".
In `@packages/ui/src/screens/v2-manager-screen.tsx`:
- Around line 738-753: Update the Progress value calculation in the jobProgress
display to guard against progress().total being zero, ensuring Progress never
receives NaN. Preserve the existing percentage calculation for positive totals
and choose an appropriate bounded fallback for the zero-total case.
In `@packages/ui/src/screens/v2-search-screen.tsx`:
- Line 1: Guard the preview panels in the relevant render blocks of the search
and source-media screens so they only render when the selected media still
exists, by adding the `previewMedia()` presence check to each `<Show>`
condition. Use the existing `previewMedia` accessor and preserve the current
`props.renderMediaPreview` requirement.
- Around line 21-23: Update the fallback value returned by sourceName in
v2-search-screen.tsx from the English label to the Japanese equivalent “メディア一覧”,
matching the existing wording in v2-source-media-screen.tsx.
In `@packages/ui/src/source-media-grid.tsx`:
- Around line 182-199: Update updateMediaGridMetrics so scrollMargin in element
scroll mode is calculated relative to the resolved scroller from
scrollElement(), rather than using mediaGridRef.offsetTop; preserve the
document-based calculation for other scroll modes and ensure the virtual-row
translation remains aligned to the scroll container.
In `@packages/ui/src/v2/search-toolbar.tsx`:
- Around line 406-415: Remove the setFilterOpen(true) handler from the sort
button in search-toolbar.tsx, keeping it dedicated to sorting. Ensure the filter
popover is opened only through the filter button’s PopoverTrigger, or through an
explicitly shared trigger group that preserves the correct anchoring.
---
Nitpick comments:
In `@apps/server/src/components/media/media-grid-item.tsx`:
- Around line 25-80:
統合可能な2つのLink分岐で重複しているクリック処理を、detailLink内の共通ハンドラへまとめてください。routeVersionに応じて遷移先だけを切り替え、v2の場合のみ通常クリック時のsessionStorage保存を実行し、プレビュー選択時は両方でpreventDefaultと処理終了を一貫して適用してください。
In `@apps/server/src/components/v2/v2-app-shell.tsx`:
- Around line 110-120: Replace outline-none with outline-hidden wherever the v2
components intentionally replace the native focus outline with custom
focus-visible rings. In
apps/server/src/components/v2/v2-app-shell.tsx#L110-L120, update
V2NavigationItem and the same-file CollapsibleTrigger, V2Sidebar Link, and
V2SourceActions PopoverTrigger; also update v2CategoryButtonClass in
packages/ui/src/v2/management-layout.tsx#L37-L43, the token removal button’s
focus-visible:outline-none in packages/ui/src/v2/search-toolbar.tsx#L291-L301 to
focus-visible:outline-hidden, and the v2 grid-item link class in
packages/ui/src/media-grid-item.tsx#L56-L64.
In
`@apps/server/src/routes/sources/`$mediaSourceId/components/source-media-page.tsx:
- Around line 155-164: Update onOpenMediaDetail in SourceMediaPage so navigation
respects props.variant instead of always targeting the v2 route. Keep the
existing v2 destination for the v2 variant, and route the default variant
through the legacy destination or avoid passing this callback to it, ensuring
the legacy UI cannot navigate to /v2.
In `@apps/server/src/tests/e2e/v2-routes.responsive.spec.ts`:
- Around line 29-53: Update the “V2 routes survive direct navigation and reload”
test to use explicit viewport dimensions instead of Playwright’s default,
preferably by looping through the intended mobile and desktop widths. Wrap each
route-and-viewport iteration in test.step(path, ...) while preserving the
existing navigation, reload, visibility, health, and overflow assertions.
In `@packages/ui/src/import-review-modal.tsx`:
- Around line 135-147: Extract the duplicated discard-confirmation flow into a
shared useDiscardGuard hook and confirmation-dialog component, centralizing
wording, v2-theme application, and condition handling. In
packages/ui/src/import-review-modal.tsx:135-147, replace
requestClose/discardAndClose and the related AlertDialog usage with the shared
abstractions. In packages/ui/src/source-form-modal.tsx:289-300, use the hook and
pass form.reset(defaultValues()) as the discard callback. In
packages/ui/src/upload-media-modal.tsx:280-290, use the hook with the combined
dirty state from the form, selected file, and loading state.
In `@packages/ui/src/screens/config-screen.tsx`:
- Around line 131-159: Remove the redundant inner Show wrapper around the
form.Subscribe block in the non-V2 settings header; the outer Show already gates
this content with !isV2(). Preserve the existing form subscription, button
behavior, and layout unchanged.
In `@packages/ui/src/screens/manager-screen.tsx`:
- Around line 111-119: Update the variant selection in the manager screen
component to use a reactive conditional such as Show or Switch instead of the
component-body early return. Keep the existing V2ManagerScreen props and
behavior unchanged, while ensuring changes to props.variant are reflected during
rerendering.
In `@packages/ui/src/screens/v2-manager-screen.tsx`:
- Around line 1523-1563: Consolidate the duplicated EntityTablePanel instances
in the isCrudCategory(activeCategory()) branch into one shared component. Define
a hasIpLoadFailure helper using the active tab and ips query phase, render the
warning banner conditionally with Show based on that helper, and keep the
existing retry behavior and EntityTablePanel props unchanged.
- Around line 1060-1063: Update the SelectValue usages in the source selectors,
including the one rendering selectedSource() and the instances near the other
source-selection blocks, to use the concrete SafeMediaSource type instead of
unknown. Replace each `(selected as { name: string }).name` access with the
typed SelectValue value so the casts are no longer needed.
In `@packages/ui/src/screens/v2-search-screen.tsx`:
- Line 1: Extract the duplicated page, filterStates, and mount-gate derivation
from v2-search-screen.tsx and search-screen.tsx into shared helpers, preserving
canRenderContent behavior. Similarly extract the corresponding logic from
v2-source-media-screen.tsx and source-media-screen.tsx, preserving
shouldRenderGrid behavior, and update all four screens to use the shared
implementations.
In `@packages/ui/src/source-form-modal.tsx`:
- Around line 289-295: Update requestClose to show the discard confirmation
whenever form.state.isDirty, regardless of props.variant. Preserve the existing
immediate props.onClose() behavior for clean forms.
In `@packages/ui/src/source-media-grid.tsx`:
- Around line 144-173: Update the virtualizer setup around
createWindowVirtualizer, createVirtualizer, and mediaRowVirtualizer so only the
virtualizer matching props.scrollMode is created and subscribed. Preserve the
existing configuration and selection behavior for both "element" and window
modes while preventing the inactive virtualizer from reacting to scroll, resize,
rowCount, or scrollMargin changes.
- Around line 338-340: Update the MediaGridSkeleton aspect-ratio selection in
the surrounding source-media grid component to use an explicit `"4/3" | "3/4"`
token prop instead of comparing props.itemAspectRatio with `4 / 3`. Adjust the
prop definition and current callers, including the existing 4/3 usage, so the
intended ratio is passed directly and no numeric fallback comparison remains.
In `@packages/ui/src/toast.tsx`:
- Around line 50-66: Consolidate the repeated option conversions used by the
toastImpl error, success, info, warning, and message implementations into one
shared mapping helper. Keep the unavoidable SonnerToast parameter cast confined
to that helper, and add a concise comment documenting the library-boundary type
mismatch; remove the duplicated casts from each toast method.
In `@packages/ui/src/upload-media-modal.tsx`:
- Around line 280-286: Update requestClose so an initialFile preloaded into
selectedFiles does not by itself trigger the discard dialog. Compare the current
selection against props.initialFile and treat the form as changed only when the
user has modified the initial selection, while preserving prompts for other
selected files, dirty form state, or active URL fetching.
In `@packages/ui/src/v2/collection-inspector.tsx`:
- Around line 42-50: Update the Show block in collection-inspector.tsx to avoid
remounting the preview whenever props.media receives a new object reference for
the same media; remove keyed or key the condition by the stable media identifier
such as props.media?.id, while preserving the existing fallback and preview
behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e00c881-1716-496d-9e05-6229ca2c4eb3
📒 Files selected for processing (48)
REPORT.mdapps/server/src/components/imports/pending-downloads-indicator.tsxapps/server/src/components/media/media-grid-item.tsxapps/server/src/components/media/media-sidebar.tsxapps/server/src/components/media/media-viewer.tsxapps/server/src/components/v2/v2-app-shell.tsxapps/server/src/routes/__root.tsxapps/server/src/routes/sources/$mediaSourceId/components/source-media-page.tsxapps/server/src/routes/v2/$.tsxapps/server/src/routes/v2/about.tsxapps/server/src/routes/v2/config.tsxapps/server/src/routes/v2/index.tsxapps/server/src/routes/v2/jobs.tsxapps/server/src/routes/v2/manager.tsxapps/server/src/routes/v2/route.tsxapps/server/src/routes/v2/search.tsxapps/server/src/routes/v2/sources/$mediaSourceId/$mediaId/index.tsxapps/server/src/routes/v2/sources/$mediaSourceId/index.tsxapps/server/src/tests/e2e/v2-routes.responsive.spec.tspackages/ui/src/dialog.tsxpackages/ui/src/hooks/use-manager-page.tspackages/ui/src/hooks/use-search-page.tspackages/ui/src/hooks/use-source-media-page.tspackages/ui/src/import-review-modal.tsxpackages/ui/src/media-grid-item.tsxpackages/ui/src/media-viewer.tsxpackages/ui/src/pending-downloads-indicator.tsxpackages/ui/src/screens/config-screen.tsxpackages/ui/src/screens/config-state-screen.tsxpackages/ui/src/screens/manager-screen.tsxpackages/ui/src/screens/media-detail-screen.tsxpackages/ui/src/screens/search-screen.tsxpackages/ui/src/screens/source-media-screen.tsxpackages/ui/src/screens/v2-jobs-screen.tsxpackages/ui/src/screens/v2-manager-screen.tsxpackages/ui/src/screens/v2-search-screen.tsxpackages/ui/src/screens/v2-source-media-screen.tsxpackages/ui/src/skeleton.tsxpackages/ui/src/source-form-modal.tsxpackages/ui/src/source-media-grid.tsxpackages/ui/src/source-media-page.tsxpackages/ui/src/styles/theme.csspackages/ui/src/toast.tsxpackages/ui/src/upload-media-modal.tsxpackages/ui/src/v2/collection-inspector.tsxpackages/ui/src/v2/icons.tsxpackages/ui/src/v2/management-layout.tsxpackages/ui/src/v2/search-toolbar.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/source-media-grid.tsx (1)
185-201: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winレイアウト変化時にも
scrollMarginを再計算してください。
ResizeObserverはmediaGridRefだけを監視します。V2 画面ではQueryStatusまたはFilterErrorBannerがグリッドの前に表示されます。これらの高さが変わってもグリッド自体のサイズは変わらないため、Line 188-190 の位置計算は古い値のままです。仮想行はその古い値を使用して上下にずれます。グリッドのレイアウト親も監視してください。
修正案
const resizeObserver = new ResizeObserver(() => { updateMediaGridMetrics(); }); if (mediaGridRef) { resizeObserver.observe(mediaGridRef); + if (mediaGridRef.parentElement) { + resizeObserver.observe(mediaGridRef.parentElement); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/source-media-grid.tsx` around lines 185 - 201, Update the ResizeObserver setup associated with the scroll-margin calculation to also observe the grid’s layout parent, not only mediaGridRef. Ensure changes in preceding QueryStatus or FilterErrorBanner content trigger the existing position calculation to recompute scrollMargin, while preserving the current mediaGridRef observation and scroll-mode behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/hooks/scroll-container.ts`:
- Line 5: Update the selector lookup in currentScrollPosition() to catch
SyntaxError from document.querySelector for invalid selectors and return null
instead, preserving the helper’s existing nullable return contract and ensuring
cleanup continues.
---
Outside diff comments:
In `@packages/ui/src/source-media-grid.tsx`:
- Around line 185-201: Update the ResizeObserver setup associated with the
scroll-margin calculation to also observe the grid’s layout parent, not only
mediaGridRef. Ensure changes in preceding QueryStatus or FilterErrorBanner
content trigger the existing position calculation to recompute scrollMargin,
while preserving the current mediaGridRef observation and scroll-mode behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e731079c-b264-40a1-a9e6-acc5466e490a
📒 Files selected for processing (23)
apps/server/src/components/imports/pending-downloads-indicator.tsxapps/server/src/components/media/media-sidebar.tsxapps/server/src/components/v2/v2-app-shell.tsxapps/server/src/routes/v2/sources/$mediaSourceId/$mediaId/index.tsxapps/server/src/tests/e2e/v2-routes.responsive.spec.tsapps/server/src/tests/unit/application/services/maintenance-service.test.tspackages/application/src/services/media-processing-service.tspackages/ui/src/combobox.tsxpackages/ui/src/hooks/scroll-container.tspackages/ui/src/hooks/use-search-page.tspackages/ui/src/hooks/use-source-media-page.tspackages/ui/src/import-review-modal.tsxpackages/ui/src/media-grid-item.tsxpackages/ui/src/pending-downloads-indicator.tsxpackages/ui/src/screens/config-screen.tsxpackages/ui/src/screens/v2-manager-screen.tsxpackages/ui/src/screens/v2-search-screen.tsxpackages/ui/src/screens/v2-source-media-screen.tsxpackages/ui/src/search-control-panel.tsxpackages/ui/src/search-filters.tsxpackages/ui/src/sort-controls.tsxpackages/ui/src/source-media-grid.tsxpackages/ui/src/v2/search-toolbar.tsx
💤 Files with no reviewable changes (1)
- packages/application/src/services/media-processing-service.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/ui/src/screens/v2-search-screen.tsx
- apps/server/src/components/imports/pending-downloads-indicator.tsx
- apps/server/src/routes/v2/sources/$mediaSourceId/$mediaId/index.tsx
- packages/ui/src/pending-downloads-indicator.tsx
- packages/ui/src/screens/v2-source-media-screen.tsx
- packages/ui/src/media-grid-item.tsx
- apps/server/src/components/v2/v2-app-shell.tsx
- packages/ui/src/import-review-modal.tsx
- packages/ui/src/screens/v2-manager-screen.tsx
- apps/server/src/components/media/media-sidebar.tsx
| selector: string | undefined, | ||
| ): HTMLElement | null { | ||
| if (!selector || typeof document === "undefined") return null; | ||
| return document.querySelector<HTMLElement>(selector); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
無効な CSS セレクタを安全に処理してください。
Line 5 の document.querySelector() は、無効な selector で SyntaxError を送出します。currentScrollPosition() は cleanup から呼ばれるため、cleanup 処理も中断します。例外を null に変換して、このヘルパーの戻り値契約を維持してください。
修正案
export function resolveScrollContainer(
selector: string | undefined,
): HTMLElement | null {
if (!selector || typeof document === "undefined") return null;
- return document.querySelector<HTMLElement>(selector);
+ try {
+ return document.querySelector<HTMLElement>(selector);
+ } catch {
+ return null;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return document.querySelector<HTMLElement>(selector); | |
| export function resolveScrollContainer( | |
| selector: string | undefined, | |
| ): HTMLElement | null { | |
| if (!selector || typeof document === "undefined") return null; | |
| try { | |
| return document.querySelector<HTMLElement>(selector); | |
| } catch { | |
| return null; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/hooks/scroll-container.ts` at line 5, Update the selector
lookup in currentScrollPosition() to catch SyntaxError from
document.querySelector for invalid selectors and return null instead, preserving
the helper’s existing nullable return contract and ensuring cleanup continues.
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)
packages/ui/src/v2/search-toolbar.tsx (1)
405-415: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win「すべて解除」が保留中の変更を取り消しません。
clearPresetFilters()はsearchStateを初期化します。しかしpendingSuggestions、pendingRemovals、およびsearchApplyTimerはそのまま残ります。デバウンス中に「すべて解除」を押すと、250ms後にapplyPendingChangesが実行され、解除したはずの条件が再びsearchStateへ書き戻されます。同じ理由で、449行の「適用」ボタンも保留中の変更を反映せずに検索します。その後デバウンスが発火し、2回目の検索が走ります。
タイマーを解除して保留状態を破棄する処理を追加してください。
🐛 提案する修正
+ const discardPendingChanges = () => { + if (searchApplyTimer) clearTimeout(searchApplyTimer); + searchApplyTimer = undefined; + setPendingSuggestions([]); + setPendingRemovals([]); + };<Button class="h-7 px-2 text-xs" onClick={() => { + discardPendingChanges(); clearPresetFilters(); props.onSearch(); }}適用ボタンでは、破棄ではなく即時反映が適切です。
onClick={() => { - props.onSearch(); + applyPendingChanges(); + props.onSearch(); setFilterOpen(false); }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/v2/search-toolbar.tsx` around lines 405 - 415, Update the 「すべて解除」 handler to cancel searchApplyTimer and discard pendingSuggestions and pendingRemovals before calling clearPresetFilters and onSearch. Update the 「適用」 handler to immediately apply pending changes before searching, clearing the debounce timer so applyPendingChanges does not run a second time.
🧹 Nitpick comments (5)
packages/ui/src/v2/search-composer-utils.ts (2)
33-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
SUGGESTION_CONFIGSの索引型を明示してください。
Record<string, SuggestionConfig>は、未定義キーの参照でもSuggestionConfig型を返します。61行目のif (!config)ガードは実行時には正しく動作しますが、型上は常に真として扱われます。索引アクセスの結果をundefinedを含む型にすると、ガードが型システムでも意味を持ちます。♻️ 提案する変更
-const SUGGESTION_CONFIGS: Record<string, SuggestionConfig> = { +const SUGGESTION_CONFIGS: Record<string, SuggestionConfig | undefined> = {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/v2/search-composer-utils.ts` around lines 33 - 44, Update the SUGGESTION_CONFIGS index type to include undefined for missing keys, such as using a partial record, so lookups correctly produce SuggestionConfig | undefined and the existing config guard remains type-valid.
131-139: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value候補の並び順を前方一致優先にしてください。
現在は部分一致した順に上位100件を切り出します。候補が多い場合、前方一致する値が100件目より後ろに来ると表示されません。前方一致を先に並べると、入力に対する候補の適合度が上がります。
♻️ 提案する変更
return getSourceValues(filterData, active.config.source) .filter(({ value }) => { const normalized = value.toLowerCase(); if (selectedValues.has(value) || seen.has(value)) return false; if (!normalized.includes(query)) return false; seen.add(value); return true; }) + .sort((a, b) => { + const aPrefix = a.value.toLowerCase().startsWith(query) ? 0 : 1; + const bPrefix = b.value.toLowerCase().startsWith(query) ? 0 : 1; + return aPrefix - bPrefix; + }) .slice(0, 100)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/v2/search-composer-utils.ts` around lines 131 - 139, Update the filtering flow in the source-value candidate list around getSourceValues so prefix matches are ordered before other substring matches before applying the 100-item slice. Preserve the existing selectedValues and seen de-duplication behavior while ensuring values beginning with query are not excluded because they occur after the limit.packages/ui/src/v2/search-toolbar.tsx (1)
234-251: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win保留変更の適用処理が
submitDraftと重複します。234-247行のループは、
submitDraft（287-293行）の処理と同一です。共通のヘルパーへ抽出すると、片方だけを変更する不整合を防げます。♻️ 提案する変更
+ const commitPending = ( + additions: SearchSuggestion[], + removals: SearchToken[], + ) => { + for (const token of removals) removeToken(token); + for (const suggestion of additions) { + setSearchState(suggestion.key, [ + ...new Set([...searchState[suggestion.key], suggestion.value]), + ]); + } + }; const applyPendingChanges = () => { searchApplyTimer = undefined; const additions = pendingSuggestions(); const removals = pendingRemovals(); if (additions.length === 0 && removals.length === 0) return; - batch(() => { - for (const token of removals) removeToken(token); - for (const suggestion of additions) { - setSearchState(suggestion.key, [ - ...new Set([...searchState[suggestion.key], suggestion.value]), - ]); - } - }); + batch(() => commitPending(additions, removals));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/v2/search-toolbar.tsx` around lines 234 - 251, Extract the duplicated token-application logic from applyPendingChanges and submitDraft into a shared helper, then call that helper from both paths. Preserve the existing batch behavior and ensure the helper handles both removals and additions consistently.packages/ui/src/v2/search-composer.test.ts (1)
16-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
removeActiveSearchTokenのテストを追加してください。
removeActiveSearchTokenは公開関数で、候補選択後のドラフト書き換えを担当します（search-composer.tsxのhandleSelect）。現在この関数のテストがありません。先頭トークンのみの場合、複数トークンの場合、末尾が空白の場合の3ケースを追加すると、回帰を検出できます。💚 追加テストの例
+import { + getSearchComposerSuggestions, + removeActiveSearchToken, + type SearchToken, +} from "./search-composer-utils"; + +describe("removeActiveSearchToken", () => { + it("removes only the trailing token", () => { + expect(removeActiveSearchToken("tag:blue")).toBe(""); + expect(removeActiveSearchToken("image tag:blue")).toBe("image "); + expect(removeActiveSearchToken("image ")).toBe("image "); + }); +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/v2/search-composer.test.ts` around lines 16 - 64, search-composer.test.ts に公開関数 removeActiveSearchToken のテストを追加してください。先頭トークンのみを削除する場合、複数トークンから対象トークンを削除する場合、末尾が空白のドラフトから削除する場合の3ケースを検証し、handleSelect から利用されるドラフト書き換えが期待どおりになることを確認してください。packages/ui/src/v2/search-composer.tsx (1)
120-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value検索アイコンの垂直位置を中央基準にしてください。
ComboboxControlの高さはモバイルでmin-h-11、sm以上でmin-h-9です。アイコンはtop-[0.7rem]の固定値なので、高さが変わると中央からずれます。トークンが折り返して高さが増えた場合も同様です。♻️ 提案する変更
- class="absolute top-[0.7rem] left-3 z-10 text-[var(--v2-text-muted)]" + class="absolute top-5 left-3 z-10 -translate-y-1/2 text-[var(--v2-text-muted)]"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/v2/search-composer.tsx` around lines 120 - 124, Update the Search icon positioning in the ComboboxControl area to vertically center it relative to the control instead of using the fixed top-[0.7rem] offset. Use the existing absolute-positioning context with a 50% vertical offset and corresponding translation so the icon remains centered for responsive heights and wrapped tokens.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/v2/search-composer.tsx`:
- Around line 80-89: Update handleSelect so the draft value produced by
removeActiveSearchToken(props.draft) is also assigned to suggestionDraft when
the draft changes. Keep suggestionDraft synchronized with the value passed to
props.onDraftChange, preventing stale tokens from reopening the suggestions menu
on focus.
In `@packages/ui/src/v2/search-toolbar.tsx`:
- Around line 219-231: Update the pendingSuggestions token mapping in the tokens
memo to set destructive: true when the suggestion represents an exclusion tag,
matching the behavior established by tokensFromState. Preserve non-exclusion
suggestions’ existing appearance and fields.
---
Outside diff comments:
In `@packages/ui/src/v2/search-toolbar.tsx`:
- Around line 405-415: Update the 「すべて解除」 handler to cancel searchApplyTimer and
discard pendingSuggestions and pendingRemovals before calling clearPresetFilters
and onSearch. Update the 「適用」 handler to immediately apply pending changes
before searching, clearing the debounce timer so applyPendingChanges does not
run a second time.
---
Nitpick comments:
In `@packages/ui/src/v2/search-composer-utils.ts`:
- Around line 33-44: Update the SUGGESTION_CONFIGS index type to include
undefined for missing keys, such as using a partial record, so lookups correctly
produce SuggestionConfig | undefined and the existing config guard remains
type-valid.
- Around line 131-139: Update the filtering flow in the source-value candidate
list around getSourceValues so prefix matches are ordered before other substring
matches before applying the 100-item slice. Preserve the existing selectedValues
and seen de-duplication behavior while ensuring values beginning with query are
not excluded because they occur after the limit.
In `@packages/ui/src/v2/search-composer.test.ts`:
- Around line 16-64: search-composer.test.ts に公開関数 removeActiveSearchToken
のテストを追加してください。先頭トークンのみを削除する場合、複数トークンから対象トークンを削除する場合、末尾が空白のドラフトから削除する場合の3ケースを検証し、handleSelect
から利用されるドラフト書き換えが期待どおりになることを確認してください。
In `@packages/ui/src/v2/search-composer.tsx`:
- Around line 120-124: Update the Search icon positioning in the ComboboxControl
area to vertically center it relative to the control instead of using the fixed
top-[0.7rem] offset. Use the existing absolute-positioning context with a 50%
vertical offset and corresponding translation so the icon remains centered for
responsive heights and wrapped tokens.
In `@packages/ui/src/v2/search-toolbar.tsx`:
- Around line 234-251: Extract the duplicated token-application logic from
applyPendingChanges and submitDraft into a shared helper, then call that helper
from both paths. Preserve the existing batch behavior and ensure the helper
handles both removals and additions 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ec59348-eba5-44b5-b3c5-b258c4b30b62
📒 Files selected for processing (8)
apps/server/src/router.tsxpackages/ui/src/combobox.tsxpackages/ui/src/layouts/app-nav.tsxpackages/ui/src/screens/design-concept-screen.tsxpackages/ui/src/v2/search-composer-utils.tspackages/ui/src/v2/search-composer.test.tspackages/ui/src/v2/search-composer.tsxpackages/ui/src/v2/search-toolbar.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui/src/combobox.tsx
Comment on lines
+80
to
+89
| const handleSelect = (suggestion: SearchSuggestion | null) => { | ||
| if (!suggestion) return; | ||
| props.onSelectSuggestion(suggestion); | ||
| setSelectedSuggestion(suggestion); | ||
| setMenuOpen(false); | ||
| requestAnimationFrame(() => { | ||
| setSelectedSuggestion(null); | ||
| props.onDraftChange(removeActiveSearchToken(props.draft)); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
候補選択後に suggestionDraft が古いままになります。
handleSelect は props.onDraftChange でドラフトからトークンを削除します。しかし suggestionDraft は onInputChange（138-142行）でしか更新されません。そのため選択後もデバウンス済みドラフトは tag:blue のような旧トークンを保持します。この状態で入力欄に再フォーカスすると、onFocus（184-186行）が suggestions().length > 0 を真と判定し、ドラフトに存在しないトークンの候補メニューを開きます。
ドラフトを書き換えるときに suggestionDraft も同じ値へ更新してください。
🐛 提案する修正
requestAnimationFrame(() => {
setSelectedSuggestion(null);
- props.onDraftChange(removeActiveSearchToken(props.draft));
+ const nextDraft = removeActiveSearchToken(props.draft);
+ props.onDraftChange(nextDraft);
+ setSuggestionDraft(nextDraft);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleSelect = (suggestion: SearchSuggestion | null) => { | |
| if (!suggestion) return; | |
| props.onSelectSuggestion(suggestion); | |
| setSelectedSuggestion(suggestion); | |
| setMenuOpen(false); | |
| requestAnimationFrame(() => { | |
| setSelectedSuggestion(null); | |
| props.onDraftChange(removeActiveSearchToken(props.draft)); | |
| }); | |
| }; | |
| const handleSelect = (suggestion: SearchSuggestion | null) => { | |
| if (!suggestion) return; | |
| props.onSelectSuggestion(suggestion); | |
| setSelectedSuggestion(suggestion); | |
| setMenuOpen(false); | |
| requestAnimationFrame(() => { | |
| setSelectedSuggestion(null); | |
| const nextDraft = removeActiveSearchToken(props.draft); | |
| props.onDraftChange(nextDraft); | |
| setSuggestionDraft(nextDraft); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/v2/search-composer.tsx` around lines 80 - 89, Update
handleSelect so the draft value produced by removeActiveSearchToken(props.draft)
is also assigned to suggestionDraft when the draft changes. Keep suggestionDraft
synchronized with the value passed to props.onDraftChange, preventing stale
tokens from reopening the suggestions menu on focus.
Comment on lines
+219
to
+231
| const tokens = createMemo(() => { | ||
| const removalIds = new Set(pendingRemovals().map(tokenId)); | ||
| return [ | ||
| ...tokensFromState(searchState).filter( | ||
| (token) => !removalIds.has(tokenId(token)), | ||
| ), | ||
| ...pendingSuggestions().map((suggestion) => ({ | ||
| key: suggestion.key, | ||
| prefix: suggestion.prefix, | ||
| value: suggestion.value, | ||
| })), | ||
| ]; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
保留中の除外タグに destructive が付きません。
tokensFromState は除外タグへ destructive: true を設定します（154-197行の範囲）。しかし pendingSuggestions から生成するトークンは key、prefix、value だけを渡します。そのため -tag: の候補を選んだ直後、チップは通常色で表示されます。デバウンス適用後に赤色へ変わり、見た目が切り替わります。
🐛 提案する修正
...pendingSuggestions().map((suggestion) => ({
+ destructive: suggestion.key === "excludeTags",
key: suggestion.key,
prefix: suggestion.prefix,
value: suggestion.value,
})),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const tokens = createMemo(() => { | |
| const removalIds = new Set(pendingRemovals().map(tokenId)); | |
| return [ | |
| ...tokensFromState(searchState).filter( | |
| (token) => !removalIds.has(tokenId(token)), | |
| ), | |
| ...pendingSuggestions().map((suggestion) => ({ | |
| key: suggestion.key, | |
| prefix: suggestion.prefix, | |
| value: suggestion.value, | |
| })), | |
| ]; | |
| }); | |
| const tokens = createMemo(() => { | |
| const removalIds = new Set(pendingRemovals().map(tokenId)); | |
| return [ | |
| ...tokensFromState(searchState).filter( | |
| (token) => !removalIds.has(tokenId(token)), | |
| ), | |
| ...pendingSuggestions().map((suggestion) => ({ | |
| destructive: suggestion.key === "excludeTags", | |
| key: suggestion.key, | |
| prefix: suggestion.prefix, | |
| value: suggestion.value, | |
| })), | |
| ]; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/v2/search-toolbar.tsx` around lines 219 - 231, Update the
pendingSuggestions token mapping in the tokens memo to set destructive: true
when the suggestion represents an exclusion tag, matching the behavior
established by tokensFromState. Preserve non-exclusion suggestions’ existing
appearance and fields.
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.
概要
DESIGN.mdで定義したデザイン骨子を実アプリケーションへ移植し、V2ルートとして検索・ソース一覧・メディア個別表示・Manager・Jobs・Settingsを操作可能にします。
変更内容
コミット構成
検証
integration testはtarプロセス起動がsandboxでEPERMになったため、sandbox外で再実行して全件成功を確認しています。
補足
管理画面のさらなるコンポーネント分割は #649 で追跡します。Jobs履歴APIなどバックエンド未実装機能は、推測データを表示せずUnavailable状態として明示しています。
Summary by CodeRabbit