feat: 내 정보 뽀각 페이지 UI 구현 #15
Conversation
woo-jk
requested review from
qkrdmstlr3,
Collection50 and
eunbeann
as code owners
src/utils/date.ts
Outdated
| @@ -0,0 +1,9 @@ | |||
| export const formatToYYMMDD = (dateString: string) => { | |||
There was a problem hiding this comment.
함수에서 yy, mm, dd가 .기호를 사이에 두고 반환된다는 것이 잘 드러나지 않는 것 같아요. 두 번째 prop으로 option객체같은 것을 뚫어서 명시해주는 것은 어떤가요?
그렇게되면 다른 기호(-)도 받을 수 있을 것 같아요!
There was a problem hiding this comment.
const formatToYYMMDD = (dateString: string, separator: string = '') => {
const date = new Date(dateString);
const yy = String(date.getFullYear()).slice(-2);
const mm = String(date.getMonth() + 1).padStart(2, '0');
const dd = String(date.getDate()).padStart(2, '0');
return [yy, mm, dd].join(separator);
};넵 말씀하신대로 함수명과 반환되는 값이 불일치한 것 같아서 위와 같이 코드를 변경해봤는데, 혹시 코멘트 의도를 잘 이해한 것이 맞을까요?
There was a problem hiding this comment.
이런 느낌은 어떠세요?? 두번째 인자가 separator를 가지는지 알기 어려울 수 있어서, option객체로 묶어서 표현해볼 수 있을 것 같아요!
type Option = { separator?: string }
const formatToYYMMDD = (dateString: string, { separator = '.' }?: Option) => {
const date = new Date(dateString);
const yy = String(date.getFullYear()).slice(-2);
const mm = String(date.getMonth() + 1).padStart(2, '0');
const dd = String(date.getDate()).padStart(2, '0');
return [yy, mm, dd].join(separator);
};There was a problem hiding this comment.
아하 parameter를 option 객체로 받으면 조금 더 확장성 있어지겠군요! 반영하겠습니다~!
There was a problem hiding this comment.
shadcn에서 컴포넌트를 install하면 자동으로 이 ui 폴더에 설치가 되는데, 이대로 두는게 괜찮을까요?
아니면 ui 폴더를 벗겨서 다른 컴포넌트와 폴더 구조를 똑같이 가져가는게 좋을까요?
다른 분들의 의견을 듣고 싶습니다!
There was a problem hiding this comment.
저는 ui폴더 밖으로 빼서 다른 컴포넌트들과 동일한 구조를 가지는 것이 어떤가 싶어요!
There was a problem hiding this comment.
저희 프로젝트와 일관된 구조를 갖는게 좋다는 생각으로 은식님 의견에 동의합니다 !
| <div className="mb-[9px] text-[12px] text-neutral-20">{formattedDate}</div> | ||
| <div className="w-auto truncate text-[16px] font-semibold">{title}</div> | ||
| </div> | ||
| <div> |
There was a problem hiding this comment.
이 div 태그가 없어도 괜찮을 것이라고 생각하는데 어떻게 생각하시나요??
There was a problem hiding this comment.
해당 div 태그가 없다면 상단 div의 flex 속성 때문에 위 사진처럼 more 버튼의 height가 늘어나게 됩니다(원래는 정사각형).
버튼의 모양은 유지하면서 버튼의 배치만 지정하기 위해 div 태그로 한 번 감싸주게 되었습니다!
| {cardTagList.map((tag) => ( | ||
| <Tag key={tag.id} color={TAG_TYPE_COLOR[tag.type]}> | ||
| {tag.name} | ||
| </Tag> | ||
| ))} |
There was a problem hiding this comment.
구조 분해 할당을 사용하시는 것은 어떻게 생각하시나요-??
There was a problem hiding this comment.
넵 좋은것 같아요! 반영했습니다 :)
| // TODO: API 연동 시 response data로 변경 | ||
| const infoCount = mockInfoCount; | ||
| const infoList = mockInfoList; |
| const infoList = mockInfoList; | ||
|
|
||
| return ( | ||
| <div> |
There was a problem hiding this comment.
CardList라면 section 태그가 어떤지 여쭤보고 싶습니다~
There was a problem hiding this comment.
넹 좋은 것 같습니다! 반영했어요!
There was a problem hiding this comment.
저희 프로젝트와 일관된 구조를 갖는게 좋다는 생각으로 은식님 의견에 동의합니다 !
| export interface TagProps { | ||
| color?: 'default' | 'blue' | 'purple'; | ||
| } |
There was a problem hiding this comment.
이 타입도 types/info에 분리하는 건 어떠신가요??
There was a problem hiding this comment.
해당 타입은 info 데이터의 값이라기보단 Tag 컴포넌트의 variant 같은 느낌이라 types/info 보다는 현재 위치가 좀 더 잘 어울리는 것 같습니다!
api의 info 데이터에도 태그의 color 값을 내려주는 것이 아니라 type 값(인성/역량)을 내려주고 있고, 이 type에 따라 프론트에서 자체적으로 color를 지정해주는 형식입니다.
어떻게 생각하시나요!
|
main 브랜치에서 pull 후 컨플릭트 나는거 풀어서 커밋했습니다. 한번씩 확인 후 approve 부탁드려요 |
|
|
||
| export const mockInfoCount = { | ||
| '경험 정리': 1, | ||
| 자기소개서: 3, |
There was a problem hiding this comment.
목데이터라 큰 의미 업겟지만... 혹시.. 여기만 string으로 안묶은 이유가 따로 잇는건가요??
There was a problem hiding this comment.
경험 정리와 면접 질문은 중간에 띄어쓰기가 들어가기 때문에 객체의 key값으로 인식 못해서 따옴표로 묶어줬어용
반면에 자기소개서는 띄어쓰기가 없기 때문에 따옴표 없이도 key 값으로 인식합니다!
| import { Inter } from 'next/font/google'; | ||
| import './globals.css'; | ||
| import { cn } from '@/utils/tailwind-util'; |
There was a problem hiding this comment.
넹 clsx와 twMerge의 기능을 같이 해주는 유틸함수입니다!
1. 무슨 이유로 코드를 변경했나요?
내 정보 뽀각 페이지의 UI 마크업 구현
2. 어떤 위험이나 장애를 발견했나요?
코멘트로 달겠습니다
3. 관련 스크린샷을 첨부해주세요.
2024-07-23.2.39.45.mov
4. 완료 사항