-
Notifications
You must be signed in to change notification settings - Fork 1
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] 텍스트박스 공통컴포넌트 퍼블리싱 #30
Conversation
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.
너무 깔끔하게 코드를 잘 짜주셔서 모든 코드가 직관적으로 잘 이해가 되었습니다. 빠른 작업 하시느라 넘넘 수고 많으셨고 props 컨벤션 부분만 고쳐 주시면 좋을 것 같습니당 👍
|
||
import Icons from '@/assets/svg/index'; | ||
|
||
type 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.
interface 선언 : 컴포넌트명+props 로 네이밍하기로 했어서 TextInputTimeProps로 네이밍 변경해주시면 좋을 것 같습니다!
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.
수정했습니다!
|
||
import { theme } from '@/styles/theme'; | ||
|
||
interface TextboxInputType { |
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.
여기도 TextboxInputProps로 네이밍만 변경해주시면 감사하겠습니당 ㅎㅎ
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 Props = { | ||
time: 'start' | 'end' | 'total'; | ||
}; |
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을 따로 사용한 이유가 있을까영???
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.
interface 로 수정했습니다! tsrafce 한 걸 그대로 사용했더니 type로 작성되었네요
<DailydateContainer> | ||
<DateText>{date}일</DateText> | ||
{/* CAPTION_02 추가 후 수정 필요 */} | ||
<DayText>{getNameOfDay(dayOfTheWeek)}</DayText> |
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.
getDay
쓰면 dayOfTheWeek
가 숫자로 반환되는건가요??? 😯
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.
https://www.w3schools.com/jsref/jsref_getday.asp
0-6으로 요일이 반환됩니다!
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/assets/svg/index.ts
Outdated
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.
Icons.아이콘명으로 쉽게 아이콘 접근할 수 있도록 설정하고 싶었습니다!
[CHORE] textbox-view 브랜치 최신화
작업 내용 🧑💻
알게된 점 🚀
css 내부에서 theme import 해서 테마 사용 가능
styled.input 내에서 props 에 따라 css 적용하기
리뷰 요구사항 💬
재사용하기에 괜찮은 구조인지 확인부탁드립니다!
이 컴포넌트는 버튼 작업이 아직 안 되어서 �차후 수정 예정입니다
날짜, 시간 인풋 받을 때 정규표현식으로 형식 설정하기 적용 안 되어있습니다! 이후 작업 필요합니다
해당 컴포넌트에서 Date 로 날짜, 요일 받아 표시하도록 제작했는데 이후 전체 페이지에서 날짜 다룬다면 props 로 뚫어서 넘겨받으면 될 것 같습니다
0-6 => 영문 요일 (Monday 등) 으로 변환하는 함수 utils 폴더 내에 추가해두었습니다
관련 이슈
close #26
스크린샷 (선택)