bezier-react icon indicating copy to clipboard operation
bezier-react copied to clipboard

add withoutParallelIndent prop and apply Radix to Divider component

Open Dogdriip opened this issue 2 years ago • 7 comments

Summary

해당 PR은 Radix 도입 및 Bezier와의 integration에 대한 PoC 정도로, 여기서 충분한 논의를 거쳐 적용 방식 등을 통일한 후 점진적으로 적용해 나가면 좋겠습니다.

See also: #788

  • [x] Divider 컴포넌트에 withoutParallelIndent prop을 추가합니다.
  • [x] Divider 컴포넌트에 Radix를 적용합니다.
  • [x] ARIA Role 등에 기반한 단위 테스트를 추가합니다.

Details

변경점

  • BREAKING_CHANGE: Divider가 더 이상 HTMLHRElement가 아닌 HTMLDivElement입니다.
    • 기존 사용처에서 & > hr 등과 같이 태그 이름을 지정해 스타일링한 경우가 있다면 마이그레이션이 필요합니다.
  • withoutParallelIndent?: boolean = false prop을 추가합니다.
    • Divider와 수평 방향의 여백 (horizontal의 경우 상하 여백, vertical의 경우 좌우 여백)의 유무를 조절합니다.
    • 기존의 'Divider 양 끝'의 여백 유무를 조절하던 withoutSideIndent와 네이밍을 최대한 맞췄습니다.
  • withoutIndent?: boolean = false prop을 추가합니다.
    • Divider의 모든 방향 여백 유무를 조절합니다.
  • Radix를 이용해 Accessibility를 강화합니다.
    • decorative: boolean | undefined prop을 추가합니다.
      • 정말로 semantic하게 컨텐츠를 나누는 것인지, 단순한 graphical seperator인지를 나타냅니다. ARIA 속성의 적용 여부를 조절합니다.
    • 이외의 Radix 도입과 관한 이야기는 하술합니다.
  • Rounding에 round1을 추가합니다.

Radix 도입과 관한 이야기

  • Radix Primitive(이하 Radix)에서는 A11y 속성이 적용된 컴포넌트를 부분별로 제공합니다.
    • 이 PR은 Separator를 적용했습니다.
    • '부분별로 제공'한다는 말은 더 복잡한 컴포넌트에서 이해할 수 있습니다. Slider의 경우, Root, Track, Range, Thumb를 각각 제공합니다.
  • 관건은 '이를 Bezier와 어떻게 integrate할 것인가'입니다. 아래는 제가 시도했던 방법들과 각 방법에 대한 생각입니다.
    • ❌ Radix 컴포넌트에 그대로 bezier-reactstyled 등을 사용합니다.
      • Foundation 등이 적용되지 않습니다.
    • ⚠️ Radix 컴포넌트(Primitive)나 @radix-ui/react-polymorphic을 이용한 Polymorphic component를 extend하여 bezier를 주입합니다.
      • @radix-ui/react-polymorphic는 Deprecated되었으며, 아래 방법이 훨씬 간단하기 때문에 굳이 이렇게까지 할 필요는 없다고 생각했습니다.
    • asChild prop을 이용해 bezier 컴포넌트에 accessibility를 주입합니다.
      • Radix Primitive를 단순히 child 컴포넌트에 accessibility나 기타 functional requirements를 주입하는 용도로 사용할 수 있습니다. 미리 구현된 디자인 시스템 등에 Radix를 사용할 경우 대부분 이렇게 사용하는 듯합니다.
      • 다음은 Divider.tsx의 일부입니다.
        <SeparatorPrimitive.Root
          asChild
          orientation={orientation}
          decorative={decorative}
        >
          <Styled.Divider
            ref={forwardedRef}
            data-testid={testId}
            orientation={orientation}
            decorative={decorative}
            withoutSideIndent={withoutSideIndent}
            withoutParallelIndent={withoutParallelIndent}
            withoutIndent={withoutIndent}
            {...rest}
          />
        </SeparatorPrimitive.Root>
        

