Tabs 컴포넌트 리팩터링

[jinhokim98]

issue

• close Tabs 컴포넌트 리팩터링 #980

구현 사항

탭 사이즈와 관련된 기능 커스텀 훅으로 분리

이 기능은 처음 로드 됐을 때 이후에 작동하지 않습니다. 또한 탭의 동작 기능에 영향을 주지 않으므로 Tabs 컴포넌트 내부에 코드를 위치하지 않고 useTabSizeInitializer 훅으로 분리했습니다. 아래처럼 tabWidth를 받아서 아래 컴포넌트에 적용해줬습니다

const tabWidth = useTabSizeInitializer(tabRefs);

return (
  ...
   {tabRefs.current && tabWidth !== 0 && (
     <li value={activeTabIndex} css={indicatorStyle({theme, tabWidth, activeTabIndex})} />
   )}
  ...
)

Tab 관련 코드 Tab.tsx로 이동

이전의 Tab은 이렇게 생겼습니다.

const Tab: React.FC<TabProps> = () => {
  return <></>;
};

이렇게 선언한 이유는 Tabs 내부에 선언된 activeIndex, setter를 이용하기 위함입니다.
반복문으로 호출하여 index도 바로 알아낼 수 있고 prop으로 setter를 넘겨주지 않아도 바로 작동하니깐 안에 작성했는데 Tabs 컴포넌트의 가독성이 너무 좋지 않기도 하며, Tab 컴포넌트가 위에 달랑 Fragment로 되어있는 것도 이상했기 때문에 이동했습니다.

그렇다면 Tabs의 자식인 Tab에게 index와 선택 여부를 넘겨줄 수 있을지 고민했고 cloneElement를 사용하게 됐습니다.
이 메서드를 사용하면 리액트 컴포넌트 요소를 재정의 할 수 있습니다. 그래서 아래처럼 Children으로 오는 컴포넌트에게 selected와 index라는 prop을 전달할 수 있게 됐습니다.

하지만 단점으로 Tab을 사용하는 외부에도 이 값이 노출될 수 있습니다. prop으로 넘겨준 만큼 사용처에서도 이 prop을 넣어줄 수 있습니다. 우려가 되는 부분이지만 이를 모두가 알고 사용하지 않는다면 문제가 생기지 않습니다! (이를 알릴 수 있는 방법도 궁금해요..)

{children.map((tabItem, index) =>
  cloneElement(tabItem as React.ReactElement<TabProps>, {
    ref: (el: HTMLLIElement) => (tabRefs.current[index] = el),
    selected: activeTabIndex === index,
    index,
  }),
}

비슷한 방식으로 여기서 인덱스를 바꾸는 setter도 이렇게 넘겨줄 수 있지 않을까? 생각할 수 있지만 의도적으로 넘겨주지 않았습니다. setter는 상태를 변경시키는 함수로 외부에 노출시키고 싶지 않았기 때문입니다. 그래서 setter 만큼은 context를 사용해서 tab으로 넘겨주는 방식을 사용했습니다.

Tab은 Tabs에 종속된 컴포넌트이므로 context에 종속되어도 크게 문제가 되지 않는다고 생각해서 추가했습니다.

const {handleActiveTabIndex} = useTabContext();

Tab을 클릭할 때 onClick 메서드 추가

탭을 클릭하여 인덱스를 조정하기 전, 특정 기능을 수행하고 싶을 때를 위해 onClick 메서드를 넣어줬습니다.
이 메서드는 인덱스를 조정하는 메서드가 아닙니다

이 기능을 추가한 이유는 Amplitude 트래킹 코드에서 탭을 클릭할 때 track 요청을 하고 싶어서 추가했습니다.

중점적으로 리뷰받고 싶은 부분

리팩터링한 구조가 적절한지, 더 좋은 방법이 있을지 리뷰 받고 싶어요.

🫡 참고사항

[github-actions]

🚀 storybook: https://66a9dea5db27afc2b8f91ba7-djnwfwwimd.chromatic.com/

[Todari]

고생 많았어요!!!!!!!
comment 그냥 읽고 생각만 해보세용~!~!

[Todari]

초기에 Tabs에 대한 type의 설계가 애매하게 된 것 같네요 ㅜㅜ
TabProps를 가진 애들만 children으로 사용할 수 있고... content는 따로 ReactNode를 받으면서 그 ReactNode의 prop들은 또 LIElement ref를 받고...

[Todari]

만약 제가 다시 설계한다면,
LIElement를 extends해서 받고, label: string과 content: ReactNode 대신 이를 하나로 합쳐서 right: ReactNode 를 받아서 넘겨줄 것 같아요!
그러고 index는 tabs 내부에서 ReactChildren을 이용해서 찾아낼 것 같습니당

참고만 하시고 쿠키가 편한 방식으로 구현해 주세요!!!

[Todari]

접근성 고려가 되면 좋을 것 같아요!!

  const handleKeyDown = (e: React.KeyboardEvent) => {
    const tabCount = children.length;
    
    switch(e.key) {
      case 'ArrowLeft':
        e.preventDefault();
        setActiveTabIndex(prev => (prev - 1 + tabCount) % tabCount);
        tabRefs.current[activeTabIndex]?.focus();
        break;
      case 'ArrowRight':
        e.preventDefault();
        setActiveTabIndex(prev => (prev + 1) % tabCount);
        tabRefs.current[activeTabIndex]?.focus();
        break;
      case 'Home':
        e.preventDefault();
        setActiveTabIndex(0);
        tabRefs.current[0]?.focus();
        break;
      case 'End':
        e.preventDefault();
        setActiveTabIndex(tabCount - 1);
        tabRefs.current[tabCount - 1]?.focus();
        break;
    }
  };

[Todari]

{children.map((tabItem, index) =>
            cloneElement(tabItem as React.ReactElement<TabProps>, {
              ref: (el: HTMLLIElement) => (tabRefs.current[index] = el),
              selected: activeTabIndex === index,
              tabIndex: activeTabIndex === index ? 0 : -1,
              index,
            }),
          )}

그리고 이게 되려면 Tab.type.ts에 남긴 comment 처럼 TabProps type 자체를 좀 더 넓혀야 가능할 것 같아요.
LIElement를 extends해서 구현하거나, 다른 방법을 생각해보면 좋을 것 같습니다!!