Allow `Modal` to keep open by adding `preventHideOnOutsideClick` property #1617

yangwooseong · 2023-09-11T10:04:58Z

Self Checklist

I wrote a PR title in English and added an appropriate label to the PR.
I wrote the commit message in English and to follow the Conventional Commits specification.
I added the changeset about the changes that needed to be released. (or didn't have to)
I wrote or updated documentation related to the changes. (or didn't have to)
I wrote or updated tests related to the changes. (or didn't have to)
I tested the changes in various browsers. (or didn't have to)
- Windows: Chrome, Edge, (Optional) Firefox
- macOS: Chrome, Edge, Safari, (Optional) Firefox

Related Issue

Resolves Support title, subtitle, closeIcon in ModalHeader #1457

Summary

preventHideOnOutsideClick 속성을 Modal 컴포넌트 인터페이스에 추가해서, 모달 밖을 클릭해도 모달이 닫히지 않게 할 수 있도록 합니다.

Details

radix-ui 에서 제공하는 Dialog.Content 의 onPointerDownOutside 콜백에서 이벤트 전파를 막는 것으로 구현하였습니다.

Breaking change? (Yes/No)

No

References

changeset-bot · 2023-09-11T10:05:01Z

🦋 Changeset detected

Latest commit: fb05843

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

codecov · 2023-09-11T10:10:12Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% 🎉

Comparison is base (842c80b) 87.00% compared to head (fb05843) 87.02%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1617      +/-   ##
==========================================
+ Coverage   87.00%   87.02%   +0.01%     
==========================================
  Files         281      281              
  Lines        3879     3884       +5     
  Branches      817      820       +3     
==========================================
+ Hits         3375     3380       +5     
  Misses        430      430              
  Partials       74       74

Files Changed	Coverage Δ
...r-react/src/components/Modals/Modal/Modal.types.ts	`100.00% <ø> (ø)`
...react/src/components/Modals/Modal/ModalContent.tsx	`93.75% <100.00%> (+1.15%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2023-09-11T10:10:25Z

Chromatic Report

🚀 Congratulations! Your build was successful!

sungik-choi · 2023-09-12T02:34:53Z

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

+          onPointerDownOutside={(e) => {
+            if (preventHideOnOutsideClick) {
+              e.preventDefault()
+            }
+          }}
+        >


https://github.com/radix-ui/primitives/blob/28bebf2c6992d056244845c898abeff45dec2871/packages/react/alert-dialog/src/AlertDialog.tsx#L135-L136

radix-ui AlertDialog를 참고: onInteractOutside 이벤트도 preventDefault 처리되면 좋을 거 같습니다.

preventInteractOutside ..? 기존 이름이 동작을 더 구체적으로 나타내서 좋긴 한 거 같은데 말이죠

preventInteractOutside 라 하면 이게 false 일 때 모달 밖에 있는 버튼과 interact 가능하다는 의미로 받아들여지지 않을까요?

그럴 수 있을 거 같고, interact outside 라면 hide on outside click -> interact outside 로 생각을 2단계 거쳐야하는 느낌이라 처음에 작성해주신 속성명이 더 좋은 거 같아요

sungik-choi · 2023-09-12T02:43:01Z

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

@@ -40,6 +41,7 @@ export const ModalContent = forwardRef(function ModalContent({
  ...rest
 }: ModalContentProps, forwardedRef: React.Ref<HTMLDivElement>) {
  const [contentContainer, setContentContainer] = useState<HTMLElement>()
+  const preventHideOnOutsideClick = useModalContext()


ModalContent 의 속성으로 전달해줘도 되지 않을까요? Modal 의 컨텍스트로 전달해주신 이유가 궁금합니다.

모달이 열리고 닫히는 동작을 제어하는 속성이라서 모달의 상태나 핸들러(show, onShow 등등)과 같은 곳에 위치하는게 적절할 것 같았습니다

DialogPrimitive 의 인터페이스를 따르는 게 전체적인 일관성에 있어서 더 좋지 않을까 생각했어요

sungik-choi

인터랙션 테스트 추가 부탁드립니다!

sungik-choi · 2023-09-13T05:36:59Z

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

+      it('should keep modal open when the user clicks outside of the modal if preventHideOnOutsideClick property is true', async () => {
+        const { queryByRole, container } = renderOpenedModal({
+          modalProps: {
+            preventHideOnOutsideClick: true,
+            defaultShow: true,
+          },
+        })
+        await user.click(container)
+        expect(queryByRole('dialog')).toBeInTheDocument()
+      })


sungik-choi · 2023-09-13T06:04:02Z

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

+          onPointerDownOutside={(e) => {
+            if (preventHideOnOutsideClick) {
+              e.preventDefault()
+            }
+          }}
+        >


그럴 수 있을 거 같고, interact outside 라면 hide on outside click -> interact outside 로 생각을 2단계 거쳐야하는 느낌이라 처음에 작성해주신 속성명이 더 좋은 거 같아요

…onent

…dalContent

…nt` component

@yangwooseong

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @channel.io/bezier-react@1.13.0 ### Minor Changes - Add `preventHideOnOutsideClick` property to `Modal` component ([#1617](#1617)) by @yangwooseong ### Patch Changes - Fixes an issue where the height of `TextArea` component is not specified correctly. Modify the build settings to match the package.json exports fields change in 8.5.0 of `react-textarea-autosize`. ([#1637](#1637)) by @sungik-choi - Add `ProgressBarSize`, `ProgressBarVariant` string literal type and deprecate enum ([#1595](#1595)) by @Dogdriip ## bezier-figma-plugin@0.4.1 ### Patch Changes - Updated dependencies - @channel.io/bezier-react@1.13.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

yangwooseong added the enhancement Issues or PR related to making existing features better label Sep 11, 2023

yangwooseong requested a review from sungik-choi as a code owner September 11, 2023 10:04

yangwooseong self-assigned this Sep 11, 2023

sungik-choi reviewed Sep 12, 2023

View reviewed changes

sungik-choi reviewed Sep 13, 2023

View reviewed changes

yangwooseong requested a review from sungik-choi September 13, 2023 06:55

sungik-choi approved these changes Sep 13, 2023

View reviewed changes

yangwooseong added 5 commits September 13, 2023 17:06

feat(mdoal): add preventHideOnOutsideClick property to Modal comp…

ef44338

…onent

chore(changeset): add changeset

5c36f6c

feat(modal): call e.preventDefault() in onInteractOutside props of Mo…

20dd2cc

…dalContent

feat(modal): add interaction test

c231b8a

feat(modal): move preventHideOnOutsideClick property to `ModalConte…

fb05843

…nt` component

yangwooseong force-pushed the feat/modal-hide-on-blur-property branch from 7dc945f to fb05843 Compare September 13, 2023 08:06

yangwooseong merged commit 3c3a9fd into channel-io:main Sep 14, 2023
6 checks passed

yangwooseong deleted the feat/modal-hide-on-blur-property branch September 14, 2023 03:02

This was referenced Sep 14, 2023

ci(changesets): version packages #1622

Merged

ci(changesets): version packages yangwooseong/bezier-react#50

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow `Modal` to keep open by adding `preventHideOnOutsideClick` property #1617

Allow `Modal` to keep open by adding `preventHideOnOutsideClick` property #1617

yangwooseong commented Sep 11, 2023

changeset-bot bot commented Sep 11, 2023 •

edited

Loading

codecov bot commented Sep 11, 2023 •

edited

Loading

github-actions bot commented Sep 11, 2023 •

edited

Loading

sungik-choi Sep 12, 2023

yangwooseong Sep 13, 2023

sungik-choi Sep 13, 2023

sungik-choi Sep 12, 2023

yangwooseong Sep 12, 2023

sungik-choi Sep 13, 2023

sungik-choi left a comment

sungik-choi Sep 13, 2023

sungik-choi Sep 13, 2023

Allow Modal to keep open by adding preventHideOnOutsideClick property #1617

Allow Modal to keep open by adding preventHideOnOutsideClick property #1617

Conversation

yangwooseong commented Sep 11, 2023

Self Checklist

Related Issue

Summary

Details

Breaking change? (Yes/No)

References

changeset-bot bot commented Sep 11, 2023 • edited Loading

🦋 Changeset detected

codecov bot commented Sep 11, 2023 • edited Loading

Codecov Report

github-actions bot commented Sep 11, 2023 • edited Loading

Chromatic Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sungik-choi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Allow `Modal` to keep open by adding `preventHideOnOutsideClick` property #1617

Allow `Modal` to keep open by adding `preventHideOnOutsideClick` property #1617

changeset-bot bot commented Sep 11, 2023 •

edited

Loading

codecov bot commented Sep 11, 2023 •

edited

Loading

github-actions bot commented Sep 11, 2023 •

edited

Loading