Enhance the `Modal` component #1077

sungik-choi · 2022-12-13T16:13:01Z

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

없음

Summary

Modal 컴포넌트를 애플리케이션에 적용하면서, 부족한 부분들을 개선합니다.

Details

오버레이가 스크롤 가능하도록 개선

Modal 내부에 Overlay 컴포넌트가 포함될 경우, Portal을 통해 동일하게 rootElement(body) 아래 렌더하는 케이스에서 Overlay 컴포넌트에 클릭이 불가능한 문제가 있었습니다. Modal 이 modal={true} 속성을 가져 외부에 이벤트가 전달되지 않기 때문입니다. 이를 해결하고자 overflow hidden 속성을 제거하고 오버레이 영역 전체가 스크롤 가능하게 변경했습니다. 애플리케이션에서는 Overlay container 속성 지정을 통해 Modal 내부에 그리는 방식으로 구현하게 됩니다.
관련해서 UI를 테스트해볼 수 있도록 스토리를 개선합니다.

`Modal` hide 애니메이션 제거

어플리케이션에서 onHide 콜백 호출 시, 즉시 특정 상태가 변경되며 UI가 바뀌는 케이스가 있습니다. 애니메이션이 있을 경우 의도치 않은 UI가 애니메이션 실행 시간동안 노출되는 문제가 있어 우선 제거합니다.

`ModalBody` 스타일 개선

ModalBody 가 다른 엘리먼트의 자식이 된 경우에도 스타일링이 잘 적용될 수 있도록 CSS 선택자를 개선합니다.

chore: 스토리북 개선

root에 css variable을 추가하여 오버레이 색상이 잘 표시되도록 합니다.

Breaking change or not (Yes/No)

No

References

https://www.radix-ui.com/docs/primitives/components/dialog#scrollable-overlay

changeset-bot · 2022-12-13T16:13:04Z

🦋 Changeset detected

Latest commit: 17cb0dd

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	Patch
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

github-actions · 2022-12-13T16:19:46Z

Chromatic Report

🚀 Congratulations! Your build was successful!

codecov · 2022-12-14T12:40:10Z

Codecov Report

Base: 72.07% // Head: 72.06% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (17cb0dd) compared to base (85ba690).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           next-v1    #1077      +/-   ##
===========================================
- Coverage    72.07%   72.06%   -0.02%     
===========================================
  Files          223      223              
  Lines         3051     3053       +2     
  Branches       840      840              
===========================================
+ Hits          2199     2200       +1     
- Misses         728      729       +1     
  Partials       124      124

Impacted Files	Coverage Δ
...-react/src/components/Modals/Modal/Modal.styled.ts	`100.00% <ø> (ø)`
...c/components/Modals/Modal/ModalAnimation.styled.ts	`75.00% <ø> (-8.34%)`	⬇️
...react/src/components/Modals/Modal/ModalContent.tsx	`86.66% <100.00%> (+2.05%)`	⬆️

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

… close icon

to prevent to show unintended changing of view(state)

junbong · 2022-12-22T10:52:19Z

