-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Re-implement Modal
component using Dialog
in Radix UI
#1036
Re-implement Modal
component using Dialog
in Radix UI
#1036
Conversation
🦋 Changeset detectedLatest commit: 3b2e1a4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
Codecov ReportBase: 71.18% // Head: 71.72% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## next-v1 #1036 +/- ##
===========================================
+ Coverage 71.18% 71.72% +0.53%
===========================================
Files 208 217 +9
Lines 2978 3063 +85
Branches 818 842 +24
===========================================
+ Hits 2120 2197 +77
- Misses 739 742 +3
- Partials 119 124 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Chromatic Report🚀 Congratulations! Your build was successful! |
Modal
component using Dialog
in Radix UIModal
component using Dialog
in Radix UI
<Styled.HeadingGroup | ||
role="group" | ||
aria-roledescription="Heading group" | ||
> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
packages/bezier-react/src/components/Modals/Modal/ModalHeader.tsx
Outdated
Show resolved
Hide resolved
packages/bezier-react/src/components/Modals/Modal/ModalHeader.tsx
Outdated
Show resolved
Hide resolved
c7ed98b
to
3ce9a92
Compare
leftContent={leftContent} | ||
rightContent={rightContent} | ||
titleSize={titleSize} | ||
hidden={hidden} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hidden prop의 경우에는 사용처에서 어떤 식으로 활용할 수 있는걸까요? (어떻게 사용해야 접근성 측면에서 올바른 사용법인지)
관련 링크가 있다면 알려주심 감사하겠습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…) to document.body
…ate new stacking context
…lled modal's initial state
fd7c5b3
to
49a05d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
packages/bezier-react/src/components/Modals/Modal/ModalHeader.tsx
Outdated
Show resolved
Hide resolved
…TMLDivElement to HTMLElement
Self Checklist
CODEOWNERS
file.Related Issue
Modal
,ConfirmModal
component usingDialog
,AlertDialog
in Radix UI #978 (1/2)ConfirmModal
을 radix-ui의AlertDialog
를 사용하도록 재구현할 예정입니다.Summary
Radix UI의 Dialog 컴포넌트를 사용하여
Modal
컴포넌트를 재구현합니다.Details
styled
사용 없이 쉽게 할 수 있도록ModalContentProps
에width
,height
속성을 추가합니다.Anatomy
AS-IS
TO-BE
Modal
: RadixDialog.Root
를 추상화한 컴포넌트입니다. Context의 역할만 담당하고, 컴포넌트를 렌더하지는 않습니다. 모달의 상태와 핸들러를 관리합니다.ModalContent
: RadixDialog.Portal
,Dialog.Overlay
,Dialog.Content
를 추상화한 컴포넌트입니다. 모달 전체가 렌더되는 위치, 스타일을 관리합니다.ModalHeader
: 기존ModalContent
가 담당하던 역할 중 제목에 대한 역할만을 담당하는 컴포넌트입니다. 제목, 부제목, 설명을 렌더하는 책임을 가집니다.ModalFooter
: 기존ModalAction
과 동일한 책임을 가진 컴포넌트입니다.ModalBody
: 모달 내용물을 감싸는 단순한 Wrapper 프리셋 컴포넌트입니다.ModalTrigger
: 모달을 여는 버튼을 감쌀 때 사용할 수 있는 헬퍼 컴포넌트입니다. 자신은 렌더하지 않고 하위 컴포넌트(children)에게 자신의 속성을 전달합니다. 속성에는 모달의 상태에 관련한 접근성 속성이 포함되어 있습니다. 하위 컴포넌트를 클릭하면Modal
의onShow
핸들러를 호출합니다. 비제어 컴포넌트로 사용할 경우 모달을 엽니다.ModalClose
: 모달을 닫는 버튼을 감쌀 때 사용할 수 있는 헬퍼 컴포넌트입니다. 자신은 렌더하지 않고 하위 컴포넌트(children)에게 자신의 속성을 전달합니다. 하위 컴포넌트를 클릭하면Modal
의onHide
핸들러를 호출합니다. 비제어 컴포넌트로 사용할 경우 모달을 닫습니다.Design Decision
컴포넌트 추가 및 네이밍 변경
Radix의 Dialog의 구현상 기존의
Modal
(=BaseModal
),ModalContent
,ModalAction
3가지 컴포넌트로 이루어진 구성으로는 사용하기 어려웠습니다. 특히Modal
과ModalContent
컴포넌트의 인터페이스를 그대로 유지한 채 Radix Dialog를 사용해서 구현하게 되면Dialog.Trigger
사용이 불가능한 점이 걸렸습니다. 기존엔Modal
컴포넌트가 모달 Wrapper를 그리는 책임, Portal을 열고 렌더하는 책임을 담당하고 있었는데 이 경우 children으로 기존 렌더 트리아래 그려져야 할 트리거 컴포넌트를 넘겨줄 수 없었기 때문입니다. 마찬가지로,Modal
이Portal
의 역할을 담당하게 하지 않으려면(= 트리거 컴포넌트를 사용하려면) 기존처럼ModalContent
와ModalAction
컴포넌트를 병렬로 배치하는 방식도 사용이 불가능했습니다.새로운 모달 컴포넌트는 비제어형 컴포넌트로도 쓰일 수 있었으면 했고, 접근성 속성도 꼭 챙겼으면 했습니다. 추후 확장 가능성, 마이그레이션 비용을 고려해보면 어느정도 브레이킹 체인지가 있더라도 괜찮다고 생각했습니다. 상기한대로
ModalContent
가 Portal, Overlay의 역할을 가지게 되면서 기존 구조를 그대로 가져가기가 어려웠으므로, 기존 피그마와 통일했었던 구조에서(#734 (review)) 다시ModalHeader
,ModalBody
,ModalFooter
를 가지는 구조로 변경하게 되었습니다(#). 구조 변경에 따른 인터페이스 변경도 함께 진행했습니다.Examples
Controlled
채널 데스크 어플리케이션에서 사용하는 패턴입니다. 트리거 컴포넌트와 모달 컴포넌트의 거리가 먼 경우, 같은 컨텍스트를 공유하지 못하는 경우엔
ModalTrigger
없이도 사용할 수 있습니다(접근성 속성을 챙기지 못한다는 점은 아쉽습니다).Uncontrolled
비동기 로직 등이 없는 간단한 모달의 경우 별도의 상태 없이도 구현이 가능합니다.
TODO
Breaking change or not (Yes/No)
Yes
Modal
의 내부 구현에BaseModal
을 사용하지 않습니다.Modal
은LegacyModal
로 이름이 변경되며, 후속 PR에서 제거됩니다.ModalAction
컴포넌트의 이름이ModalFooter
로 변경됩니다.ModalProps
의targetElement
속성의 이름이container
로 변경됩니다.showCloseIcon
속성이ModalProps
에서ModalContentProps
로 옮겨집니다.title
,subTitle
,description
,titleSize
속성이ModalContentProps
에서 새로운ModalHeaderProps
로 옮겨집니다.References