[MZO-48] toast 공용 모듈 및 UI 구현 by haryung-lee · Pull Request #34 · EmoHiphopSeason2/mz2mo-client

haryung-lee · 2023-07-12T18:41:18Z

Jira Issue 번호

#48

작업 내용

toast 컴포넌트 생성
toast wrapper(toaster)로 toast 관리
svg태그에서 width, height 속성을 기존 current -> 100%로 변경

To Reviewers

현재 도메인에 따른 모달(emoji-picker, modal, toast, youtube-player) portal이 여러개 존재합니다. depth가 깊어지는 문제가 생겨서 이와 관련해 고민해봤는데, portal를 하나만 두고 z-index로 각 layer를 분리시는 로직은 어떨까요?
지금 구현한것은 최소한의 기능만 있는 toast입니다. 조금더 범용성을 가져가야할지(위치 등) 고민이 되는데 루키 생각은 어떤지 궁금합니다!
아래와 같은 에러가 생겨서 svg태그의 width, height 속성을 변경하였습니다. 구체적으로 왜 이런 에러 메세지가 나오는지는 원인을 파악하지 못했습니다..

- toast는 아래와 같이 사용하면 됩니다.

  const addToast = useSetAtom(useToastAtom);

  const handleOpenToast = ({
    type,
    title,
    message,
  }: Omit<ToastItemType, 'id'>) => {
    addToast(type)(title, message); // 커링함수
  };

Checklist

Code Review 요청
PR 제목 commit convention에 맞는지 확인
Label 설정
Jira 코드 리뷰 상태로 변경

haryung-lee · 2023-07-12T18:46:57Z

+        opacity: opacity,
+        transition: 'all 0.35s ease-in-out',
+        transform: `translateY(${opacity === 0 ? '-10px' : '0'})`,
+      }}


컴포넌트 변수를 style에 적용해야 해서 따로 css를 빼지 않고 바로 작성하였습니다.

RookieAND

고생하셨습니다 하랭.
작업과 관련하여 몇 가지 의견을 작성했으니 참고 부탁드립니다.
궁금한 점이 있다면 언제든 코멘트로 부탁드려요!

RookieAND · 2023-07-13T05:30:19Z

