Refactor/button: Button 컴포넌트 리팩토링

[se030]

🤠 개요

• Closes Refactor: Button 컴포넌트 리팩토링 #96

• Button 컴포넌트 리팩토링 작업입니다.

💫 설명

• ButtonVariant 타입에 "plain"이 추가되었습니다. background와 border를 제외한 스타일링을 제공합니다.

• aria-label로 사용하기 위한 label을 필수 props로 추가했습니다.

• 내부 컨텐츠에 대한 책임을 컴포넌트 외부로 분리합니다. 아래와 같이 사용할 수 있습니다.

  const Template: ComponentStory<typeof Button> = (args) => (
    <Button {...args}>
      <MdCelebration />
      상장하기
    </Button>
  );

  • 이 변동에 따라 다른 컴포넌트 스토리들에 대수정이 필요합니다. 🙇🏻‍♀️🙇🏻‍♀️🙇🏻‍♀️

  • 해당 작업 이후 머지할 예정이니 리뷰 천천히 부탁드려요 ..👀

• a 태그를 사용하는 버튼을 ButtonLink 컴포넌트로 분리했습니다.

  • props나 디자인, 사용 방식에는 큰 차이가 없어 기존 스토리를 사용했는데 분리해야 한다고 생각하시면 말씀해주세요!

• 각 컴포넌트 props에 HTML 기본 속성들을 포함시켰습니다.

고민한 부분

TL;DR

• 버튼 인터페이스를 네이티브 HTML처럼 사용하도록 개선해보려고 합니다.

  • 개발자에게 더 예상 가능한 인터페이스를 제공하고,

  • Button 컴포넌트에서 Icon에 대한 책임을 분리할 수 있습니다.

  <Button>텍스트</Button>
  
  <Button>
    <Icon size={} source={} /> 텍스트
  </Button>
  
  <Button>
    <Icon size={} source={} />
  </Button>
  
  <Button>
    텍스트 <Icon size={} source={} />
  </Button>

• a 태그를 별도 컴포넌트로 만듭니다.

• 아이콘 관련 props가 너무 많고 직관적인 사용이 어렵다는 나쁜 느낌이 들어요. 초반에 IconTypography가 찝찝했던 이유와 같아요.

  

    <Button
      icon={MdCelebration}
      text="상장하기"
      href="https://github.com/c-h-w-h/cds"
      label="차가운 디자인 시스템 레포지토리 링크"
    />

  그래서 스타일만 남겨두되 이런식으로 사용하게 하면 어떨까 생각도 들어요. 어떤 방식이 더 괜찮은가요? 의견을 듣고싶어요.

  <Button
    href="https://github.com/c-h-w-h/cds"
    label="차가운 디자인 시스템 레포지토리 링크"
  >
     <MdCelebration />
     상장하기
  </Button>

• 현재 Button 컴포넌트에는 button 태그와 a 태그가 사용되고 있어요. 링크의 역할을 하는 요소와 버튼이 동일한 디자인을 가지는 경우가 많아서인데요.

  아래처럼 버튼의 HTML 기본 속성을 사용하고 싶은데, 내부 태그가 두개이다보니 어떻게 해야할지 고민돼요.

  사실은 저거 전부 구조분해할당으로 가져와서 button에만 넣어주면 되긴하는데.. 별로 아름답질 못해서 ㅠㅠ 혹시 Link 컴포넌트를 따로 두는게 좋을까요? 저는 로그인 버튼, 문자나 전화 버튼, 아이콘 링크 등을 생각해보면 Button으로 사용하고 싶을 것 같거든요.

  들어오는 props에 따라서 타입을 바꿔주는건 타입스크립트 동작 단계를 생각했을 때 불가능한 것 같아요. emotion as props에 따라서 제네릭으로 유동적인 타입을 쓸 수 없을까 엄청 많이 시도해봤는데 실패했었는데, 지금보니 당연히 안되는거라고 생각돼요..

  interface ButtonHTMLAttributes<T> extends HTMLAttributes<T> {
      autoFocus?: boolean | undefined;
      disabled?: boolean | undefined;
      form?: string | undefined;
      formAction?: string | undefined;
      formEncType?: string | undefined;
      formMethod?: string | undefined;
      formNoValidate?: boolean | undefined;
      formTarget?: string | undefined;
      name?: string | undefined;
      type?: 'submit' | 'reset' | 'button' | undefined;
      value?: string | ReadonlyArray<string> | number | undefined;
  }

📷 스크린샷 (Optional)

[prayinforrain]

아이콘 관련 props가 너무 많고 직관적인 사용이 어렵다는 나쁜 느낌이 들어요. 초반에 IconTypography가 찝찝했던 이유와 같아요.

동의해요. 지금 떠오르는 방법만 대충 정리해 볼게요.

• iconSize는 사실 react-icons에서 1em을 기본값으로 주고 있기 때문에 사실 Typography 들어가는 부분만 조금 손본다면 따로 지정하지 않아도 text와 글자크기를 맞추거나 - Button 컴포넌트 자체에서 그냥 textSize로 받아서 통일해 줄 수 있을 것 같아요. 우리 아이콘이 react-icons 컴포넌트만 받는 건 아니지만 이미지 아이콘도 비슷하게 맞추기 쉬우니까 이 예시만 들었어요.
• iconPosition이 left | right 뿐이라면 차라리 leftIcon, rightIcon으로 받는게 나을 수도 있지 않을까.. 이건 chakra UI에서 사용하는 방법이에요.
• iconTranslateY는 지금 보니 없어도 되지 않나 하는 생각이 들어요. 윈도우만 그런지 모르겠는데 지금 적용된 스토리 보면 좀.. 위로 쏠려있어요.

