Skip to content

Conversation

@sungik-choi
Copy link
Contributor

@sungik-choi sungik-choi commented Apr 7, 2023

Self Checklist

  • I wrote a PR title in English.
  • I added an appropriate label to the PR.
  • I wrote a commit message in English.
  • I wrote a commit message according to the Conventional Commits specification.
  • I added the appropriate changeset for the changes.
  • [Component] I wrote a unit test about the implementation.
  • [Component] I wrote a storybook document about the implementation.
  • [Component] I tested the implementation in various browsers.
    • Windows: Chrome, Edge, (Optional) Firefox
    • macOS: Chrome, Edge, Safari, (Optional) Firefox
  • [New Component] I added my username to the correct directory in the CODEOWNERS file.

Related Issue

Fixes #707

Summary

SegmentedControl 컴포넌트를 재구현합니다.

// Anatomy of the SegmentedControl type="radiogroup"
<SegmentedControl type="radiogroup">
  <SegmentedControlItem />
</SegmentedControl>

// Anatomy of the SegmentedControl type="tabs"
<SegmentedControl type="tabs">
  <SegmentedControlTabList>
    <SegmentedControlItem />
  </SegmentedControlTabList>
  <SegmentedControlTabContent />
</SegmentedControl>

Details

SegmentedControl 컴포넌트는 라디오 그룹, 탭 2개 용도로 사용될 수 있는 컴포넌트입니다.

형태가 완벽히 동일한 컴포넌트를 다른 이름(e.g. SegmentedControlRadio, SegmentedControlTabs)으로 export하는 건 디자인 시스템의 컴포넌트 스펙과도 일치하지 않고(= 컴포넌트가 2개가 아님), 너무 서술적이라고 생각했습니다.

이 PR에서는 SegmentedControl 이라는 하나의 컴포넌트와 type 속성을 통해 런타임에서 다른 컴포넌트로 사용할 수 있는 방식을 선택하여 구현했습니다.

컴포넌트 구조

컴포넌트 구조는 아래와 같습니다.

  • 실선이 향하는 컴포넌트는 라이브러리 사용처에서 접근할 수 있는, 즉 export된 컴포넌트입니다.
  • 점선이 향하는 컴포넌트는 내부적으로 해당 컴포넌트가 호출하고 있는 컴포넌트입니다.
flowchart TD
    C(Consumer) --> S[SegmentedControl]
    C --> I[SegmentedControlItem]
    C --> TL[SegmentedControlTabList]
    C --> TC[SegmentedControlTabContent]

    S -.-> T{type}
    T -.->|radiogroup| A[SegmentedControlRadioGroup]
    T -.->|tabs| B[SegmentedControlTabs]
    B -.-> Lib::RadixUI.Tabs.Root
    A -.-> L[SegmentedControlItemList]
    TL -.-> L

    L -.-> T2{type}
    T2 -.->|radiogroup| Lib::RadixUI.RadioGroup.Root
    T2 -.->|tabs| Lib::RadixUI.Tabs.List

    I -.-> T3{type}
    T3 -.-> |radiogroup| Lib::RadixUI.RadioGroup.Item
    T3 -.-> |tabs| Lib::RadixUI.Tabs.Trigger

    subgraph tabs-only components
        TL
        TC -.-> Lib::RadixUI.Tabs.Content
        end
Loading

내부 구현은 다른 폼 컴포넌트와 마찬가지로 Radix UI의 RadioGroup, Tabs 컴포넌트에 의존하고 있습니다. 두 컴포넌트는 비슷하게 동작하기도 하면서, 컴포넌트를 조합하는 방식은 또 크게 다릅니다.

Radix의 Tabs 의 경우 가장 바깥의 Root 컴포넌트 + 각 탭(Trigger)을 묶는 List 컴포넌트 + 각 컨텐츠를 랩핑하는 Content 컴포넌트 총 4개 컴포넌트의 조합입니다. 그에 반해 RadioGroup 의 경우 각 라디오를 감싸는 Root 와 라디오 Item 2개 컴포넌트의 조합으로 구성되어 있습니다. RadioGroup.Root 의 경우엔 해당 컴포넌트가 상태 제어 + 아이템 간의 스타일링 모두를 담당하지만, Tabs.Root 의 경우엔 상태 제어만 담당, 아이템 간의 스타일링은 Tabs.List 컴포넌트가 담당합니다. 따라서 두 케이스를 동일한 JSX문으로 처리하기는 어려웠습니다.

