-
Notifications
You must be signed in to change notification settings - Fork 4
✨feat: 공통 컴포넌트 인풋 구현 #26
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
Conversation
|
@jeschun is attempting to deploy a commit to the projects Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
고생하셨습니다 :) 코멘트 확인해주세요!
그리고 PR 제목이 모달로 되어있어서 인풋으로 변경이 필요해보입니다!
src/components/ui/input/input.tsx
Outdated
| <label htmlFor={id} className='text-[var(--black)]/80 text-sm'> | ||
| {label} |
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.
text-sm 는 테일윈드에서 자체적으로 사용하는 값으로 보여지는데요!
이것을 사용하셔도 괜찮지만 line-height , letter-space(있는경우) 적용 같이 부탁드립니다.
현재 폰트셋은 globals.css + tailwind.config 에 선언되어있어서 참고해서 사용해주시면 일관성있는 텍스트가 보여질것 같습니다
(참고) 현재 base-input 은 tailwind config 에 있는 폰트로 선언되어있습니다
또한 컬러는 text-black 으로 확인되는데 /80을 주신 이유가 있으실까요!?
src/components/ui/input/input.tsx
Outdated
| /> | ||
|
|
||
| {suffix && ( | ||
| <span className='pointer-events-none absolute right-5 top-1/2 -translate-y-1/2 text-sm text-[var(--gray-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.
text-[var(--gray-500)] 컬러 사용할때 tailwind config 에 전부 정의가 되어 있으므로 text-gray-500 으로 사용하셔도 동일하게 나오기 떄문에 불편하게 var 작성 안하셔도 될것 같습니다 :)
src/components/ui/input/input.tsx
Outdated
| </div> | ||
|
|
||
| {hasError && ( | ||
| <p id={errorId} className='text-xs text-[var(--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.
이부분도 text-red-500 으로 사용 가능할것 같습니다 :) 그리고 text-xs 또한 위의 코멘트 참고 부탁드립니다
src/components/ui/input/input.tsx
Outdated
| className={[ | ||
| 'base-input', // ← 전역 공통 | ||
| 'w-full outline-none transition', | ||
| hasError ? 'border-[var(--red-500)]' : '', | ||
| hasError ? '' : 'focus:border-[var(--red-500)]', | ||
| isDisabled ? 'cursor-not-allowed bg-[var(--gray-100)]' : '', | ||
| suffix ? 'pr-10' : '', // md 드롭다운과 동일한 오른쪽 여백 | ||
| ].join(' ')} | ||
| /> |
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.
이런경우 cn 라이브러리를 사용하면 조금더 편하게 사용할 수 있습니다 cn() 함수 안에 , 로 구분해서 넣게되면 .join('') 과 같은 문자열 조합을 하지 않아도 됩니다.
또한 foo ? value : "" 조합 에서 foo && value 조합으로 변경하여 쉽게 사용할 수 있습니다 ( 빈값을 사용하지 않아도 됩니다)
className={cn(
'base-input w-full outline-none transition',
hasError && 'border-red-500',
!hasError && 'focus:border-red-500',
isDisabled && 'cursor-not-allowed bg-gray-100',
suffix && 'pr-10'
)}
|
피드백 주신대로 고쳐봤습니다! |
📝 작업 개요 (필수)
폼 일관성 및 접근성 강화를 위한 Input 컴포넌트 정리(라벨/에러/사이즈/다크모드).
✨ 작업 내용 (필수)
📸 스크린샷
🧐 해결해야 하는 문제
🤔 리뷰어 확인 필요 사항
🔗 관련 이슈
🛠️ 후속 작업
✅ 체크리스트 (필수)