그리고 사용 방법에 대해서는.. children을 받는게 native HTML같아서 더 직관적이긴 한데 실제 사용된 코드를 보면 Props로 받는게 더 가독성이 좋을 것 같아서, 취향차이(?)이지 않을까.. 😅

현재 Button 컴포넌트에는 button 태그와 a 태그가 사용되고 있어요. 링크의 역할을 하는 요소와 버튼이 동일한 디자인을 가지는 경우가 많아서인데요.

걱정되는 부분이 있어요. button과 anchor는 서로 다른 역할을 가지고 있고, 분리되어야 한다고 생각해요. 물론 버튼 모양을 한 링크 - plain Text 버튼이 필요할 수는 있지만, 이걸 Button이라는 이름의 컴포넌트가 지원한다면 이름은 버튼이지만 a태그의 역할을 하거나, a태그로 이루어진 요소가 되어 혼동을 줄 것 같아요. 물론 디자인이 중심이 되어야겠지만 Button 컴포넌트가 a 태그까지 관리하는 건 약간 범위를 넘는(?) 것 같아요.
둘을 분리한다면 말씀하신 문제가 해결될 것 같은데, 어떻게 생각하시는지 궁금해요!

지금 코드 볼 상황이 아니어서 리뷰는 조금만 나중에 할게요 😭

[se030]

그리고 사용 방법에 대해서는.. children을 받는게 native HTML같아서 더 직관적이긴 한데 실제 사용된 코드를 보면 Props로 받는게 더 가독성이 좋을 것 같아서, 취향차이(?)이지 않을까.. 😅

ㅋㅋㅋㅋㅋㅋ 취향 차이가 맞는 것 같아요 ... 제 취향이 확고하지 못할뿐 .. 말씀하신대로 스타일 관련 props들은 제거해도 괜찮을 것 같아요. 내부 배치를 선언적으로 보여주는건 네이티브 인터페이스라는 생각도 들고 ..ㅎㅎ Icon 컴포넌트가 이미 만들어져 있어서 사용자가 가져다 사용하면 되거든요.

걱정되는 부분이 있어요. button과 anchor는 서로 다른 역할을 가지고 있고, 분리되어야 한다고 생각해요. 물론 버튼 모양을 한 링크 - plain Text 버튼이 필요할 수는 있지만, 이걸 Button이라는 이름의 컴포넌트가 지원한다면 이름은 버튼이지만 a태그의 역할을 하거나, a태그로 이루어진 요소가 되어 혼동을 줄 것 같아요. 물론 디자인이 중심이 되어야겠지만 Button 컴포넌트가 a 태그까지 관리하는 건 약간 범위를 넘는(?) 것 같아요. 둘을 분리한다면 말씀하신 문제가 해결될 것 같은데, 어떻게 생각하시는지 궁금해요!

비슷한 부분에서 찝찝한 마음이 들었어요. 그리고 어찌됐든 네이티브 속성들을 props로 사용할 수 있어야 하는게 맞다고 생각해요.
LinkButton 정도로 분리해보면 어떨까요 ..? 라이브러이에서 제공하는 형태를 공유한다 점이 어느정도 드러났으면하고, Link/Anchor로 분리하기에는 텍스트 내부에 포함되는 링크 등등과 구분될 필요가 있다고 생각돼서요.

[iyu88]

-- 아이콘 관련

아이콘 관련해서 분리해도 괜찮지만 일단 둬도 괜찮을 것 같아요.
개인적으로 props 이름으로 충분히 잘 읽히고 가독성이 완전 (매우) 나쁜 편은 아닌 것 같아요.
props를 왕창 사용하고 있는 버튼들을 아직 못봐서 그런 것일 수도 있지만요.

-- 링크 관련

원래는 Button(Anchor)에 href와 onClick가 동시에 부여해도 되나 싶어서 찾아보니까
그렇게 하는 경우가 더러 있는 것 같더라구요.
어색하다고 생각했는데 괜찮을 것 같기도 해요.
근데 또 생각이 드는게 form에 있는 버튼에 href가 있다면?
뭔가 막을 방법이 있을까요? 🤔

[leesunmin1231]

많은 고민의 흔적이 느껴져요.. ㅎㅎ 고생하셨습니다~~~

[leesunmin1231]

이번에 버튼 써보면서 느낀건데, 그 html button 태그처럼 text를 props말고 children으로 줄 수 있으면 좋겠어요..!! button은 당연히 이렇게 동작하겠지 하는 부분들은 그렇게 동작해야 사용하기 편할거 같아요!

[se030]

반영하겠습니다!

[leesunmin1231]

고생하셨어요~~!! 훅으로 스타일 로직 분리해주셔서 코드 읽기 편했어요. 굿굿 👍

[iyu88]

수고하셨어요~
코멘트만 확인 부탁드립니다

[prayinforrain]

지금 코드 볼 상황이 아니어서 리뷰는 조금만 나중에 할게요 😭

나중에라고 해놓고 3주 지나서 보는 스스로가 얼탱이가 없네요..

[prayinforrain]

이거 나중에 흰색 배경도 같이 색이 변했으면 좋겠어요. 저는 보통 filter: brightness를 사용하긴 하는데 나중에 컬러 팔레트가 고쳐지면 수정해요!