Browser Compatibility

OS / Engine 호환성을 반드시 확인해주세요.

Windows

  • [x] Chrome - Blink
  • [ ] Edge - Blink
  • [ ] Firefox - Gecko (Option)

macOS

  • [x] Chrome - Blink
  • [ ] Edge - Blink
  • [x] Safari - WebKit
  • [x] Firefox - Gecko (Option)

References

  • Radix: https://www.radix-ui.com/
  • Radix Seperator: https://www.radix-ui.com/docs/primitives/components/separator
  • ARIA seperator: https://www.w3.org/TR/wai-aria-1.2/#separator

Dogdriip avatar Jul 12 '22 13:07 Dogdriip

🦋 Changeset detected

Latest commit: 486e8ed1a9cc52d4dedcbc03268dd927d658a661

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@channel.io/bezier-react Minor
bezier-figma-plugin Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Jul 12 '22 13:07 changeset-bot[bot]

Codecov Report

Merging #873 (486e8ed) into next-v1 (46caba3) will increase coverage by 0.13%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           next-v1     #873      +/-   ##
===========================================
+ Coverage    70.87%   71.00%   +0.13%     
===========================================
  Files          204      204              
  Lines         2884     2894      +10     
  Branches       797      807      +10     
===========================================
+ Hits          2044     2055      +11     
+ Misses         722      721       -1     
  Partials       118      118              
Impacted Files Coverage Δ
...ier-react/src/components/Divider/Divider.styled.ts 100.00% <100.00%> (ø)
...es/bezier-react/src/components/Divider/Divider.tsx 100.00% <100.00%> (+16.66%) :arrow_up:
...ages/bezier-react/src/foundation/Rounding/index.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Jul 14 '22 07:07 codecov[bot]

  • 윈도우 포함, 브라우저별로 잘 표시되는 지 확인부탁드려요!
  • 사방 마진을 제거하는 withoutIndent prop이 추가되면 사용처에서 더 쉽게 사용할 수 있을 거 같습니다. 아마 대부분 withoutSideIndent, withoutIndent 둘만 사용하게 될 거 같아요
  • 윈도우는 집 가서 체크해보겠습니당 🏡
  • 지금 props에서 withoutIndent를 하나 더 추가하자는 말씀이 맞을까요?

Dogdriip avatar Jul 15 '22 06:07 Dogdriip

  • 윈도우 포함, 브라우저별로 잘 표시되는 지 확인부탁드려요!
  • 사방 마진을 제거하는 withoutIndent prop이 추가되면 사용처에서 더 쉽게 사용할 수 있을 거 같습니다. 아마 대부분 withoutSideIndent, withoutIndent 둘만 사용하게 될 거 같아요
  • 윈도우는 집 가서 체크해보겠습니당 🏡
  • 지금 props에서 withoutIndent를 하나 더 추가하자는 말씀이 맞을까요?

네 추가하는 방향이 맞습니다! 테스트용 윈도우 노트북이 사내에 있으니, 크로마틱 배포된 스토리북으로 접속하셔서 확인하셔도 될거에요

sungik-choi avatar Jul 15 '22 07:07 sungik-choi

Browser Compatibility만 체크되면 Approve 하겠습니다!

sungik-choi avatar Jul 20 '22 12:07 sungik-choi

Browser Compatibility만 체크되면 Approve 하겠습니다!

최대한 체크해 보았는데 대부분의 브라우저에서 문제 없는 것 같습니다. PR 본문에 체크해두었으니 확인 부탁드립니다! 🙇

Dogdriip avatar Jul 21 '22 05:07 Dogdriip

Chromatic Report

🚀 Congratulations! Your build was successful!

github-actions[bot] avatar Jul 26 '22 05:07 github-actions[bot]