Conversation
7 tasks
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds new shared UI primitives: form components, a Checkbox, file uploader components (including thumbnail preview and file list), Storybook stories for these components, three SVG icons, and updates the shared UI barrel exports. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Uploader as FileUploader component
participant Input as Hidden <input type="file">
participant Consumer as onFileSelect handler
User->>Uploader: click / drop files
Uploader->>Input: open file picker / receive drop
Input-->>Uploader: FileList selected
Uploader->>Uploader: convert FileList -> File[]
Uploader->>Consumer: onFileSelect(files)
Consumer-->>Uploader: (handles files)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
src/shared/ui/checkbox/checkbox.stories.tsx (1)
19-20: Story control state can desync fromargs.checked.At Line [19], local state is initialized once. If controls change
checked, the rendered checkbox may not follow.🧪 Proposed fix
-import { useState } from "react"; +import { useEffect, useState } from "react"; ... const InteractiveCheckbox = (args: CheckboxProps) => { const [checked, setChecked] = useState(args.checked || false); + useEffect(() => { + setChecked(Boolean(args.checked)); + }, [args.checked]); return <Checkbox {...args} checked={checked} onChange={(e) => setChecked(e.target.checked)} />; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/checkbox/checkbox.stories.tsx` around lines 19 - 20, The story initializes local state with useState(args.checked || false) which won't update when controls change args.checked; in the story component (where useState, checked, setChecked and <Checkbox {...args} checked={checked} onChange=...} are used) replace or augment the state with a synchronization step: add an effect that watches args.checked and calls setChecked(args.checked) (or use args.checked directly to render the Checkbox as a controlled prop) so the rendered Checkbox follows storybook control changes.src/shared/ui/file-uploader/file-uploader.stories.tsx (1)
47-66: Combined story currently can’t validate the core interaction flow.At Line 54, Line 62, and Line 65, handlers are no-ops, so upload/remove paths aren’t actually exercised in this “combined” scenario. Consider wiring minimal local state here too (like in
Thumbnail) so this story verifies behavior, not just layout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/file-uploader/file-uploader.stories.tsx` around lines 47 - 66, The CombinedUploader story uses no-op handlers so upload/remove flows aren't exercised; change the story to hold minimal local React state (e.g., thumbnail state and an array of files) and implement the handlers passed to ThumbnailUploader (onUpload, onRemove) and FileUploader (onFileSelect) and FileListItem (onRemove) to update that state (add/remove items) so the combined story actually demonstrates interaction and state changes for the components ThumbnailUploader, FileUploader, and FileListItem.src/shared/ui/form/form.stories.tsx (1)
100-100:FileUploaderis rendered but add-file behavior is not demonstrated.At Line 100,
onFileSelect={() => {}}makes the integrated form story partially non-functional. Hooking this tosetFileswould make the Storybook scenario much more useful for regression checks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/form/form.stories.tsx` at line 100, The FileUploader in the story is rendered with a no-op onFileSelect which prevents the story from demonstrating add-file behavior; update the story to connect FileUploader's onFileSelect to the story's state updater (e.g., call setFiles or the story args setter) so selected files update the component state—locate the FileUploader usage in the form story (FileUploader onFileSelect prop) and replace the empty handler with a call that forwards the selected files to setFiles (or the appropriate state/args setter) to enable interactive file-add behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/shared/ui/checkbox/checkbox.tsx`:
- Around line 13-17: The visible checkbox lacks a keyboard focus indicator:
update the inputHiddenClasses constant to include the "peer" class so the hidden
input can drive peer styles, and update boxBaseClasses to add appropriate
"peer-focus-visible:..." utilities (e.g. peer-focus-visible:ring-2
peer-focus-visible:ring-offset-1 peer-focus-visible:ring-primary or your design
tokens) so the visible box shows a focus ring when keyboard-focused; modify the
constants inputHiddenClasses and boxBaseClasses accordingly to enable a clear
focus-visible state.
- Around line 50-64: The Checkbox component currently only derives visual state
from the checked prop, breaking uncontrolled usage; update Checkbox (the
React.forwardRef function) to support controlled and uncontrolled modes by
deriving an internal state (e.g., localChecked) initialized from
props.defaultChecked when props.checked is undefined, use a computed value
(controlledChecked = props.checked ?? localChecked) for getBoxClasses and
CheckIcon, and wire the input's onChange to update localChecked and call any
provided props.onChange so both native input state and visual state stay in
sync; ensure you only pass a checked attribute to the <input> when in controlled
mode and otherwise let the input manage its own defaultChecked.
In `@src/shared/ui/file-uploader/file-uploader.tsx`:
- Around line 101-103: The UI advertises drag-and-drop but the FileUploader
component lacks any drag handlers; add drag support by wiring
onDragOver/onDragEnter/onDragLeave/onDrop on the upload wrapper inside
file-uploader.tsx, preventDefault in those handlers, extract files from
event.dataTransfer.files in onDrop and pass them to the same file-processing
routine used by the input change handler (e.g., the existing handleFileSelect or
onFilesSelected function), and optionally track an isDragging state to toggle a
visual class while dragging; alternatively, if you prefer click-only, change the
copy in the <p> text to remove "드래그하거나" so it only mentions clicking.
- Around line 57-60: The file input change handlers (handleFileChange) in
ThumbnailUploader and FileUploader should reset the input value after processing
so selecting the same file again will fire change; update both handleFileChange
functions to set e.currentTarget.value = "" after calling onUpload(file) (or
after the early-return logic) to ensure the input is cleared for subsequent
identical-file uploads.
- Around line 85-92: The upload trigger uses a non-semantic div with onClick
(see getUploaderClasses, handleClick, fileInputRef, handleFileChange) and must
be keyboard-accessible: add role="button", tabIndex={0}, and a descriptive
aria-label to the div, and implement an onKeyDown handler that invokes the same
behavior as handleClick when Enter or Space is pressed; apply the identical
changes to the other upload-trigger div used elsewhere in this component so both
triggers support keyboard and assistive technology.
In `@src/shared/ui/form/form.stories.tsx`:
- Around line 92-97: The FileListItem onRemove handler closes over the current
files array causing potential stale state; change the setFiles call inside the
FileListItem prop to the functional updater form so updates use the latest state
(replace setFiles(files.filter(f => f.id !== id)) with setFiles(prev =>
prev.filter(f => f.id !== id))). Update the code where FileListItem is rendered
(the files map and onRemove prop) to use this functional update.
In `@src/shared/ui/form/form.tsx`:
- Around line 9-13: The FormLabel currently always renders a <label> even when
htmlFor is undefined; update the component that uses the FormLabelProps (the
FormLabel render function/component) to render a <span> instead of a <label>
when props.htmlFor is falsy, preserving all children, required indicator and
className handling; keep behavior unchanged when htmlFor is provided (render a
<label htmlFor={htmlFor}>), and ensure typings (FormLabelProps) remain the same
so this change is non-breaking.
---
Nitpick comments:
In `@src/shared/ui/checkbox/checkbox.stories.tsx`:
- Around line 19-20: The story initializes local state with
useState(args.checked || false) which won't update when controls change
args.checked; in the story component (where useState, checked, setChecked and
<Checkbox {...args} checked={checked} onChange=...} are used) replace or augment
the state with a synchronization step: add an effect that watches args.checked
and calls setChecked(args.checked) (or use args.checked directly to render the
Checkbox as a controlled prop) so the rendered Checkbox follows storybook
control changes.
In `@src/shared/ui/file-uploader/file-uploader.stories.tsx`:
- Around line 47-66: The CombinedUploader story uses no-op handlers so
upload/remove flows aren't exercised; change the story to hold minimal local
React state (e.g., thumbnail state and an array of files) and implement the
handlers passed to ThumbnailUploader (onUpload, onRemove) and FileUploader
(onFileSelect) and FileListItem (onRemove) to update that state (add/remove
items) so the combined story actually demonstrates interaction and state changes
for the components ThumbnailUploader, FileUploader, and FileListItem.
In `@src/shared/ui/form/form.stories.tsx`:
- Line 100: The FileUploader in the story is rendered with a no-op onFileSelect
which prevents the story from demonstrating add-file behavior; update the story
to connect FileUploader's onFileSelect to the story's state updater (e.g., call
setFiles or the story args setter) so selected files update the component
state—locate the FileUploader usage in the form story (FileUploader onFileSelect
prop) and replace the empty handler with a call that forwards the selected files
to setFiles (or the appropriate state/args setter) to enable interactive
file-add 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 817a42ff-1393-41bd-a586-3e106d9717d4
📒 Files selected for processing (11)
src/shared/ui/checkbox/checkbox.stories.tsxsrc/shared/ui/checkbox/checkbox.tsxsrc/shared/ui/checkbox/index.tssrc/shared/ui/file-uploader/file-uploader.stories.tsxsrc/shared/ui/file-uploader/file-uploader.tsxsrc/shared/ui/file-uploader/index.tssrc/shared/ui/form/form.stories.tsxsrc/shared/ui/form/form.tsxsrc/shared/ui/form/index.tssrc/shared/ui/icons/index.tsxsrc/shared/ui/index.ts
Comment on lines
+50
to
+64
| export const Checkbox = React.forwardRef<HTMLInputElement, CheckboxProps>( | ||
| ({ label, checked, disabled, className = "", ...props }, ref) => { | ||
| return ( | ||
| <label className={[containerBaseClasses, className].join(" ")}> | ||
| <input | ||
| type="checkbox" | ||
| ref={ref} | ||
| checked={checked} | ||
| disabled={disabled} | ||
| className={inputHiddenClasses} | ||
| {...props} | ||
| /> | ||
| <div className={getBoxClasses(checked, disabled)}> | ||
| <CheckIcon size={14} className={checked ? "text-white" : "text-transparent"} /> | ||
| </div> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/shared/ui/checkbox/checkbox.tsxRepository: ajou-industry-matching/aim-frontend
Length of output: 2768
🏁 Script executed:
rg -A 10 "type CheckboxProps" src/shared/ui/checkbox/Repository: ajou-industry-matching/aim-frontend
Length of output: 1524
Support both controlled and uncontrolled modes for the Checkbox component.
Visual state breaks when using uncontrolled mode (defaultChecked without checked). The component accepts defaultChecked via React.InputHTMLAttributes, but the visual state (box and icon) only depends on the checked prop. When checked is undefined, the UI remains visually unchecked while the native input tracks its own state independently.
Implement the controlled/uncontrolled pattern to handle both cases:
🔧 Proposed fix
export const Checkbox = React.forwardRef<HTMLInputElement, CheckboxProps>(
- ({ label, checked, disabled, className = "", ...props }, ref) => {
+ ({ label, checked, defaultChecked, disabled, className = "", onChange, ...props }, ref) => {
+ const isControlled = checked !== undefined;
+ const [internalChecked, setInternalChecked] = React.useState(Boolean(defaultChecked));
+ const isChecked = isControlled ? checked : internalChecked;
+
+ const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
+ if (!isControlled) setInternalChecked(e.target.checked);
+ onChange?.(e);
+ };
+
return (
<label className={[containerBaseClasses, className].join(" ")}>
<input
type="checkbox"
ref={ref}
- checked={checked}
+ checked={isChecked}
disabled={disabled}
className={inputHiddenClasses}
+ onChange={handleChange}
{...props}
/>
- <div className={getBoxClasses(checked, disabled)}>
- <CheckIcon size={14} className={checked ? "text-white" : "text-transparent"} />
+ <div className={getBoxClasses(isChecked, disabled)}>
+ <CheckIcon size={14} className={isChecked ? "text-white" : "text-transparent"} />
</div>
{label && <span className={getLabelClasses(disabled)}>{label}</span>}
</label>
);
},
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/checkbox/checkbox.tsx` around lines 50 - 64, The Checkbox
component currently only derives visual state from the checked prop, breaking
uncontrolled usage; update Checkbox (the React.forwardRef function) to support
controlled and uncontrolled modes by deriving an internal state (e.g.,
localChecked) initialized from props.defaultChecked when props.checked is
undefined, use a computed value (controlledChecked = props.checked ??
localChecked) for getBoxClasses and CheckIcon, and wire the input's onChange to
update localChecked and call any provided props.onChange so both native input
state and visual state stay in sync; ensure you only pass a checked attribute to
the <input> when in controlled mode and otherwise let the input manage its own
defaultChecked.
sebeeeen
reviewed
Mar 28, 2026
Comment on lines
+4
to
+12
| import { Input, Textarea } from "@/shared/ui/inputBox/inputBox"; | ||
| import { Checkbox } from "@/shared/ui/checkbox/checkbox"; | ||
| import { | ||
| ThumbnailUploader, | ||
| FileListItem, | ||
| FileUploader, | ||
| type FileItem, | ||
| } from "@/shared/ui/file-uploader/file-uploader"; | ||
| import { Button } from "@/shared/ui/button/button"; |
Member
There was a problem hiding this comment.
내부 파일을 직접 참조하면 내부 구현이 변경될 때 import 경로가 깨질 수 있어 아래와 같이 index.ts(public API)를 통해 import하는 방식을 제안드립니다~!
import { Input, Textarea } from "@/shared/ui/inputBox";
import { Checkbox } from "@/shared/ui/checkbox";
import { ThumbnailUploader, ... } from "@/shared/ui/file-uploader";
import { Button } from "@/shared/ui/button";
This was referenced Apr 1, 2026
5 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
2 participants
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.
[Feat] #64 - 포트폴리오 작성용 공통 폼(Form) 컴포넌트군 구현
🔎 What is this PR?
서비스 내 다양한 입력 폼에서 공통으로 사용할 수 있는 표준화된 폼 UI 컴포넌트 세트를 구현했습니다. 특히 포트폴리오 작성
페이지의 복잡한 구조를 FSD 아키텍처 가이드에 맞춰 효율적으로 구축할 수 있도록 설계했습니다.
📝 Changes
📸 Screenshots
📚 Background / Context
함.
✔ Checklist
🙏 Request
Summary by CodeRabbit
New Features
Documentation