정확히 말하자면 SegmentedControltabs 타입일 경우, prop으로 TabContent 컨텐츠(e.g. Array<{ value: string, content: React.Node }>)를 주입하는 방식으로 구현 가능합니다. 하지만 라이브러리 전체를 봤을 때, Radix를 통해 재구현한 다른 컴포넌트들과 일관적인 사용 방식을 유지하기 위해선 조합형으로 구현하는 방향이 옳다고 판단했습니다.

// 이런 형태는 어렵습니다.
<SegmentedControl type="radiogroup|tabs">
  <SegmentedControlItem />
</SegementedControl>

완벽히 동일한 JSX 표현식의 형태를 띄진 못하더라도, Root(SegmentedControl) -> Item(SegmentedControlItem)으로 이어지는 위계는 모든 타입에 일관적으로 가져가고 싶었습니다. 하지만 상술했듯 RadioGroup, TabsRoot 컴포넌트의 역할이 각기 달랐기 때문에, 내부적으로 SegmentedControlItemList 컴포넌트를 구현하여 이를 해결하고자 했습니다.

조건부 로직 & Radix에 관련된 부분은 제거하고, 컨텍스트를 중점으로 그린 플로우차트는 아래와 같습니다.

flowchart TD
    Consumer ==> SegmentedControl
    Consumer ==> SegmentedControlTabList
    Consumer ==> SegmentedControlItem
    SegmentedControl -.-> S
    S[(SegmentedControlContext)] -.-> SegmentedControlItemList
    S[(SegmentedControlContext)] -.-> SegmentedControlItem
    SegmentedControl --> SegmentedControlRadioGroup
    SegmentedControl --> SegmentedControlTabs
    SegmentedControlRadioGroup --> SegmentedControlItemList
    SegmentedControlTabList --> SegmentedControlItemList
    SegmentedControlItemList -.-> SI
    SI[(SegmentedControlItemListContext)] -.-> SegmentedControlItem
Loading
  • SegmentedControl상태에 관련된 속성을 각 타입별 컴포넌트에 prop을 통해 주입합니다.
    • SegmentedControlTabs: 상태에 대한 요구사항 충족. (완료) 그러나 컴포넌트 전체적으로 스타일링은 이루어지지 않은 상태입니다.
    • SegmentedControlRadioGroup: 상태에 대한 요구사항은 충족되었으나, 스타일링에 대한 요구사항은 충족되지 않음
  • 스타일 속성을 prop을 통해 주입할 경우, tabs 타입에서 스타일을 담당하는 Tabs.List 컴포넌트에게 스타일 속성을 전달해줄 수 없습니다. 따라서 스타일에 관련된 속성(size, width)은 컨텍스트에 담아 전달합니다(className, style 의 경우 컨텍스트를 통해 다른 컴포넌트에게 전달하면 기대하는 일반적인 동작 방식-타게팅한 컴포넌트를 직접 스타일링할 것이다-을 위반하므로 포함하지 않았습니다).type 속성또한 전역적으로 사용되므로 컨텍스트에 포함합니다.
    • SegmentedControlRadioGroup : 상태에 대한 요구사항 충족, 컨텍스트를 통해 size, width 속성에 접근하여 스타일링에 대한 요구사항또한 충족합니다. (완료)
  • SegmentedControlTabList: 상태에 대한 책임 없이 스타일링에 대한 책임만을 담당하므로 해당 속성을 컨텍스트를 통해 접근하여 사용. tabs의 케이스에도 상태/스타일링 요구사항이 모두 충족됩니다.
  • SegmentedControlRadioGroup, SegmentedControlTabList 의 중복 구현을 제거하기 위해 공통 로직 + 스타일이 포함된 컴포넌트를 SegmentedControlItemList 로 분리합니다.

결과적으로 아래와 같이 동작하게 됩니다.

radiogroup 타입일 경우

  • SegmentedControlItemList(RadioGroup) 컴포넌트는
    • SegmentedControl 로부터 상태 관련 속성을 prop drilling으로 주입
    • SegmentedControlContext 로부터 스타일링 관련 속성을 사용

tabs 타입일 경우

  • SegmentedControlTabs 컴포넌트는
    • SegmentedControl 로부터 상태 관련 속성을 prop drilling으로 주입
  • SegmentedControlItemList(TabList) 컴포넌트는
    • SegmentedControlContext 로부터 스타일링 관련 속성을 사용