+
+export const useToastAtom = atom(
+  (get) => get(toasterAtom),
+  (get, set, type: 'success' | 'error' | 'info') =>


여기서도 아까 정의했던 Type (ToastStateType) 을 사용하면 더 좋을 것 같습니다.

엇 이거 ToastType으로 수정해놨었는데 왜 이렇게 보이는거죠..?!

RookieAND · 2023-07-13T05:30:28Z

@@ -0,0 +1,8 @@
+export type Type = 'success' | 'info' | 'error';


단순히 Type이라는 명칭보다는 Toast의 상태를 나타내는 것이니 ToastStateType 으로 표기하는 게 혼선이 덜할 것 같아요.

여기도.. 수정해놨던건데 반영이 안됐네용 ㅜ

RookieAND · 2023-07-13T05:31:50Z

+    <div
+      style={{
+        opacity: opacity,
+        transition: 'all 0.35s ease-in-out',


animation time 의 경우에는 커스텀하지 않아도 괜찮으려나요? 불필요하다면 상수로 관리하게 좋겠네요.

저희가 animation이 어디까지 들어갈지 아직 미정이고 각 animation마다 써야하는게 좀 달라질수도 있지 않을까요?? 상수로 관리해야할지 의문이 들어서요!

하기야 그렇겠네요. 지금은 그냥 넣어도 좋아 보여요

RookieAND · 2023-07-13T05:33:20Z

+  return (
+    <AppPortal.Provider portalName="toast-portal">
+      <div className="fixed space-y-2 -translate-x-1/2 left-1/2 top-4">
+        {toasts.map((toast) => {


return 키워드를 사용하지 않고 곧바로 컴포넌트를 반환하면 더 코드가 간결해질 것 같아요.

{toasts.map((toast) => ( <Toast key={toast.id} {...toast} />; ))}

RookieAND · 2023-07-13T05:34:22Z

+import Toast from '@/components/toast/Toast';
+import { useToastAtom } from '@/stores/toast';
+
+const Toaster = () => {


Toaster 컴포넌트의 역할은 Toast 컴포넌트가 렌더링되는 포탈을 제공하는 Provider의 역할이기 때문에
개인적으로는 ToastProvider 로 네이밍을 적는 게 좋아 보이는데 어떻게 생각하시나요?

다른 라이브러리에서 Toaster라고 해서 참고한건데 ToastProvider도 괜찮아보이네요!

오 그래요? 다른 라이브러리에서는 Toaster라고 하는 군요.. 하랭이 조율하셔도 괜찮아 보입니다.

루키가 제안주신 네이밍이 더 좋은거 같아서 수정했습니닷 ㅎㅎ

RookieAND · 2023-07-13T05:35:49Z

+const Toaster = () => {
+  const toasts = useAtomValue(useToastAtom);
+  return (
+    <AppPortal.Provider portalName="toast-portal">


[NIT]
Provider는 Portal을 생성하기 위한 HTMLDivElement 를 제공하는 역할이긴 한데,
약간 고민이 되는게 해당 DOM에 className을 넘겨줄 수 있게 할지가 좀 고민이네요.

하랭의 경우 Provider 하단에 바로 toast 를 띄우는 Div를 만들었는데, 이럴 필요 없이 Portal의 parent가 되는 DOM에 classNames를 인계할 수 있게 하는 건 조금 그러려나요? 하랭의 의견이 궁금하네요.

아 요부분 제가 Wrapper로 하려던걸 provider로 했네요.. 저도 루키 의견에 동의합니다!

RookieAND · 2023-07-13T05:39:04Z

@@ -0,0 +1,29 @@
+import { atom } from 'jotai';
+import { v4 as uid } from 'uuid';


toast 별로 uuid를 부여해서 고유한 key를 부여하는 방식이군요. 정말 좋은 아이디어라고 생각합니다.
다만 저는 아래와 같은 방식도 나쁘지 않다고 생각했습니다.

toasterAtom 에서 현재 "몇 개의" toast를 생성했는지를 체크하는 속성인 sequence 를 생성한다.

toast가 한 개 추가될 때마다 sequence 값을 1 증가시키고, 해당 toast 객체의 uuid 처럼 부여한다.

toast가 삭제되더라도 sequence는 그대로 유지된다. 오직 "새로운 toast가 생성될 때만" 1 증가한다.

이렇게 되면 uuid를 사용하지 않더라도 고유함이 유지될 것이라 생각했습니다.
물론 고유한 값이라는 명목 하에는 uuid가 가장 절대적이긴 하지만 toast 에서까지 써야 하나? 라는 생각이 들어서요.
위와 같은 이유로 저는 Atom 에서 자체적으로 sequence 를 가지는 건 어떨까 싶네요. 하랭의 의견을 들려주세요.

저도 이부분을 고민했었는데.. react-hot-toast에서 루키가 말한 방식으로 하고 있더라구요. 그런데 또 한편으로는 uuid를 안쓸 이유도 없다고 생각해서 사용했습니다.

toast에서 사용하는 id는 일시적인 값이라 루키가 그렇게 생각했을수도 있을것 같아요. 그러면 요건 루키가 말한 방식으로 수정해볼게요!

맞아요 사실 일시적으로 잠깐 쓰이다가 말 값이라 굳이 UUID로 하지 않아도 된다는 게 제 의견이긴 했습니다.

RookieAND

LGTM, 고생 하셨습니다 하랭!

haryung-lee added 6 commits July 12, 2023 18:55

feat: add alert svg

436c007

fix: fix svg width, height

90113e9

chore: add uuid

a3ee713

feat: add toast store

12fe0fe

feat: add toast component

26e0527

fix: add toaster

fd9bbf5

haryung-lee added the ✨ Feature 기능 개발 label Jul 12, 2023

haryung-lee added this to the 4주차 스프린트 milestone Jul 12, 2023

haryung-lee self-assigned this Jul 12, 2023

haryung-lee requested a review from RookieAND July 12, 2023 18:41

haryung-lee changed the base branch from main to develop July 12, 2023 18:42

fix: change const name

c4ed5cc

haryung-lee commented Jul 12, 2023

View reviewed changes

haryung-lee added 3 commits July 13, 2023 03:53

fix: rename type

27cdfa9

chore: delete scss

b9caf86

Merge branch 'develop' into feature/MZO-48

623cfa4

haryung-lee marked this pull request as draft July 12, 2023 19:03

RookieAND reviewed Jul 13, 2023

View reviewed changes

haryung-lee added 4 commits July 13, 2023 16:43

rename: rename toaster

99cf044

chore: delete uuid

d7a78fb

fix: add sequence for id

4aeeed5

design: fix top position

b239f92

haryung-lee marked this pull request as ready for review July 13, 2023 08:37

RookieAND approved these changes Jul 13, 2023

View reviewed changes

haryung-lee merged commit 706ec54 into develop Jul 13, 2023

haryung-lee deleted the feature/MZO-48 branch July 13, 2023 08:48

RookieAND linked an issue Jul 14, 2023 that may be closed by this pull request

[MZO-48] toast 공용 모듈 및 UI 구현 #28

Closed

		@@ -0,0 +1,8 @@
		export type Type = 'success' \| 'info' \| 'error';

		@@ -0,0 +1,29 @@
		import { atom } from 'jotai';
		import { v4 as uid } from 'uuid';

Conversation

haryung-lee commented Jul 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Jira Issue 번호

작업 내용

To Reviewers

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RookieAND left a comment

Choose a reason for hiding this comment

Uh oh!

RookieAND Jul 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RookieAND Jul 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RookieAND left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

haryung-lee commented Jul 12, 2023 •

edited

Loading

RookieAND Jul 13, 2023 •

edited

Loading

RookieAND Jul 13, 2023 •

edited

Loading