Conversation
There was a problem hiding this comment.
한 가지 제안 사항으로,, 주로 많이 쓰이는 flex 속성을 style 폴더 안에 CommonStyle로 분리하여 재사용하면 어떨까요??
예를 들면 이런식으로 공통 스타일을 정의하고,
// commonStyles.ts
import { css } from 'styled-components';
export const flexCenter = css`
display: flex;
justify-content: center;
align-items: center;
`;
export const flexAlignCenter = css`
display: flex;
align-items: center;
`;
컴포넌트에서 이런식으로 사용하면 좀 더효율적이지 않을까 싶습니다..! 워낙 많이 쓰이는 속성들이다 보니 flex 속성을 줄때마다 반복되는 css 속성들이 많아서 생각해본 방식이었습니다 !
const Container = styled.div`
${flexCenter};
`
| <ListContainer> | ||
| {meetingData.map((meeting, index) => ( | ||
| <MeetingListItem | ||
| key={index} | ||
| isSelected={meeting.isSelected} | ||
| title={meeting.title} | ||
| description={meeting.description} | ||
| location={meeting.location} | ||
| participants={meeting.participants} | ||
| time={meeting.time} | ||
| /> | ||
| ))} | ||
| </ListContainer> |
There was a problem hiding this comment.
map 함수에서 배열의 index를 key로 사용하면 배열의 변경(추가,삭제 등 ) 시 리렌더링 문제가 있을 수 있다고 알고 있어서 index보단 데이터의 고유 id 값을 생성하고 그 값을 key로 사용하는 게 더 좋을 것 같습니다!
There was a problem hiding this comment.
넵! 지금은 그냥 mock data를 쓰는 중이라 그냥 저렇게 뒀고,,, 후에 api 연결 시 key 값 제대로 넣겠습니다!
|
|
||
| const Description = styled.div` | ||
| font-size: 14px; | ||
| color: ${theme.COLORS.gray[300]}; |
There was a problem hiding this comment.
현재 작성하신 css 들을 보면 이렇게 theme 객체를 바로 사용하는 방식
( color: ${theme.COLORS.gray[300]}; ) 과
theme 객체를 props에서 구조 분해 할당으로 사용하는 방식( color: ${({ theme }) => theme.COLORS.main}; )이 살짝 혼용되어 사용되고 있는데 이를 통일해서 사용하면 코드의 일관성과 가독성이 더 좋아질 것 같습니다!
찾아보니 Themeprovider를 사용하면 theme은 기본적으로 props로 주입되기 때문에 구조 분해 할당 방식으로 통일하는게 더 좋다고 하네요
There was a problem hiding this comment.
넵 일단 작성한 것들은 나중에 수정하고 남은 작업사항에 대해서는 구조분해할당 방식 적용 하겠습니다!
| {tabs.map((tab, index) => ( | ||
| <Tab | ||
| key={index} | ||
| isSelected={selectedIndex === index} | ||
| onClick={() => onTabClick(index)} | ||
| > | ||
| {tab} |
There was a problem hiding this comment.
이 부분도 index값보다는 고유 id값을 활용하는게 좋을 것 같습니다!
There was a problem hiding this comment.
넵 이건 id 값 만들어서 넣어 둘게용
|
수고하셨습니다❤ |
| import homeIcon from '@/assets/images/ic_home.svg'; | ||
| import homeIconActive from '@/assets/images/ic_home_color.svg'; | ||
| import participantsIcon from '@/assets/images/ic_people.svg'; | ||
| import participantsIconActive from '@/assets/images/ic_people_color.svg'; | ||
| import myPageIcon from '@/assets/images/ic_navi_logo.svg'; | ||
| import mypageIconActive from '@/assets/images/ic_navi_logo_color.svg'; |
There was a problem hiding this comment.
아 그리고 그때 svg 가져오는 방식을 img 태그말고 컴포넌트로 쓸 수 있도록 설정해보신다고 하신 것 같은데 이 부분은 설정은 됐는데 그냥 까먹고 img import하는 걸로 쓰신 건지 ,, 아니면 설정이 안된건지 궁금합니다 ..!
There was a problem hiding this comment.
까먹음 이슈.... 이것도 적용해 보겠습니다~!!
| <Container> | ||
| <Logo src={logo} alt="로고" /> | ||
|
|
||
| <NoticeContainer> | ||
| <div>배달팟 실시간 모집 중</div> | ||
| <img src={rightArrow} alt="오른쪽 화살표" /> | ||
| </NoticeContainer> |
There was a problem hiding this comment.
혹시 제가 만들어둔 Header 컴포넌트 확인해보셨나요?? 이렇게 말구 헤더 컴포넌트 재사용하는 게 좋을 것 같아요! 스토리북 실행해보시고 확인부탁드립니다. 재사용할 수 있도록 헤더 컴포넌트를 구현해놓긴 했는데 혹시 수정이 필요하다면 알려주세용
There was a problem hiding this comment.
아 헤더에 로고만 있는 줄 알았네용 With Sub Text에 오른쪽 화살표 이미지가 추가 되어야 할 것 같습니다~!! 이건 제가 추가 할게요!
오~~ 좋은 방법 같습니다! |
그러면 제가 만들까요?-? 아니면 만들어주실런지 ... |
📝 Work Description
📸 Screenshot
글쓰기 버튼 위치가 애매한 이유는 저게 모바일로 봤을 때 더 내려가면 네비랑 너무 딱 붙어서 어쩔 수 없었습니다,, 최선핑,,
그 배달팟에서 배달 목록... 디자인이 조금 어려워서 .... 일단 이렇게 구현 했습니다.. 추후 디자이너님과 PM님께 말씀드리고 타협점을 찾아 볼게요.. 하하..
📣 To Reviewers
까먹고 커밋에 이슈번호 안 붙엿습니다... 다음 이슈부터는 적용할게욥 죄송 ㅠㅠ
그리고 탭이랑 바텀네비 그냥 컴포넌트로 만들었는데 필터링 컴포넌트 만들면서 스토리북화 시키겟습니다!