chore: Tab 컴포넌트 관련 리팩토링 및 코드 리뷰 반영했습니다.

[summermong]

🤔 어떤 문제가 발생했나요?

• barrel export 미반영
• 조건부 스타일링 코드가 지저분함
• type과 interface의 혼용으로 야기할 수 있는 디버깅 어려움

🎉 어떻게 해결했나요?

• barrel export 반영
• css, cx를 통해 스타일링 코드 리팩토링
• interface로 통일

📷 이미지 첨부 (Option)

UI는 문제 없이 잘 반영되었습니다!

2024-07-25.3.56.28.mov

[Jungjjeong]

Tab과 tab 디렉토리가 두개인데, 요거 어떤 차이일까요?

[Jungjjeong]

밑에 중복으로 선언되어 있어 하나로 합쳐도 좋을 것 같습니다.

type?: TabTypeProps;

[Jungjjeong]

추가로 variant도 별도로 나눠 하나의 타입으로 관리하면 조금 더 가독성과 재사용성이 좋을 것 같아요

interface TabVariant = 'fill' | 'fit-content';


export interface TabItemProps {
  ...,
  variant?: TabVariant;
  type?: TabTypeProps;
}

export interface TabProps {  children?: ReactNode } extends TabVariant;

[Jungjjeong]

type에 따른 하나의 스타일들만 들어가고 있어서 Map 객체를 사용하면 조금 더 간결하게 표현할 수 있을 것 같아요.

const typeStylesMap = new Map([
  ['primary', primaryStyles],
  ['secondary', secondaryStyles],
  ['assistive', assistiveStyles],
]);

const variantStylesMap = new Map([
  ['fit-content', fitContentStyles],
  ['full', fullWidthStyles],
]);

const tabItemStyles = cx(
  baseStyles,
  typeStylesMap.get(type),
  variantStylesMap.get(variant)
);

참고로 Map.get에 type이 들어가지 않으면 undefined를 반환합니다
따라서 props를 optional하게 받아올 때, default 값을 명시해주는 것이 좋지 않을까 제안드려요!

물론 이건 지극히 개인적인 의견이기 때문에 무시해주셔도 됩니다!!!!!!!!!!!!!!

[penrose15]

코드리뷰는 이렇게 하는 거였군요....

[Jungjjeong]

default 값이 잆다면 type = 'primary' / variant = 'fill' 이렇게 지정해줘도 좋을 것 같습니다.

[Jungjjeong]

요기서도 타입 재사용이 가능하지 않을까 싶습니다.

[wokbjso]

👍🔥🔥

[wokbjso]

display: 'flex' 를 선언해야 하는 스타일에는 Panda에서 제공해주는 flex pattern 사용하면 좀 더 간편할 것 같아요~~

Suggested change

	const baseStyles = css({
	display: 'flex',
	justifyContent: 'center',
	alignItems: 'center',
	cursor: 'pointer',
	});
	const baseStyles = flex({
	justifyContent: 'center',
	alignItems: 'center',
	cursor: 'pointer',
	});

[wokbjso]

제 의견을 하나 말씀드리자면, TabItemProps 에 사용되는 타입이 TabProps와 TabTypeProps에도 같이 사용되고 있더라고요!!

근데 유틸리티 타입으로 확장을 하자니 Tab 자체가 TabItem 보다는 더 큰 범주에 있기 때문에, TabProps 에서 TabItemProps를 extend 하는거는 의미상 말이 안되기도 하고요.

그래서 TabItem 관련 interface에서 TabProps를 extends 하셔서 쓰면 굳이 같은 prop을 재선언 해주지 않아도 되고, 의미도 더 명확해질 것 같습니다~!!

Suggested change

	export interface TabItemProps {
	export interface TabProps {
	variant?: 'fill' | 'fit-content';
	children?: React.ReactNode;
	}
	export interface TabTypeProps {
	type?: 'primary' | 'secondary' | 'assistive';
	}
	export interface TabItemProps extends Pick<TabProps ,'variant'> & TabTypeProps {
	selected: boolean;
	text: string;
	onClick: () => void;
	}

[wokbjso]

margin 같은 외부 스타일을 주입할 수 있도록 className?: string 속성 하나 추가해주시면 좋을 것 같아요~!!!

[github-actions]

🚀storybook: https://668f732cef9e25b9abbd3fc6-ftxtgukomn.chromatic.com/

[ywonchae1]

지영언니 리뷰는 보면 볼 수록 기분이 좋아지는 마법이 있어요...🥹
현민오빠 꼼꼼한 코드리뷰까지 언제나 열일 하시는궁요
이렇게 대단한 코드 짜온 윤이언니도 고생했고 허준도 명심보감이야~~~~ 웹팀 최고야아 멋져어 🩷

[github-actions]

🚀storybook: https://668f732cef9e25b9abbd3fc6-uotgljfrmc.chromatic.com/