[Feature/BAR-36] 모달 컴포넌트 개발 #21

miro-ring · 2024-01-03T15:48:53Z

Summary

구현 내용 및 작업한 내역을 요약해서 적어주세요

fdb7950 modalStore 작성
c7a4811 모달 개발
0d2c1e6 글 삭제 모달 개발

To Reviewers

스토리북의 내용이 많이 빈약한데, 이후 PR에서 토스트에 대한 스토리를 수정하면서 이 부분도 함께 내용 추가 진행하겠습니다. (두 개 모두 버튼 클릭시 노출되는 형태로 변경할 예정입니다.)
모든 모달은 src/components/Modal/components 하위에 구현하는 형태로 설계하였습니다. 각 모달 컴포넌트는 자신만의 키값으로 구분됩니다.
해당 형태의 구성은 모달이 뒷페이지에 종속성이 있는 경우 문제가 있습니다. 현재 피그마의 모달들을 확인해보았을 때 종속성이 있는 경우는 '폴더 이름 수정' 하나 입니다. 이 경우에만 해당 값을 전역 store로 가져오면 어떨까 생각하고 있습니다.
ModalContainer는 기본적인 모달 형태에 맞게 개발하였습니다. 로그인 모달의 경우 다른 모달과는 그 형태가 다르기 때문에 ModalContainer를 사용하지 않고 개발할 계획입니다.
button에 대한 공통 컴포넌트 개발은 이후 진행할 예정입니다. 모달의 버튼도 이때 함께 수정할 계획입니다.
우측 상단의 닫기 버튼은 아이콘 작업이 완료되면 추가할 예정입니다.
모달에 대한 액션 가이드가 없어서 이 부분은 추가되지 않은 상태입니다.

How To Test

모달에 대한 정상 노출 확인이 필요합니다.

github-actions · 2024-01-03T15:50:03Z

🚀 storybook: https://65808d663dd3d83a46e24b57-rfumfxadtn.chromatic.com/

github-actions · 2024-01-03T16:02:38Z

🚀 storybook: https://65808d663dd3d83a46e24b57-skwngmxukj.chromatic.com/

dmswl98

수고하셨습니당~ 👍🏻
Button 컴포넌트는 공통화가 빨리 필요해보이는데 제가 빠르게 만들까요??

📌 추가로 portal 관련 내용이 보이지 않는데, 해당 Modal 컴포넌트는 현재 portal로 구현된 게 맞을까요?

dmswl98 · 2024-01-04T17:24:34Z

src/components/Modal/ModalContainer.tsx

+interface ModalContainerProps {
+  title: string;
+  firstButtonProps?: ModalButtonProps;
+  secondButtonProps?: ModalButtonProps;
+}


단순 의견; first, second 보다 left, right이 더 명시적으로 보일 것 같습니당

@dmswl98
버튼이 1개만 존재하는 경우는 아직 없지만, 그런 형태가 들어온다고 생각해봤을 때는 first, second라는 명명이 좋을 것 같다고 생각했는데 혹시 이 의견에 대해서는 어떻게 생각하시나요!?

first, second는 2개 이상의 버튼을 내포할 수 있다고 생각됩니다 !

보여주신 형태는 ConfirmModal의 형태로 파악되는데 만약 한 개만 필요한 경우를 고려한다면 차라리 confirm, cancel 등의 네이밍은 어떨까용 ?

first, second는 2개 이상의 버튼을 내포할 수 있다고 생각됩니다 !

보여주신 형태는 ConfirmModal의 형태로 파악되는데 만약 한 개만 필요한 경우를 고려한다면 차라리 confirm, cancel 등의 네이밍은 어떨까용 ?

이 의견도 좋다고 생각합니다!

dmswl98 · 2024-01-04T17:29:23Z

tsconfig.json