packages/bezier-react/src/components/Modals/Modal/ModalAnimation.styled.ts

  closed: css`
    opacity: 0;
-    transform: translate(-50%, -48%) scale(0.95);
+    transform: translate(0, -2%) scale(0.95);


애니메이션 사용처에 will-change: transform; 넣어서 유의미한 퍼포먼스 차이가 있는지 체크 해보면 좋겠습니다.

유의미한 퍼포먼스 차이가 없어서, 이른 최적화라고 생각하여 추가하지 않으려고 합니다. (#)

packages/bezier-react/src/components/Modals/Modal/ModalContent.tsx

Dogdriip · 2022-12-22T12:13:47Z

packages/bezier-react/.storybook/preview.js

+      <BezierProvider foundation={foundation}>
+        <div style={wrapperStyle}>


Suggested change

<BezierProvider foundation={foundation}>

<div style={wrapperStyle}>

<div style={wrapperStyle}>

div 상위의 BezierProvider는 빠져도 될 것 같은데 맞을까요?

두개 테마만 사용되고 있으니 빠져도 괜찮아 보입니다 👀

root에 css variable을 추가하여 오버레이 색상이 잘 표시되도록 합니다.

themeVarsScope 를 제거해서 root에 CSS Variable을 생성하기 위해서 추가한 부분입니다.

(구두 싱크) 이해했습니당

Dogdriip · 2022-12-26T02:46:30Z

packages/bezier-react/.storybook/preview.js

+      <BezierProvider foundation={foundation}>
+        <div style={wrapperStyle}>


(구두 싱크) 이해했습니당

sungik-choi added enhancement Issues or PR related to making existing features better component labels Dec 13, 2022

sungik-choi self-assigned this Dec 13, 2022

sungik-choi added 8 commits December 22, 2022 19:21

fix(modal): fix style to be scrollable overlay

34bcfc5

test(story): add css variable to root

fac4705

test(modal): enhance the story to test overflow

b113a37

fix(modal): move z-index style from content to overlay

c2096ef

fix(modal): rollback position relative to fix invalid position of the…

999f89c

… close icon

feat(modal): enhance ModalBody styling

b61e5af

fix(modal): delete modal disappear animation

2ccdacc

to prevent to show unintended changing of view(state)

feat(modal): add width 100% style to title of ModalHeader

c20fb77

sungik-choi force-pushed the fix/modal-scrollable branch from 1384691 to c20fb77 Compare December 22, 2022 10:21

chore(changeset): add changeset

595cdaf

sungik-choi changed the title ~~[WIP] Fix the Modal overlay to be scrollable~~ Enhance Modal component Dec 22, 2022

sungik-choi changed the title ~~Enhance Modal component~~ Enhance the Modal component Dec 22, 2022

sungik-choi marked this pull request as ready for review December 22, 2022 10:35

sungik-choi requested review from CHEWCHEWW, Dogdriip, carlos-channel-io and quino0627 and removed request for quino0627 December 22, 2022 10:35

junbong reviewed Dec 22, 2022

View reviewed changes

Dogdriip reviewed Dec 22, 2022

View reviewed changes

CHEWCHEWW approved these changes Dec 23, 2022

View reviewed changes

carlos-channel-io approved these changes Dec 23, 2022

View reviewed changes

refactor(modal): apply useMemo

17cb0dd

sungik-choi requested a review from Dogdriip December 26, 2022 02:44

Dogdriip approved these changes Dec 26, 2022

View reviewed changes

sungik-choi merged commit 84e9eef into channel-io:next-v1 Dec 26, 2022

sungik-choi deleted the fix/modal-scrollable branch December 26, 2022 02:48

github-actions bot mentioned this pull request Dec 26, 2022

ci(changesets): version packages (next-v1) #1080

Merged

sungik-choi added the #reimplementation Issue or PR related to Reimplementation of existing components (#1105) label Jan 17, 2023

github-actions bot mentioned this pull request May 30, 2023

ci(changesets): version packages #1369

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enhance the `Modal` component #1077

Enhance the `Modal` component #1077

Uh oh!

sungik-choi commented Dec 13, 2022 •

edited

Loading

Uh oh!

changeset-bot bot commented Dec 13, 2022 •

edited

Loading

Uh oh!

github-actions bot commented Dec 13, 2022 •

edited

Loading

Uh oh!

codecov bot commented Dec 14, 2022 •

edited

Loading

Uh oh!

junbong Dec 22, 2022

Uh oh!

sungik-choi Dec 26, 2022

Uh oh!

Uh oh!

Dogdriip Dec 22, 2022

Uh oh!

carlos-channel-io Dec 23, 2022

Uh oh!

sungik-choi Dec 26, 2022

Uh oh!

Dogdriip Dec 26, 2022

Uh oh!

Dogdriip Dec 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		<BezierProvider foundation={foundation}>
		<div style={wrapperStyle}>

Enhance the Modal component #1077

Enhance the Modal component #1077

Uh oh!

Conversation

sungik-choi commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Self Checklist

Related Issue

Summary

Details

오버레이가 스크롤 가능하도록 개선

Modal hide 애니메이션 제거

ModalBody 스타일 개선

chore: 스토리북 개선

Breaking change or not (Yes/No)

References

Uh oh!

changeset-bot bot commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Chromatic Report

Uh oh!

codecov bot commented Dec 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

junbong Dec 22, 2022

Choose a reason for hiding this comment

Uh oh!

sungik-choi Dec 26, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Dogdriip Dec 22, 2022

Choose a reason for hiding this comment

Uh oh!

carlos-channel-io Dec 23, 2022

Choose a reason for hiding this comment

Uh oh!

sungik-choi Dec 26, 2022

Choose a reason for hiding this comment

Uh oh!

Dogdriip Dec 26, 2022

Choose a reason for hiding this comment

Uh oh!

Dogdriip Dec 26, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Enhance the `Modal` component #1077

Enhance the `Modal` component #1077

sungik-choi commented Dec 13, 2022 •

edited

Loading

`Modal` hide 애니메이션 제거

`ModalBody` 스타일 개선

changeset-bot bot commented Dec 13, 2022 •

edited

Loading

github-actions bot commented Dec 13, 2022 •

edited

Loading

codecov bot commented Dec 14, 2022 •

edited

Loading