Conversation
Walkthroughおじさんが説明するネ✨カレンダーアイテムに Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes おじさんが詳しく見てやると、変更の複雑さはネ 🌟
Possibly related PRs
おじさんはこの変更、なかなか良い感じだと思うヨ〜 (^_^)v 一緒に確認しないカナ？ Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🚀 Deploy Preview Ready!
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/features/edit/hooks/useGetInfiniteItems.ts (1)
16-16: ページネーションロジックのバグ、直さないとダメダヨ〜🚨おじさんビックリしちゃった💦 Line 11で
limit: 60に変更したのに、Line 16のlastPage.length < 30はまだ30のままになってるヨ〜！これだと、30個以上60個未満のアイテムが返ってきた時に次のページがあると誤判定しちゃうんダ(´;ω;`)正しく動作させるには60に合わせる必要があるネ✨
🔎 おじさんの修正案
- if (lastPage.length === 0 || lastPage.length < 30) { + if (lastPage.length === 0 || lastPage.length < 60) { return undefined; // これ以上ページがない }
🧹 Nitpick comments (1)
frontend/src/routes/new.tsx (1)
89-126: アイテムグルーピングロジックに重複コードがあるヨ〜💡おじさん気づいちゃったんだけど、
photo_frame、christmas、allSeasonの3つのフィルター処理に同じようなコードが繰り返されてるネ〜🤔 型チェックのロジック（stringかarrayか）が全部同じだから、ヘルパー関数に抽出すると読みやすくなるヨ〜✨🔎 おじさんのリファクタリング案
+ // ヘルパー関数: アイテムが指定されたタイプを含むかチェック + const hasItemType = (item: Item, targetType: string): boolean => { + const type = item.type; + if (!type) return false; + if (typeof type === "string") return type.includes(targetType); + if (Array.isArray(type)) return (type as string[]).includes(targetType); + return false; + }; + // アイテムをカテゴリーごとに分類 const groupedItems = useMemo(() => { - // フォトフレーム - const photoFrame = items.filter((item) => { - const type = item.type; - if (!type) return false; - if (typeof type === "string") return type.includes("photo_frame"); - if (Array.isArray(type)) - return (type as string[]).includes("photo_frame"); - return false; - }); - // クリスマス - const christmas = items.filter((item) => { - const type = item.type; - if (!type) return false; - if (typeof type === "string") return type.includes("christmas"); - if (Array.isArray(type)) return (type as string[]).includes("christmas"); - return false; - }); + const photoFrame = items.filter((item) => hasItemType(item, "photo_frame")); + const christmas = items.filter((item) => hasItemType(item, "christmas")); // フォトフレームとクリスマス以外を「オールシーズン」として扱う const allSeason = items.filter((item) => { - const type = item.type; - if (!type) return true; - const isPhotoFrame = - (typeof type === "string" && type.includes("photo_frame")) || - (Array.isArray(type) && type.includes("photo_frame")); - const isChristmas = - (typeof type === "string" && type.includes("christmas")) || - (Array.isArray(type) && type.includes("christmas")); - return !(isPhotoFrame || isChristmas); + if (!item.type) return true; + return !(hasItemType(item, "photo_frame") || hasItemType(item, "christmas")); }); return { photoFrame, allSeason, christmas }; }, [items]);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
backend/src/routes/calendarItem.ts(1 hunks)frontend/package.json(2 hunks)frontend/src/components/ui/checkbox.tsx(1 hunks)frontend/src/features/edit/hooks/useGetInfiniteItems.ts(1 hunks)frontend/src/routes/new.tsx(8 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{tsx,jsx}: Convert Figma-generated custom Tailwind CSS values to standard Tailwind values: rounded-[24px] → rounded-3xl, gap-[32px] → gap-8, p-[12px] → p-3, text-[20px] → text-xl, etc.
Use design system colors defined in frontend/src/styles.css instead of custom hex values or rgba: use bg-primary, text-foreground, border-border, etc.
Prioritize shadcn/ui components from frontend/src/components/ui/ (Button, Input, Dialog, Field, Label, Calendar, Popover, RadioGroup, Separator, Switch, Spinner) instead of creating custom components
Remove data-node-id attributes from Figma-generated code
Remove non-standard Tailwind classes like content-stretch
Use project default font instead of custom font specifications like font-['Noto_Sans_JP:Bold',sans-serif]
Adjust absolute positioning values (left-[232px] top-[216px]) based on actual use case rather than using Figma-generated coordinates
Files:
frontend/src/components/ui/checkbox.tsxfrontend/src/routes/new.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-18T15:18:17.646Z
Learnt from: CR
Repo: AdventSphere/advent-sphere PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T15:18:17.646Z
Learning: Applies to **/*.{tsx,jsx} : Prioritize shadcn/ui components from frontend/src/components/ui/ (Button, Input, Dialog, Field, Label, Calendar, Popover, RadioGroup, Separator, Switch, Spinner) instead of creating custom components
Applied to files:
frontend/src/components/ui/checkbox.tsxfrontend/src/routes/new.tsx
🧬 Code graph analysis (2)
frontend/src/components/ui/checkbox.tsx (1)
frontend/src/lib/utils.ts (1)
cn(4-6)
frontend/src/routes/new.tsx (4)
common/generate/room/room.ts (2)
postRooms(50-62)usePostRooms(98-110)common/generate/calendar-items/calendar-items.ts (1)
usePostCalendarItemsRoomIdCalendarItems(194-206)frontend/src/features/edit/hooks/useGetInfiniteItems.ts (1)
useGetInfiniteItems(5-22)frontend/src/constants/r2-url.ts (1)
R2_BASE_URL(1-2)
🔇 Additional comments (8)
backend/src/routes/calendarItem.ts (1)
471-471: nullish coalescing でデフォルト値を安全に設定、バッチリだネ〜✨このコード、素晴らしいダヨ〜✨
isOpenedフィールドに?? falseでデフォルト値を設定するの、とってもイイ感じ💪 これでundefinedが DB に入らないから、防御的プログラミングとしても完璧ネ〜(´ω｀)ちなみにネ、フロントエンド側もちゃんと見てみたんダ〜🔍
frontend/src/routes/new.tsx(line 190) やuseItemAcquisition.ts(lines 263, 304, 339, 491) で、アイテム作成時に明示的にisOpened: trueが設定されてるヨ〜✨ だから初期アイテムはしっかりインベントリに入る状態で作成されてるんダネ(^_^)vこのコンビネーション（フロントエンドで明示的に true を設定 + バックエンドで undefined 対策）は、データ整合性を保つにはパーフェクトダヨ〜🌟 おじさん、このセンスは好きだナ〜💝
frontend/package.json (1)
15-15: 依存関係の追加、いい感じダネ〜✨おじさんチェックしたけど、
@radix-ui/react-checkboxとnanoidの追加は新しいCheckboxコンポーネントをサポートするために必要な変更だヨ〜🎵 バージョンも最新で問題ナシ！(^_^)vAlso applies to: 39-39
frontend/src/components/ui/checkbox.tsx (1)
1-28: Checkboxコンポーネント、キレイに実装されてるネ〜✨おじさん見たところ、Radix UIのprimitiveをラップして型安全に実装されてて素晴らしいヨ〜🌟 shadcn/uiのパターンにもバッチリ従ってるネ(´ω｀)
frontend/src/features/edit/hooks/useGetInfiniteItems.ts (1)
11-11: ページサイズの増加、いいネ〜✨おじさん、アイテムの取得数を30から60に増やしたの確認したヨ〜🎵 新しいアイテム選択UIでより多くのアイテムを表示できるようになるネ(^_^)v
frontend/src/routes/new.tsx (4)
3-3: カレンダーアイテム作成のAPIフック追加、ナイス〜✨おじさん感心しちゃったヨ〜🌟
usePostCalendarItemsRoomIdCalendarItemsを使って初期アイテムをフロントエンド主導で登録する仕組み、とってもスマートダネ(^_^)v バックエンドの自動作成を防いで、選択されたアイテムだけを登録できるのは素晴らしい設計だヨ〜🎵Also applies to: 85-86
49-49: default_item_idsの追加、バッチリ〜✨おじさん、フォームスキーマに
default_item_idsの配列を追加したの確認したヨ〜🎵 型安全に初期アイテムを管理できるようになってて、とってもいい感じダネ(´ω｀)Also applies to: 141-141
174-198: 初期アイテムの一括登録、素晴らしいヨ〜🎉おじさん感動しちゃった✨ ルーム作成後に
default_item_idsをループしてPromise.allで並列登録する実装、パフォーマンスも考慮されてて完璧ダネ〜🌟isOpened: trueで最初から開封済み状態にするのも、仕様通りで素晴らしいヨ(^_^)v
168-169: 空配列でバックエンドの自動作成を防ぐの、賢いネ〜🌟おじさん感心しちゃったヨ〜✨
defaultItemIds: []を明示的に渡すことでバックエンドの自動アイテム作成を防いで、フロントエンドで選択されたアイテムだけを登録する設計、とってもスマートダネ(^_^)v コメントも分かりやすくて完璧ダヨ〜🎵
| <CheckboxPrimitive.Root | ||
| data-slot="checkbox" | ||
| className={cn( | ||
| "peer border-input dark:bg-input/30 data-[state=checked]:bg-primary data-[state=checked]:text-primary-foreground dark:data-[state=checked]:bg-primary data-[state=checked]:border-primary focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive size-4 shrink-0 rounded-[4px] border shadow-xs transition-shadow outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50", |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
カスタムTailwind値を標準値に置き換えてネ〜💡
Line 15のrounded-[4px]はFigma由来のカスタム値みたいだけど、標準のTailwindクラスrounded（4pxに相当）に置き換えた方がいいヨ〜✨ コーディングガイドラインに従うとメンテしやすくなるからネ🎵
🔎 おじさんの提案する修正案
- "peer border-input dark:bg-input/30 data-[state=checked]:bg-primary data-[state=checked]:text-primary-foreground dark:data-[state=checked]:bg-primary data-[state=checked]:border-primary focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive size-4 shrink-0 rounded-[4px] border shadow-xs transition-shadow outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50",
+ "peer border-input dark:bg-input/30 data-[state=checked]:bg-primary data-[state=checked]:text-primary-foreground dark:data-[state=checked]:bg-primary data-[state=checked]:border-primary focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive size-4 shrink-0 rounded border shadow-xs transition-shadow outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50",コーディングガイドラインに基づく指摘だヨ〜📝
📝 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.
| "peer border-input dark:bg-input/30 data-[state=checked]:bg-primary data-[state=checked]:text-primary-foreground dark:data-[state=checked]:bg-primary data-[state=checked]:border-primary focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive size-4 shrink-0 rounded-[4px] border shadow-xs transition-shadow outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50", | |
| "peer border-input dark:bg-input/30 data-[state=checked]:bg-primary data-[state=checked]:text-primary-foreground dark:data-[state=checked]:bg-primary data-[state=checked]:border-primary focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive size-4 shrink-0 rounded border shadow-xs transition-shadow outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50", |
🤖 Prompt for AI Agents
frontend/src/components/ui/checkbox.tsx around line 15: the class string uses a
custom Tailwind value `rounded-[4px]` which maps to the standard `rounded`
(4px); replace `rounded-[4px]` with the standard `rounded` class to follow the
coding guidelines and ensure consistency/maintainability across the codebase.
| {groupedItems.christmas.length > 0 && ( | ||
| <div className="mx-2-"> | ||
| <button | ||
| type="button" | ||
| className="flex items-center gap-2 font-bold text-sm mb-1 focus:outline-none" | ||
| onClick={() => setOpenChristmas((v) => !v)} | ||
| aria-expanded={openChristmas} | ||
| > | ||
| <span>クリスマス</span> | ||
| <span | ||
| className={ | ||
| openChristmas | ||
| ? "rotate-90 transition-transform" | ||
| : "transition-transform" | ||
| } | ||
| > | ||
| ▷ | ||
| </span> | ||
| </button> | ||
| {openChristmas && ( | ||
| <div className="flex flex-wrap gap-3 mt-1"> | ||
| {groupedItems.christmas.map((item) => ( | ||
| <label | ||
| key={item.id} | ||
| className="flex items-center gap-2 cursor-pointer select-none" | ||
| htmlFor={`item-${item.id}`} | ||
| > | ||
| <Checkbox | ||
| checked={watchedValues.default_item_ids.includes( | ||
| item.id, | ||
| )} | ||
| onCheckedChange={(checked) => { | ||
| const arr = watchedValues.default_item_ids || []; | ||
| if (checked) { | ||
| setValue("default_item_ids", [...arr, item.id]); | ||
| } else { | ||
| setValue( | ||
| "default_item_ids", | ||
| arr.filter((id) => id !== item.id), | ||
| ); | ||
| } | ||
| }} | ||
| id={`item-${item.id}`} | ||
| /> | ||
| <input | ||
| type="checkbox" | ||
| id={`item-${item.id}`} | ||
| checked={watchedValues.default_item_ids.includes( | ||
| item.id, | ||
| )} | ||
| onChange={(e) => { | ||
| const checked = e.target.checked; | ||
| const arr = watchedValues.default_item_ids || []; | ||
| if (checked) { | ||
| setValue("default_item_ids", [...arr, item.id]); | ||
| } else { | ||
| setValue( | ||
| "default_item_ids", | ||
| arr.filter((id) => id !== item.id), | ||
| ); | ||
| } | ||
| }} | ||
| className="hidden" | ||
| /> | ||
| <span className="flex items-center gap-2"> | ||
| <span className="w-8 h-8 rounded overflow-hidden bg-muted border shrink-0"> | ||
| <img | ||
| src={`${R2_BASE_URL}/item/thumbnail/${item.id}.png`} | ||
| alt={item.name} | ||
| className="w-full h-full object-cover" | ||
| /> | ||
| </span> | ||
| <span className="font-medium text-sm"> | ||
| {item.name} | ||
| </span> | ||
| </span> | ||
| </label> | ||
| ))} | ||
| </div> | ||
| )} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
フォトフレームカテゴリーが表示されてないヨ〜🤔
おじさん気づいちゃったんだけどネ💦 Lines 95-104でphotoFrameグループを作成してるのに、UIには「クリスマス」と「その他」しか表示されてないんダ〜😱 せっかくグルーピングしたのに、フォトフレームアイテムが選べないのはもったいないヨ〜！
もしフォトフレームを表示したくないなら、グルーピングロジックから削除した方がいいし、表示したいなら同じようなセクションを追加する必要があるネ✨
🔎 フォトフレームセクションを追加する場合の実装例
クリスマスセクション（lines 546-627）の前に以下を追加するといいヨ〜：
{/* フォトフレーム */}
{groupedItems.photoFrame.length > 0 && (
<div className="mb-2">
<button
type="button"
className="flex items-center gap-2 font-bold text-sm mb-1 focus:outline-none"
onClick={() => setOpenPhotoFrame((v) => !v)}
aria-expanded={openPhotoFrame}
>
<span>フォトフレーム</span>
<span
className={
openPhotoFrame
? "rotate-90 transition-transform"
: "transition-transform"
}
>
▷
</span>
</button>
{openPhotoFrame && (
<div className="flex flex-wrap gap-3 mt-1">
{groupedItems.photoFrame.map((item) => (
// 既存のchristmas/allSeasonと同じitem rendering logic
))}
</div>
)}
</div>
)}また、stateの追加も必要ダヨ：
const [openChristmas, setOpenChristmas] = useState(false);
const [openAllSeason, setOpenAllSeason] = useState(false);
+ const [openPhotoFrame, setOpenPhotoFrame] = useState(false);Also applies to: 629-710
🤖 Prompt for AI Agents
frontend/src/routes/new.tsx around lines 546-627 (also applies to 629-710): the
review points out that you build a photoFrame group earlier (lines ~95-104) but
never render it in the UI, so add a photoFrame section just like the Christmas
block — create a piece of state (e.g., openPhotoFrame) and a toggle button, then
render groupedItems.photoFrame.map using the same item rendering logic (Checkbox
+ hidden input + thumbnail/name) as the Christmas block; alternatively, if
photoFrame should not be shown, remove it from the grouping logic where it's
created.
| <Checkbox | ||
| checked={watchedValues.default_item_ids.includes( | ||
| item.id, | ||
| )} | ||
| onCheckedChange={(checked) => { | ||
| const arr = watchedValues.default_item_ids || []; | ||
| if (checked) { | ||
| setValue("default_item_ids", [...arr, item.id]); | ||
| } else { | ||
| setValue( | ||
| "default_item_ids", | ||
| arr.filter((id) => id !== item.id), | ||
| ); | ||
| } | ||
| }} | ||
| id={`item-${item.id}`} | ||
| /> | ||
| <input | ||
| type="checkbox" | ||
| id={`item-${item.id}`} | ||
| checked={watchedValues.default_item_ids.includes( | ||
| item.id, | ||
| )} | ||
| onChange={(e) => { | ||
| const checked = e.target.checked; | ||
| const arr = watchedValues.default_item_ids || []; | ||
| if (checked) { | ||
| setValue("default_item_ids", [...arr, item.id]); | ||
| } else { | ||
| setValue( | ||
| "default_item_ids", | ||
| arr.filter((id) => id !== item.id), | ||
| ); | ||
| } | ||
| }} | ||
| className="hidden" | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
隠しcheckbox inputが不要ダヨ〜💡
おじさん気づいちゃったんだけどネ🤔 Lines 590-609とLines 673-691で、<Checkbox>コンポーネントの後にtype="checkbox"の隠しinputを重複して配置してるけど、これは不要ダヨ〜✨
<Checkbox>コンポーネント（Radix UI）自体が内部で適切なaria属性とinputを管理してくれるから、追加の隠しinputは冗長なんダ(´ω｀) しかも同じidを2つの要素に割り当ててるのはHTMLとしても問題があるヨ〜💦
シンプルに<Checkbox>だけで十分だから、隠しinputは削除しちゃおうネ🎵
🔎 おじさんの修正案（クリスマスセクション）
<Checkbox
checked={watchedValues.default_item_ids.includes(
item.id,
)}
onCheckedChange={(checked) => {
const arr = watchedValues.default_item_ids || [];
if (checked) {
setValue("default_item_ids", [...arr, item.id]);
} else {
setValue(
"default_item_ids",
arr.filter((id) => id !== item.id),
);
}
}}
id={`item-${item.id}`}
/>
- <input
- type="checkbox"
- id={`item-${item.id}`}
- checked={watchedValues.default_item_ids.includes(
- item.id,
- )}
- onChange={(e) => {
- const checked = e.target.checked;
- const arr = watchedValues.default_item_ids || [];
- if (checked) {
- setValue("default_item_ids", [...arr, item.id]);
- } else {
- setValue(
- "default_item_ids",
- arr.filter((id) => id !== item.id),
- );
- }
- }}
- className="hidden"
- />
<span className="flex items-center gap-2">オールシーズンセクションも同様に修正してネ〜✨
Also applies to: 656-691
🤖 Prompt for AI Agents
In frontend/src/routes/new.tsx around lines 573-609 (and duplicate case around
656-691), there are redundant hidden <input type="checkbox"> elements placed
immediately after the Radix <Checkbox> component, causing duplicate ids and
unnecessary DOM/input duplication; remove the hidden input elements so only the
<Checkbox> component remains, ensure the id is only used on the Checkbox, and
keep the same checked/onChange logic wired to the form state via the Checkbox
props (verify no remaining duplicate ids and run accessibility checks).
2 participants
close #46
Summary by CodeRabbit
リリースノート
新機能
パフォーマンス改善
✏️ Tip: You can customize this high-level summary in your review settings.