@@ -1,10 +1,10 @@
 {
  "compilerOptions": {
+    "strict": true,


👍🏻👍🏻👍🏻

dmswl98 · 2024-01-04T17:36:11Z

src/components/Modal/ModalContainer.tsx

+  secondButtonProps?: ModalButtonProps;
+}
+
+const ModalContainer = ({


현재 디자인 상으로 Modal 타입이 여러 가지라,

해당 컴포넌트 내부에서 모든 content 요소들을 prop으로 받아서 컴포넌트 내부에서 분기 처리하여 그려내기 보다,
compound pattern으로 설계해 content는 children으로 받아서 그려내는 방식으로 구현되면 다양한 Modal 컴포넌트들에 대응이 쉬울 것 같아요.

제가 생각했던 Modal 컴포넌트의 대략적인 사용 코드는 다음과 같아요.

const LoginModal = () => { return ( <Modal> <h1>바로 시작하기</h1> <Button>구글 로그인</Button> <Button>카카오 로그인</Button> </Modal> ) }

dmswl98 · 2024-01-04T17:38:30Z

src/components/Modal/ModalContainer.tsx

+          <button
+            type="button"
+            className={styles.button}
+            style={assignInlineVars({
+              [styles.buttonColor]: firstButtonColor,
+              [styles.buttonBackgroundColor]: firstButtonBackgroundColor,
+            })}
+            onClick={onClickFirstButton}
+          >
+            {firstButtonText}
+          </button>


Button 컴포넌트도 따로 공통 컴포넌트로 만들면 좋을 것 같습니다.

dmswl98 · 2024-01-04T17:41:32Z

src/components/Modal/components/DeleteArticle.tsx

+const DeleteArticle = () => {
+  return (
+    <ModalContainer
+      title="해당 글을 삭제할까요?"
+      secondButtonProps={{
+        text: '삭제하기',
+        onClick: () => {},
+      }}
+    >
+      {'삭제한 글은 복구할 수 없어요!\n삭제하시겠어요'}
+    </ModalContainer>
+  );
+};


Suggested change

const DeleteArticle = () => {

return (

<ModalContainer

title="해당 글을 삭제할까요?"

secondButtonProps={{

text: '삭제하기',

onClick: () => {},

}}

>

{'삭제한 글은 복구할 수 없어요!\n삭제하시겠어요'}

</ModalContainer>

);

};

const DeleteArticle = () => {

return (

<ModalContainer>

<ModalHeader>

<h1>해당 글을 삭제할까요?</h1>

</ModalHeader>

<ModalContent>

<p>{'삭제한 글은 복구할 수 없어요!\n삭제하시겠어요'}</p>

</ModalContent>

<ModalFooter>

<button>취소</button>

<button>삭제하기</button>

</ModalFooter>

</ModalContainer>

);

};

추상화가 된다면 이렇게 활용할 수 있습니다!

wonjin-dev

따로 포탈이 적용되진 않았군요 ?

wonjin-dev · 2024-01-05T05:37:13Z

tsconfig.json

@@ -1,10 +1,10 @@
 {
  "compilerOptions": {
+    "strict": true,


wonjin-dev · 2024-01-05T05:41:37Z

src/components/Modal/ModalContainer.tsx

Modal.tsx가 하나의 파일에서 너무 많은 일을 하고 있는것 같습니다 !

디자인적 요소 (버튼 등)를 포함하지 않은 모달이 존재하면 더 좋을것 같아요

miro-ring · 2024-01-06T01:36:06Z

수고하셨습니당~ 👍🏻 Button 컴포넌트는 공통화가 빨리 필요해보이는데 제가 빠르게 만들까요??

📌 추가로 portal 관련 내용이 보이지 않는데, 해당 Modal 컴포넌트는 현재 portal로 구현된 게 맞을까요?

@dmswl98
기존에는 포탈 기반으로 제작할 생각이었는데, 굳이 그럴 필요가 없다고 느껴져서 제거했습니다!
버튼 컴포넌트는 제가 해당 PR에서 오늘 중으로 빠르게 작업해서 추가해봐도 괜찮을까요?

github-actions · 2024-01-07T15:23:39Z

🚀 storybook: https://65808d663dd3d83a46e24b57-cyxpcjupps.chromatic.com/

miro-ring · 2024-01-07T15:30:23Z

@wonjin-dev @dmswl98
말씀 주신 부분들 반영해보았습니다!

포탈 컴포넌트는 툴팁 PR에 존재하는 컴포넌트 기반으로 작성했습니다.

처음에 포탈을 쓰지 않았던 이유는, div#__next 하위에서 페이지 컴포넌트의 형제요소로 모달이 존재해도 문제가 없을 것으로 판단했기 때문이었습니다. div#__next 의 형제 요소로 반드시 모달을 넣어야 한다면 포탈을 쓸 수 밖에 없겠지만, 굳이 그 위치에 있을 필요는 없다고 생각했습니다.

제안해주신 Compound component 형태 적용도 고려해보았습니다. validation이 필요한 모달들이 존재하기 때문에 해당 패턴을 사용하는 경우 오히려 공통 컴포넌트를 사용하기 어려월 질 것 같다는 생각이 들어 현재 형태로 작성해보았습니다.

모달의 상세한 기능 추가 + 우측의 X 버튼 추가 + 버튼 컴포넌트 적용은 별도 PR에서 진행될 예정입니다. 이때 recipe를 활용한 CSS 공통화 작업도 수행할 계획에 있습니다.

wonjin-dev

정말 고생하셨습니다 ㅎㅎ 개인적으로는 해당 PR은 초안으로 요정도면 머지해도 좋다고 생각합니다 !!

추가적으로 금일 저녁에 모달 전역 상태 관리에 대해서 얘기 나눠보면 좋을것 같습니다 !

dmswl98

헉 제가 approve를 남기지 않았군요... 수고하셨습니당

miro-ring added 7 commits January 4, 2024 00:34

refactor: 토스트 스타일 token을 사용하는 형태로 변경

c52dd7c

feat: strict true 설정 및 이로 인해 발생하는 에러 수정

5ebb849

feat: modalStore 수정

fdb7950

feat: 모달 style 개발

cb5d559

feat: 모달 개발 및 적용

c7a4811

feat: 글 삭제 모달 작성

0d2c1e6

feat: 모달 스토리북 작성

db7b4a9

miro-ring self-assigned this Jan 3, 2024

miro-ring requested review from wonjin-dev and dmswl98 as code owners January 3, 2024 15:48

feat: store 형태 변경

9c410d1

dmswl98 reviewed Jan 4, 2024

View reviewed changes

wonjin-dev reviewed Jan 5, 2024

View reviewed changes

dmswl98 changed the title ~~[Feature/Bar-36] 모달 컴포넌트 개발~~ [Feature/BAR-36] 모달 컴포넌트 개발 Jan 6, 2024

miro-ring added 6 commits January 8, 2024 00:15

feat: ts 파일들에 대해서 no-undef 룰 제거

7ecafe4

feat: 컨벤션 변경에 따른 파일명 변경

cad1752

refactor: store 관련 파일들 리팩터링

124363a

feat: 포탈 컴포넌트 작성

eb6b42b

feat: 확장성 있는 형태로 ModalContainer 변경

ac50fb7

feat: 아티클 제거 컴포넌트 작성

fefeccc

wonjin-dev approved these changes Jan 7, 2024

View reviewed changes

dmswl98 added the feature label Jan 8, 2024

dmswl98 approved these changes Jan 9, 2024

View reviewed changes

miro-ring merged commit 958f9e6 into main Jan 9, 2024
2 of 3 checks passed

miro-ring deleted the feature/BAR-36 branch January 9, 2024 13:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature/BAR-36] 모달 컴포넌트 개발 #21

[Feature/BAR-36] 모달 컴포넌트 개발 #21

miro-ring commented Jan 3, 2024 •

edited by dmswl98

Loading

github-actions bot commented Jan 3, 2024

github-actions bot commented Jan 3, 2024

dmswl98 left a comment •

edited

Loading

dmswl98 Jan 4, 2024

miro-ring Jan 7, 2024

wonjin-dev Jan 7, 2024

dmswl98 Jan 8, 2024

dmswl98 Jan 4, 2024

wonjin-dev Jan 5, 2024

dmswl98 Jan 4, 2024

dmswl98 Jan 4, 2024

dmswl98 Jan 4, 2024

wonjin-dev left a comment

wonjin-dev Jan 5, 2024

wonjin-dev Jan 5, 2024

wonjin-dev Jan 5, 2024

miro-ring commented Jan 6, 2024

github-actions bot commented Jan 7, 2024

miro-ring commented Jan 7, 2024

wonjin-dev left a comment

dmswl98 left a comment

[Feature/BAR-36] 모달 컴포넌트 개발 #21

[Feature/BAR-36] 모달 컴포넌트 개발 #21

Conversation

miro-ring commented Jan 3, 2024 • edited by dmswl98 Loading

Summary

To Reviewers

How To Test

github-actions bot commented Jan 3, 2024

github-actions bot commented Jan 3, 2024

dmswl98 left a comment • edited Loading

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

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

wonjin-dev left a 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

miro-ring commented Jan 6, 2024

github-actions bot commented Jan 7, 2024

miro-ring commented Jan 7, 2024

wonjin-dev left a comment

Choose a reason for hiding this comment

dmswl98 left a comment

Choose a reason for hiding this comment

miro-ring commented Jan 3, 2024 •

edited by dmswl98

Loading

dmswl98 left a comment •

edited

Loading