Skip to content
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

<Name />, <Avatar />리팩터링 #107

Merged
merged 8 commits into from
Jul 14, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 70 additions & 35 deletions src/components/Common/Avatar/Avatar.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,53 +1,88 @@
import type { Meta, StoryObj } from '@storybook/react';
import type { SingleAvatarProps } from '~/components/Common/Avatar/SingleAvatar';

import { userInfo } from '~/mocks/handlers/member/data';

import Avatar from './index';

const meta: Meta<typeof Avatar> = {
const disableArgs = ['userInfo'];
const disableArgsTypes = disableArgs.reduce((acc, cur) => {
(acc as Record<string, unknown>)[cur] = {
table: {
disable: true,
},
};
return acc;
}, {});

Comment on lines +9 to +17
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 다음 PR에 유틸로 하나 만들어둘게요.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안그래도 저도 그생각했는데 이거 어디다가 둘지, 전역에 storybook 넣어서 utils만들지
고민하다가 미뤄놨어요 ㅋㅋ

const meta: Meta = {
title: 'Avatar',
component: Avatar,
argTypes: {
...disableArgsTypes,
},
};

export default meta;

type AvatarStory = StoryObj<typeof Avatar>;
interface AvatarStoryProps {
size: SingleAvatarProps['size'];
empty: boolean;
}

type AvatarStory = StoryObj<AvatarStoryProps>;

export const SingleAvatar: AvatarStory = {
name: 'Avatar',
args: {
size: 'sm',
major: true,
nickName: '전공자',
isEmpty: false,
empty: false,
},
render: (args: AvatarStoryProps) => {
const { size, empty } = args;
return (
<Avatar
userInfo={empty ? undefined : userInfo.certifiedSsafyUserInfo}
size={size}
/>
);
},
};

export const AvatarGroup = () => {
const data = [
{
major: true,
nickName: '전공자',
},
{
major: false,
nickName: '비전공자',
},
{
major: true,
nickName: 'Eng',
},
{
major: false,
nickName: 'Test',
},
{
major: true,
nickName: 'Extra',
},
];
return (
<Avatar.Group>
{data.map((d) => (
<Avatar {...d} key={d.nickName} />
))}
</Avatar.Group>
);
interface AvatarGroupStoryProps {
avatarCount: number;
maxCount: number;
visibleCount: number;
size: SingleAvatarProps['size'];
}
type AvatarGroupStory = StoryObj<AvatarGroupStoryProps>;

export const AvatarGroup: AvatarGroupStory = {
name: 'AvatarGroup',
argTypes: {},
args: {
avatarCount: 2,
maxCount: 4,
visibleCount: 4,
size: 'sm',
},

render: (args: AvatarGroupStoryProps) => {
const { avatarCount, maxCount, visibleCount, size } = args;
const data = Array(avatarCount)
.fill(undefined)
.map(() => userInfo.certifiedSsafyUserInfo);

return (
<>
<div>
<Avatar.Group maxCount={maxCount} visibleCount={visibleCount}>
{data.map((d) => (
<Avatar size={size} userInfo={d} key={d.nickname} />
))}
</Avatar.Group>
</div>
</>
);
},
};
44 changes: 27 additions & 17 deletions src/components/Common/Avatar/AvatarGroup.tsx
Original file line number Diff line number Diff line change
@@ -1,46 +1,56 @@
import type { ReactNode, ComponentPropsWithoutRef, ReactElement } from 'react';
import type { SingleAvatarProps } from './SingleAvatar';
import type { ReactNode, ComponentPropsWithoutRef } from 'react';

import { css } from '@emotion/react';
import { Children, isValidElement } from 'react';

import { flex, fontCss } from '~/styles/utils';
import { fontCss, inlineFlex } from '~/styles/utils';

import SingleAvatar from './SingleAvatar';

export interface AvatarGroupProps extends ComponentPropsWithoutRef<'div'> {
children: ReactNode;
visibleCount?: number;
maxCount: number;
}
const AvatarGroup = (props: AvatarGroupProps) => {
const { children, visibleCount = 4, ...rest } = props;
const { children, maxCount, visibleCount = 4, ...rest } = props;

const validAvatars = Children.toArray(children).filter(isValidElement);
const validAvatars = Children.toArray(children).filter(
isValidElement<SingleAvatarProps>
);

Comment on lines +19 to 22
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아까 여쭤본 props의 타입 추론을 이렇게 하는게 맞는거겠죠..?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

정확합니다!

const visibleAvatars = validAvatars.slice(0, visibleCount);

const restAvatarsNumber = validAvatars.length - visibleCount;
const emptyAvatarsNumber = visibleCount - validAvatars.length;
const emptyAvatarsCount = visibleCount - validAvatars.length;
const restAvatarsCount = maxCount - visibleCount;
const avatarSize = validAvatars[0].props?.size || 'sm';
Comment on lines +24 to +26
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

지금 보면 요구사항에 +가 채워진 아바타 수가 아니라 총 인원수를 나타내도록 되어있어서 그렇게 바꿔놨습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

size는 어차피 undefined여도 Avatar렌더링할 때 sm로 렌더링 되긴 하지만,
뒤에 남은 아바타 인원수 나타낼 때의 그 텍스트에도 크기를 지정해줘야 해서 "sm"을 디폴트 값으로 넣었어요. (textSizeCss가 undefined 키를 갖지 않기 때문에 타입 오류가 발생)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기 잘하신 것 같아요!


return (
<div css={selfCss} {...rest}>
{visibleAvatars}
{Array.from({ length: emptyAvatarsNumber }).map((_, i) => (
<SingleAvatar isEmpty key={i} />
{Array.from({ length: emptyAvatarsCount }).map((_, i) => (
<SingleAvatar key={i} size={avatarSize} />
))}
{restAvatarsNumber > 0 && <span css={textCss}>+{restAvatarsNumber}</span>}
{restAvatarsCount > 0 && (
<span css={[textCss, textSizeCss[avatarSize]]}>
+{restAvatarsCount}
</span>
)}
</div>
);
};

const selfCss = css(
{
'> div': {
marginLeft: -2,
},
},
flex('center', 'center', 'row')
{ '> div': { marginLeft: -4 } },
inlineFlex('center', 'center', 'row'),
Comment on lines +44 to +45
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inlineFlexflex가 컨테이너의 width: auto값에 대한 동작이 다르기 때문에 inlineFlex로 줬습니다.
flex는 부모넓이 기준이고 inlineFlex는 자식 엘리먼트들의 width합 만큼을 width로 갖게 돼요. (그래서 flex로 하면 아바타가 많아질때 각 아바타들이 shrink되서, 이걸 방지하려면 각 아바타마다 shrink: 0을 줘야합니다.)

이건 개수 늘려보다가 오류 발견한거라 고친거긴한데,
사실 아바타그룹은 md사이즈, visible은 4개로만 사용되는것 같아서 큰 의미는 없어요.

fontCss.family.auto
);

const textCss = css(fontCss.style.R12);
const textCss = css({ marginLeft: 4 });
const textSizeCss = {
sm: css(fontCss.style.B12),
md: css(fontCss.style.B14),
lg: css(fontCss.style.B28),
};

export default AvatarGroup;
69 changes: 28 additions & 41 deletions src/components/Common/Avatar/SingleAvatar.tsx
Original file line number Diff line number Diff line change
@@ -1,91 +1,78 @@
import type { SerializedStyles } from '@emotion/react';
import type { ComponentPropsWithoutRef } from 'react';
import type { UserInfo } from '~/services/member';

import { css } from '@emotion/react';
import React from 'react';

import { flex, fontCss } from '~/styles/utils';
import { flex, fontCss, palettes } from '~/styles/utils';

export interface AvatarProps extends ComponentPropsWithoutRef<'div'> {
export interface SingleAvatarProps extends ComponentPropsWithoutRef<'div'> {
size?: AvatarSize;
major?: boolean;
nickName?: string;
isEmpty?: boolean;
userInfo?: UserInfo;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아까 말씀드린 부분이 이거에요.
userInfo가 들어오면, 어떻게 그릴지가 Avatar컴포넌트의 책임이 되는데,
프로퍼티가 nickname, major, isEmpty 이렇게 나눠서 들어오면
null, undefined체크부터 시작해서 어떻게 그려야 할지가, Avatar를 사용하는 쪽의 책임이 되어버려서, 수정하기 어렵고, 수정이 발생했을때 변경되는 부분이 많아지고, 코드가 더 복잡해진다고 생각합니다.

유저정보를 받아서 그리는 컴포넌트는, UserInfo타입을 아토믹한 단위로 받는다고 생각하시면 될 것 같습니다.

물론 프로퍼티를 너무 명확하게 한 두개만 받는 경우엔 또 얘기가 달라지겠지만요

}
type AvatarSize = 'sm' | 'md' | 'lg';
type BackgroundColor = 'major' | 'nonMajor';

const SingleAvatar = (props: AvatarProps) => {
const {
major = false,
size = 'sm',
nickName = '샆사운드',
isEmpty = false,
...rest
} = props;
// 현재 설계상 major라는 이름으로 전공여부를 가지고오게되어 그대로 사용하기 위해 major라는 명칭을 사용하게 됨.
const SingleAvatar = (props: SingleAvatarProps) => {
const { size = 'sm', userInfo, ...restProps } = props;

return (
<div
css={[
selfCss,
sizeCss[size],
backgroundCss[major ? 'major' : 'nonMajor'],
isEmpty && emptyCss,
userInfo?.isMajor && majorCss,
!userInfo && emptyCss,
]}
{...rest}
{...restProps}
>
{isEmpty || (
<span css={[[textCss[size]], textCapitalizeCss, fontStyleCss]}>
{getFirstText(nickName)}
{userInfo && (
<span css={[textCss[size], textCapitalizeCss, lineHeightCss]}>
{getFirstText(userInfo.nickname)}
Comment on lines +29 to +31
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이쪽도 empty를 바깥에서 정의해줄 필요 없이, 데이터를 넘기지 않으면 그게 곧 Empty 아바타가 되는거로 생각했어요.

<Avatar user={user} /> => user데이터의 아바타를 그린다.
<Avatar /> => user의 아바타를 그려야 하는데, user가 없음 => empty

만약 강제로 Empty를 넣을 의도라면,
empty props추가해서 다음 시나리오로 가면 될것같아요.

  1. 유저정보가 없으면 기본적으로 빈 아바타
  2. 유저정보가 있으면 채워진 아바타
  3. 유저정보가 있는데 empty prop이 true면 빈 아바타 (즉, 유저 정보보다, empty가 우선순위가 높음. CSS의 !important같은것)

하지만, 아바타 그룹이 사용되는 곳을 보면 없는 유저에 대해서만 Empty로 그리기 떄문에 일단은 이렇게 가도 될 것 같아요.

</span>
)}
</div>
);
};

const getFirstText = (str: string) => str.at(0);
const getFirstText = (str: string) => str.at(0) || '';

const selfCss = css(
{
borderRadius: 100,
color: '#000',
border: '0.6px solid #fff',
color: palettes.black,
border: `0.6px solid ${palettes.white}`,
backgroundColor: palettes.nonMajor,
},
flex('center', 'center', 'row')
flex('center', 'center', 'row'),
fontCss.family.auto
);

const fontStyleCss = fontCss.family.manrope;

const sizeCss: Record<AvatarSize, SerializedStyles> = {
sm: css({ width: 12, height: 12 }),
md: css({ width: 18, height: 18 }),
sm: css({ width: 16, height: 16 }),
md: css({ width: 20, height: 20 }),
lg: css({ width: 40, height: 40 }),
};

const lineHeightCss = css({ lineHeight: 1 });

const textCss: Record<AvatarSize, SerializedStyles> = {
sm: css(fontCss.style.B12),
md: css(fontCss.style.B14),
lg: css(fontCss.style.B24),
lg: css(fontCss.style.B28),
};

const textCapitalizeCss = css({
textTransform: 'capitalize',
});

const backgroundCss: Record<BackgroundColor, SerializedStyles> = {
major: css({
backgroundColor: '#71E498',
// todo 팔레트로 이관
}),
nonMajor: css({
backgroundColor: '#FFBF75',
}),
};
const majorCss = css({
backgroundColor: palettes.major,
});

const emptyCss = css({
backgroundColor: '#F0F0F0',
border: '1px dotted #292929',
backgroundColor: palettes.white,
border: `1px dotted ${palettes.grey0}`,
});

export default SingleAvatar;
7 changes: 1 addition & 6 deletions src/components/Common/SsafyIcon/Track.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ type TrackIconStory = StoryObj<typeof Track>;
export const TrackIcon: TrackIconStory = {
args: { name: SsafyTrack.MOBILE, size: TrackSize.SM1 },
argTypes: {
label: {
table: {
disable: true,
},
},
style: {
table: {
disable: true,
Expand All @@ -43,7 +38,7 @@ export const AllTrackIcons: TrackIconStory = {
<div key={track} style={{ display: 'flex', gap: 10 }}>
{sizes.map((size) => (
<div key={size}>
<Track name={track} size={size} theme={'primary'}/>
<Track name={track} size={size} theme={'primary'} />
</div>
))}
</div>
Expand Down
21 changes: 6 additions & 15 deletions src/components/Common/SsafyIcon/Track.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,36 +12,27 @@ import PythonTrack from '~/assets/images/track-python.svg';
import Uncertified from '~/assets/images/track-uncertified.svg';
import { SsafyTrack } from '~/services/member/utils';
import { inlineFlex } from '~/styles/utils';
import { defaultify } from '~/utils';

// MajorType을 ~/services/member 에서 가져오면 참조 오류가 발생하는데 이유를 잘 모르겠습니다.
// SsafyTrack을 ~/services/member 에서 가져오면 참조 오류가 발생하는데 이유를 잘 모르겠습니다.
// import 구문 옆에다 주석 달면 린트 오류가 발생해서 여기 적어둡니다.

export interface TrackProps<Track extends keyof typeof tracks> {
/**
* API 명세에 `null`이 들어올 수 있다고 되어있는데
* 만약 그렇다면 `fetcher`에서 `null`을 `undefined`로 변환할 예정입니다.
*/
name?: Track;
label?: string;
size?: TrackSize;
style?: CSSProperties;
theme?: Track extends 'fallback' | undefined ? FallbackTheme : undefined;
}

const Track = <T extends keyof typeof tracks>(props: TrackProps<T>) => {
const {
name = 'fallback',
label = name,
size = TrackSize.SM1,
style = {},
theme,
} = props;
const { name = 'fallback', size = TrackSize.SM1, style = {}, theme } = props;
const safeName = defaultify(name, [null]).to('fallback') as NonNullable<T>;

const TrackComponent = tracks[name];
const TrackComponent = tracks[safeName];

return (
<div css={selfCss}>
<AccessibleIcon label={label}>
<AccessibleIcon label={safeName}>
<TrackComponent style={{ height: size, ...style }} theme={theme} />
</AccessibleIcon>
</div>
Expand Down
Loading