Feat/tag #18
Feat/tag #18
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
minimoku | 65071d8 | Dec 27 2025, 01:00 PM |
📝 WalkthroughWalkthroughEnhanced the link feature with improved validation, tag array support, and form error handling. Introduced a new FormTagInput component for managing tags with IME composition awareness. Expanded the Dialog component's API with close-behavior controls and new exports. Updated type signatures in the image upload hook. Added documentation for Claude Code usage. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shared/hooks/useUploadImage.ts (1)
52-54: Typo in error message.Minor typo: "ClouadFlare" should be "Cloudflare".
🔎 Proposed fix
- throw new Error("ClouadFlare Error : image upload failed."); + throw new Error("Cloudflare Error: image upload failed.");
🧹 Nitpick comments (4)
CLAUDE.md (2)
24-33: Add language specifier to fenced code block.The directory structure code block is missing a language specifier. Consider using
textorplaintextfor non-code content to satisfy linting rules.🔎 Proposed fix
-``` +```text src/ ├── app/ # Next.js App Router 페이지 & API 라우트
39-47: Add language specifier to fenced code block.Same issue as above—add a language specifier like
textfor the feature structure diagram.🔎 Proposed fix
-``` +```text features/{feature}/ ├── model/src/features/link/model/services/links.service.ts (1)
65-72: Tag parsing logic looks correct.The JSON parsing with silent fallback to an empty array is a reasonable defensive approach. However, consider whether silently ignoring malformed JSON is desired—if a client sends invalid JSON, it might indicate a bug worth logging.
🔎 Optional: Log parse failures for debugging
try { parsedTags = tagsString ? JSON.parse(tagsString) : []; } catch { + console.warn("Failed to parse tags JSON:", tagsString); parsedTags = []; }src/shared/components/molecules/FormTagInput.tsx (1)
98-103: Consider adding accessibility attributes to remove buttons.The remove button lacks an accessible label. Screen reader users won't know which tag they're removing.
🔎 Proposed fix
<button type="button" onClick={() => removeTag(index)} + aria-label={`${tag} 태그 제거`} className="hover:bg-minimoku-neutral ml-0.5 rounded-sm p-0.5 transition-colors"> <X className="h-3 w-3" /> </button>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CLAUDE.mdsrc/features/link/model/schema/linkSchema.tssrc/features/link/model/services/links.service.tssrc/features/link/ui/LinkAddModal.tsxsrc/shared/components/atoms/dialog.tsxsrc/shared/components/molecules/FormTagInput.tsxsrc/shared/hooks/useUploadImage.ts
🧰 Additional context used
🧬 Code graph analysis (3)
src/shared/components/molecules/FormTagInput.tsx (4)
src/shared/lib/utils/commonUtils.ts (1)
cn(5-7)src/shared/home/atomic/Typography.tsx (1)
Typography(62-71)src/shared/components/molecules/FormInput.tsx (2)
T(15-49)InputProps(8-13)src/shared/components/molecules/FormSelect.tsx (3)
FormSelectProps(14-21)T(23-71)Select(50-64)
src/features/link/ui/LinkAddModal.tsx (2)
src/features/link/model/schema/linkSchema.ts (1)
LinkSchemaType(22-22)src/features/link/model/services/links.service.ts (1)
postLink(59-113)
src/shared/components/atoms/dialog.tsx (1)
src/shared/lib/utils/commonUtils.ts (1)
cn(5-7)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Workers Builds: minimoku
🔇 Additional comments (14)
src/shared/hooks/useUploadImage.ts (1)
11-12: LGTM!Good improvement to type safety by specifying
stringinstead ofanyfor theidparameter. This aligns with how the ID is used to construct Cloudflare image URLs.src/features/link/model/services/links.service.ts (1)
74-103: LGTM!The tag data flows correctly from parsed JSON to schema validation to database write. The integration with the updated
linkSchema(which expectsz.array(z.string())) is consistent.src/features/link/ui/LinkAddModal.tsx (3)
42-42: LGTM!Correct initialization of
tagsas an empty array to match the updated schema typez.array(z.string()).
64-66: LGTM!The tag serialization using
JSON.stringify(data.tags ?? [])correctly matches the server-sideJSON.parseinpostLink.
98-101: Good UX improvement.Disabling escape and outside-click closing prevents accidental data loss during form entry. This aligns with the PR objective of fixing the backdrop removal issue when the select was open.
src/shared/components/molecules/FormTagInput.tsx (3)
36-47: LGTM!The
addTagfunction correctly handles trimming, duplicate prevention, and form state updates with validation. Good use ofuseCallbackfor memoization.
60-70: LGTM!Good handling of keyboard events with proper IME composition awareness. Preventing default on Enter/Space ensures the form isn't submitted and spaces don't appear in the input.
77-83: LGTM!Correct IME composition end handling—adding the tag when composition ends with a space is a nice touch for Korean/Chinese users.
src/features/link/model/schema/linkSchema.ts (3)
4-9: LGTM!Good schema improvements: trimming before validation ensures whitespace-only input is rejected. The validation order (trim → min → url) is correct.
18-18: LGTM!Changing
tagsfrom an optional string toz.array(z.string())properly reflects the new tag data model. This aligns with theFormTagInputcomponent output and the service layer's JSON parsing.
22-22: LGTM!The exported
LinkSchemaTypeprovides type safety for form handling inLinkAddModal.tsxand other consumers.src/shared/components/atoms/dialog.tsx (3)
31-37: LGTM!Clean interface extension for the disable flags. The Korean comments clearly document the purpose of each prop.
64-75: LGTM!Correct implementation of close behavior control. Handling both
onPointerDownOutsideandonInteractOutsideensures complete coverage for outside interactions. The pattern of callingpreventDefault()when disabled while still forwarding to user-provided handlers is the right approach.
125-136: LGTM!Good expansion of the public API surface. Exporting
DialogPortal,DialogOverlay, andDialogCloseprovides flexibility for advanced usage patterns.
| // 에러가 있으면 각 필드에 에러 메시지 설정 | ||
| if (errors) { | ||
| // Zod flatten 형태의 에러 처리 | ||
| if ("fieldErrors" in errors) { | ||
| Object.entries(errors.fieldErrors).forEach(([field, messages]) => { | ||
| console.log("field:", field, "messages:", messages); | ||
| if (messages && messages.length > 0) { | ||
| setError(field as keyof LinkSchemaType, {message: messages[0]}); | ||
| } | ||
| }); | ||
| } else { | ||
| // 단순 객체 형태의 에러 처리 (예: {folderId: "존재하지 않는 폴더입니다."}) | ||
| Object.entries(errors).forEach(([field, message]) => { | ||
| setError(field as keyof LinkSchemaType, {message: message as string}); | ||
| }); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
Remove debug console.log statement.
Line 75 contains a console.log that appears to be debug code. Remove it before merging.
🔎 Proposed fix
if ("fieldErrors" in errors) {
Object.entries(errors.fieldErrors).forEach(([field, messages]) => {
- console.log("field:", field, "messages:", messages);
if (messages && messages.length > 0) {
setError(field as keyof LinkSchemaType, {message: messages[0]});
}
});📝 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.
| // 에러가 있으면 각 필드에 에러 메시지 설정 | |
| if (errors) { | |
| // Zod flatten 형태의 에러 처리 | |
| if ("fieldErrors" in errors) { | |
| Object.entries(errors.fieldErrors).forEach(([field, messages]) => { | |
| console.log("field:", field, "messages:", messages); | |
| if (messages && messages.length > 0) { | |
| setError(field as keyof LinkSchemaType, {message: messages[0]}); | |
| } | |
| }); | |
| } else { | |
| // 단순 객체 형태의 에러 처리 (예: {folderId: "존재하지 않는 폴더입니다."}) | |
| Object.entries(errors).forEach(([field, message]) => { | |
| setError(field as keyof LinkSchemaType, {message: message as string}); | |
| }); | |
| } | |
| return; | |
| } | |
| // 에러가 있으면 각 필드에 에러 메시지 설정 | |
| if (errors) { | |
| // Zod flatten 형태의 에러 처리 | |
| if ("fieldErrors" in errors) { | |
| Object.entries(errors.fieldErrors).forEach(([field, messages]) => { | |
| if (messages && messages.length > 0) { | |
| setError(field as keyof LinkSchemaType, {message: messages[0]}); | |
| } | |
| }); | |
| } else { | |
| // 단순 객체 형태의 에러 처리 (예: {folderId: "존재하지 않는 폴더입니다."}) | |
| Object.entries(errors).forEach(([field, message]) => { | |
| setError(field as keyof LinkSchemaType, {message: message as string}); | |
| }); | |
| } | |
| return; | |
| } |
🤖 Prompt for AI Agents
In src/features/link/ui/LinkAddModal.tsx around lines 70 to 87, there is a
leftover debug console.log at line 75 that prints field and messages; remove
that console.log statement so no debug output remains in production, keeping the
surrounding error-handling logic (the Object.entries loop and setError calls)
unchanged.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.