인디케이터 애니메이션

SegmentedControlItemListContext 를 통해 선택된 아이템 엘리먼트를 가져와서, 해당 엘리먼트의 포지션을 계산하여 이동하는 방식으로 구현했습니다.

이전 구현과 다른 점은, 초기값이 없는 상태가 생기면서 애니메이션이 발생 시 좌측 상단 스타트 포지션에서 타겟 포지션까지 이어지는 트랜지션의 어색함이 두드러졌습니다. 초기 DOM Node Attach 이후 잠시동안 트랜지션을 비활성화하는 방식으로 해결했습니다.

Breaking change or not (Yes/No)

Yes

Re-implement SegmentedControl component. Legacy components are exported to the LegacySegmentedControl namespace.

References

@changeset-bot
Copy link

changeset-bot bot commented Apr 7, 2023

🦋 Changeset detected

Latest commit: f592c9a

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

@sungik-choi sungik-choi self-assigned this Apr 7, 2023
@sungik-choi sungik-choi added the #reimplementation Issue or PR related to Reimplementation of existing components (#1105) label Apr 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2023

Chromatic Report

🚀 Congratulations! Your build was successful!

@sungik-choi sungik-choi force-pushed the feat/segmented-control branch 2 times, most recently from c074744 to f73ace4 Compare April 11, 2023 10:06
@sungik-choi sungik-choi changed the title Feat/segmented control Reimplement SegmentedControl component Apr 11, 2023
@sungik-choi sungik-choi changed the title Reimplement SegmentedControl component Re-implement SegmentedControl component Apr 11, 2023
@codecov
Copy link

codecov bot commented Apr 12, 2023

Codecov Report

Patch coverage: 80.75% and project coverage change: +0.28 🎉

Comparison is base (b2629ec) 77.76% compared to head (f592c9a) 78.05%.

Additional details and impacted files
@@             Coverage Diff             @@
##           next-v1    #1272      +/-   ##
===========================================
+ Coverage    77.76%   78.05%   +0.28%     
===========================================
  Files          302      309       +7     
  Lines         3855     3974     +119     
  Branches       849      864      +15     
===========================================
+ Hits          2998     3102     +104     
- Misses         577      586       +9     
- Partials       280      286       +6     
Impacted Files Coverage Δ
...s/Forms/SegmentedControl/SegmentedControl.types.ts 100.00% <ø> (ø)
...act/src/components/Forms/SegmentedControl/index.ts 0.00% <0.00%> (ø)
...act/src/components/LegacySegmentedControl/index.ts 0.00% <0.00%> (ø)
...ySegmentedControl/LegacySegmentedControl.styled.ts 60.37% <60.37%> (ø)
.../Forms/SegmentedControl/SegmentedControl.styled.ts 75.00% <71.42%> (+14.62%) ⬆️
.../Forms/SegmentedControl/SegmentedControlContext.ts 84.61% <84.61%> (ø)
.../LegacySegmentedControl/LegacySegmentedControl.tsx 90.32% <90.32%> (ø)
...ts/Forms/SegmentedControl/SegmentedControlItem.tsx 90.47% <90.47%> (ø)
...onents/Forms/SegmentedControl/SegmentedControl.tsx 97.05% <96.55%> (+6.73%) ⬆️
...rms/SegmentedControl/SegmentedControlIndicator.tsx 96.55% <96.55%> (ø)
... and 2 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sungik-choi sungik-choi force-pushed the feat/segmented-control branch from 4b142d0 to 8110af0 Compare April 12, 2023 07:59
@sungik-choi sungik-choi added the a11y Issue or PR related to accessibility label Apr 12, 2023
@sungik-choi sungik-choi marked this pull request as ready for review April 13, 2023 09:28
Copy link
Contributor

@Dogdriip Dogdriip left a comment

Choose a reason for hiding this comment

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

💯

@sungik-choi sungik-choi mentioned this pull request Apr 25, 2023
9 tasks
@sungik-choi sungik-choi merged commit 07dc6ed into channel-io:next-v1 May 12, 2023
@sungik-choi sungik-choi deleted the feat/segmented-control branch May 12, 2023 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a11y Issue or PR related to accessibility #reimplementation Issue or PR related to Reimplementation of existing components (#1105)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Re-implement SegmentedControl component

2 participants