-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FEAT-입력컴포넌트를 생성한다. #25
base: develop
Are you sure you want to change the base?
The head ref may contain hidden characters: "feat/jackihyun/\uC785\uB825\uCEF4\uD3EC\uB10C\uD2B8\uB97C-\uC0DD\uC131\uD55C\uB2E4"
Conversation
✅ Deploy Preview for strong-cupcake-4268bc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
굿굿~
주석은 지워줘도 될 것 같어!
그리고 Input도 시간날 때 따로 Atom/Input 으로 스토리 만들어두면 좋을 듯!
나중에 Form이 다른 형태로 쓰일 수도 있으니!
src/components/Input/LabelInput.tsx
Outdated
...props | ||
}) => { | ||
return ( | ||
<div className={cn(`(LabelInputVariant) w-full`, className)} {...props}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기 (LabelInputVariant) 깜빡하구 못 지운듯?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
맞아여,,
src/components/Input/LabelInput.tsx
Outdated
|
||
const LabelInputVariant = cva('w-full text-left', { | ||
variants: { | ||
variants: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
별 거 아니긴 한데 요기 단수로 네이밍하는 게 좋을 것 같음!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variant? 말씀하시는 거져??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅇㅇㅇ맞으 위에 Input은 variant 이길래 살짝 일관되게 해주면 좋을 것 같아서👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵
src/components/Input/LabelInput.tsx
Outdated
variants: { | ||
primary: 'mb-1.5 font-normal text-gray-1', | ||
}, | ||
labelsize: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기는 camelCase 깜빡한듯
@@ -25,5 +25,4 @@ const RootLayout: React.FC<RootLayoutProps> = ({ children }) => { | |||
</html> | |||
); | |||
}; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기 공백은 실수??로 지운거지??ㅋㅋ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아...그런거 같아여...
type: 'text', | ||
placeholder: 'Input with error', | ||
error: 'This field is required', | ||
className: 'border-red-500', // 에러 시 빨간 테두리 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어차피 나중에 디자인 싹 ㅂ바뀔거라서 상관없긴 한데 바깥에서 Input 컴포넌트 사용할 때에는 스타일 색상 관련해서 className보다는 props으로만 하는 게 좋음
<label className={cn(LabelInputVariant({ labelSize }))}>{label}</label> | ||
<Input | ||
type={type} | ||
size={size} | ||
rounded={rounded} | ||
error={error} | ||
placeholder={placeholder} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
놓치고 확인을 못 했는데 htmlFor 로 Input 태그랑 연결해주느 게 좋음. 이때는 useId 써도 좋을듯
📌 PR Summary
🤔 NOTE
🔗 Related Issues
x