<Name />, <Avatar />리팩터링
#107
Conversation
There was a problem hiding this comment.
Avatar 스토리북
AvatarGroup 스토리북
Name 스토리북
이렇게 참고해주시면 될 것 같습니다.
| const disableArgsTypes = disableArgs.reduce((acc, cur) => { | ||
| (acc as Record<string, unknown>)[cur] = { | ||
| table: { | ||
| disable: true, | ||
| }, | ||
| }; | ||
| return acc; | ||
| }, {}); | ||
|
|
There was a problem hiding this comment.
이거 다음 PR에 유틸로 하나 만들어둘게요.
There was a problem hiding this comment.
안그래도 저도 그생각했는데 이거 어디다가 둘지, 전역에 storybook 넣어서 utils만들지
고민하다가 미뤄놨어요 ㅋㅋ
| const validAvatars = Children.toArray(children).filter( | ||
| isValidElement<SingleAvatarProps> | ||
| ); | ||
|
|
There was a problem hiding this comment.
아까 여쭤본 props의 타입 추론을 이렇게 하는게 맞는거겠죠..?
| const emptyAvatarsCount = visibleCount - validAvatars.length; | ||
| const restAvatarsCount = maxCount - visibleCount; | ||
| const avatarSize = validAvatars[0].props?.size || 'sm'; |
There was a problem hiding this comment.
지금 보면 요구사항에 +가 채워진 아바타 수가 아니라 총 인원수를 나타내도록 되어있어서 그렇게 바꿔놨습니다.
There was a problem hiding this comment.
size는 어차피 undefined여도 Avatar렌더링할 때 sm로 렌더링 되긴 하지만,
뒤에 남은 아바타 인원수 나타낼 때의 그 텍스트에도 크기를 지정해줘야 해서 "sm"을 디폴트 값으로 넣었어요. (textSizeCss가 undefined 키를 갖지 않기 때문에 타입 오류가 발생)
| major?: boolean; | ||
| nickName?: string; | ||
| isEmpty?: boolean; | ||
| userInfo?: UserInfo; |
There was a problem hiding this comment.
아까 말씀드린 부분이 이거에요.
userInfo가 들어오면, 어떻게 그릴지가 Avatar컴포넌트의 책임이 되는데,
프로퍼티가 nickname, major, isEmpty 이렇게 나눠서 들어오면
null, undefined체크부터 시작해서 어떻게 그려야 할지가, Avatar를 사용하는 쪽의 책임이 되어버려서, 수정하기 어렵고, 수정이 발생했을때 변경되는 부분이 많아지고, 코드가 더 복잡해진다고 생각합니다.
유저정보를 받아서 그리는 컴포넌트는, UserInfo타입을 아토믹한 단위로 받는다고 생각하시면 될 것 같습니다.
물론 프로퍼티를 너무 명확하게 한 두개만 받는 경우엔 또 얘기가 달라지겠지만요
| {userInfo && ( | ||
| <span css={[textCss[size], textCapitalizeCss, lineHeightCss]}> | ||
| {getFirstText(userInfo.nickname)} |
There was a problem hiding this comment.
이쪽도 empty를 바깥에서 정의해줄 필요 없이, 데이터를 넘기지 않으면 그게 곧 Empty 아바타가 되는거로 생각했어요.
<Avatar user={user} /> => user데이터의 아바타를 그린다.
<Avatar /> => user의 아바타를 그려야 하는데, user가 없음 => empty
만약 강제로 Empty를 넣을 의도라면,
empty props추가해서 다음 시나리오로 가면 될것같아요.
- 유저정보가 없으면 기본적으로 빈 아바타
- 유저정보가 있으면 채워진 아바타
- 유저정보가 있는데 empty prop이 true면 빈 아바타 (즉, 유저 정보보다, empty가 우선순위가 높음. CSS의 !important같은것)
하지만, 아바타 그룹이 사용되는 곳을 보면 없는 유저에 대해서만 Empty로 그리기 떄문에 일단은 이렇게 가도 될 것 같아요.
| }), | ||
| }; | ||
| const textBaseCss = css({ | ||
| maxWidth: 230, |
There was a problem hiding this comment.
네임의 maxWidth를 피그마에 있는대로 조절했습니다.
| { '> div': { marginLeft: -4 } }, | ||
| inlineFlex('center', 'center', 'row'), |
There was a problem hiding this comment.
inlineFlex와 flex가 컨테이너의 width: auto값에 대한 동작이 다르기 때문에 inlineFlex로 줬습니다.
flex는 부모넓이 기준이고 inlineFlex는 자식 엘리먼트들의 width합 만큼을 width로 갖게 돼요. (그래서 flex로 하면 아바타가 많아질때 각 아바타들이 shrink되서, 이걸 방지하려면 각 아바타마다 shrink: 0을 줘야합니다.)
이건 개수 늘려보다가 오류 발견한거라 고친거긴한데,
사실 아바타그룹은 md사이즈, visible은 4개로만 사용되는것 같아서 큰 의미는 없어요.
Issues
구분
주요 변경점
스크린샷
기타