-
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: Accordion 컴포넌트 구현 및 스토리북 작성 #35
Conversation
🚀 Storybook Preview 보러가기: https://65a6c73d536a3c43b7c5c9bb-rbsdhfcggw.chromatic.com/ |
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.
고생하셨어요!! 😊
말줄임표나 framer-motion의 활용 등 많은 고민을 하셨던 게 느껴졌어요!! 👍👍
날짜 형식을 템플릿 문자열 형태로 변환하는 유틸 함수도 좋네요!
폰트와 관련해서는.. 디자이너 팀원분들께 여쭤봐야 좋겠네요... (모든 텍스트를 SUIT 체로 통일해도 되는지? 등)
혹시 아코디언이 접히면서 순간적으로 텍스트가 두 개로 겹쳐 보이는 현상이 있는데.. 이 현상은 해결하기 어려우려나요..? 😂
(현재 동일한 text가 isOpen
불린 값에 따라 두 노드에 중복해서 렌더링되고 있는데, 텍스트는 메인 컨텐츠 영역에만 렌더링하고 불린 값에 따라서 height만 조절한다거나..?)
src/utils/dateUtils.ts
Outdated
const inboxDate = (date: Date): string => { | ||
return `${formatDate(date)}에 받은 편지`; | ||
}; | ||
|
||
const sendDate = (date: Date): string => { | ||
return formatDate(date); | ||
}; | ||
|
||
export { inboxDate, sendDate }; |
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.
P4;
Date 객체를 받아서 화면에 보여줘야 할 템플릿 형태를 반환하는 formatDate
유틸 함수를 만드신 부분이 재활용하기에 무척 좋다고 생각해요!! 👍👍
그런데, inboxDate
나 sendDate
라는 따로 유틸 함수가 없어도 컴포넌트 단에서 직접 formatDate
를 호출하는 걸로 충분히 쉽게 만들어낼 수 있는 값일 것 같아서 없어도 되지 않을까? 싶어요!!
const inboxDate = (date: Date): string => { | |
return `${formatDate(date)}에 받은 편지`; | |
}; | |
const sendDate = (date: Date): string => { | |
return formatDate(date); | |
}; | |
export { inboxDate, sendDate }; | |
export { formatDate }; |
src/components/Accordion/index.tsx
Outdated
imgUrl?: string; | ||
} | ||
|
||
const Accordion = ({ |
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.
p2;
Accordion라는 이름에서 유추하는 컴포넌트는 왠지 편지 내용이라고 하는 특정 도메인에 종속되는 것이 아니라, 카드의 특정 부분을 클릭하면 열리고 닫히는 애니메이션 효과만 있는 일반적인 컴포넌트라는 생각이 들어요!!
그런데 현재 Accordion 컴포넌트에서는 date
, type
, imgUrl
등 편지 내용이라는 도메인 을 포함하고 있는 형태이다보니.. Accordion 이라는 컴포넌트 이름보다는 도메인 맥락을 포함한 이름이어도 좋겠다는 생각이 들었어요!! (ex. LetterAccordion?)
만약 Accordion이라는 컴포넌트의 열고 닫는 기능을 활용해야 하지만.. 편지라는 도메인이 아닌 다른 내용이 추가된다면 props로 전달해야 하는 조건 분기가 많아질 수 있을 것 같아요!
그래서.. 일반적으로 활용할 수 있는 형태로 도메인 맥락을 제거하는 것도 좋지만.. 사실 쉽지 않은 추상화 작업이 될 수도 있을 것 같아서 컴포넌트의 이름에 도메인 맥락을 포함시키는 건 어떨까요??
src/components/Accordion/styles.ts
Outdated
left: 20px; | ||
width: 55px; | ||
height: 65px; | ||
padding: 5.182px 5.182px 15.547px; |
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.
ask;
크기 단위에 절대 단위인 px
을 적용할 수도 있고.. 상대 단위인 rem
을 적용할 수도 있을 것 같은데 둘 사이에는 어떤 장단점이 있을지 궁금해요!! 🧐
px vs rem
src/hooks/useCheckTextLines.tsx
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.
p5;
훅 내부에서 JSX를 작성하고 있는 형태는 아니어서 .ts
확장자를 적용해도 좋겠다는 생각이 들었어요!! 😊
src/hooks/useCheckTextLines.tsx
Outdated
if (textContainerRef.current) { | ||
const element = textContainerRef.current; | ||
const lineHeight = parseFloat( | ||
window.getComputedStyle(element).lineHeight, |
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.
오.. window 객체에 요런 API도 있었군요! 배워가요! 👍👍
그런 방법도 있겠네요! 그렇게 수정해보겠습니다!
맞아요 그냥
아 그냥 아무생각 없이 피그마에 있는거 복붙해서 다
네 그렇군요! |
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.
고생하셨어요!! 👍👍 Approve 입니당
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.
p5;
디렉토리 이름에도 LetterAccordion 를 적용하는 건 어떨까요?? 😊
/src/components/LetterAccordion
src/hooks/useCheckTextLines.ts
Outdated
} | ||
}, [text, textContainerRef, checkTextLines]); | ||
|
||
console.log(currentLineCount); |
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.
p5;
오오! 테스트용 콘솔 출력인 것 같아서 삭제되는 것도 좋을 것 같아요!! 😁
IDE에 입력한 px 단위를 편하게 rem으로 변환할 수 있는 익스텐션도 있더라구요! https://marketplace.visualstudio.com/items?itemName=cipchk.cssrem |
🧩 이슈 번호
✅ 작업 사항
useCheckTextLines
훅을 만들었습니다. (처음엔 계산해서 넣었는데 PC에서 모바일 크기로 보는 거랑 실제 모바일이랑 조금 달라서ㅠㅠ 이것저것 계속 값 변경하다가 그나마 괜찮은 결과를 찾았습니다.)webkit
을 사용하여 3줄이 넘어가면 말줄임표로 표시됩니다.framer-motion
을 사용하여 애니메이션 효과를 주었습니다.[모바일]
8f695358-7031-4be3-9044-94104df2de6e.mp4
[PC]
(+ 추가)아.. 보관함 있는 곳만 보고 만들었는데지금보니깐 답장보내는 곳도 있고 사진이 있는 경우도 있었네요😅.... 다시 수정해야되겠네요. 그리고 몇글자까지 보일 지도 props 값으로 넘겨줘야 되겠네요~~
👩💻 공유 포인트 및 논의 사항
사용방법
id
,text
,date
,line
(기본값 3),type
(기본값 inbox),imgUrl
값을 넣어줍니다.utills/dateUtils.ts
를 만들었습니다.type
에는 inbox(보관함) 버전이랑 send(편지보내기) 버전이 있습니다.imgUrl
는 폴라로이드 이미지 사진인데 이건 나중에 새로 컴포넌트로 만들어야 될 것 같네요.논의 사항
폰트도 적용시켜야 될 것 같습니다. 최대한 웹폰트를 사용하고 웹폰트가 없으면 ttf 파일 다운받으면 될까요?