사이드바에 토스트가 가려지던 문제 해결 (feat.createPortal) #841

Gilpop8663 · 2023-11-09T03:37:09Z

🔥 연관 이슈

close: #738

📝 작업 요약

사이드바에 토스트가 가려지던 문제 해결 (왼쪽/ 오른쪽 모두 대응 가능)
tanstack-query mutate 부분에서 없어도 무방한 코드 삭제 및 신고 상세 내역에서 retry 부분 주석 복구

⏰ 소요 시간

1시간 30분

🔎 작업 상세 설명

useDialog과 Dialog에서 토스트를 생성할 토스트 아이디를 받도록 하였고, createPortal로 토스트 생성하는 위치를 지정함
ToastProvider와 Drawer에서 토스트 엘리먼트 아이디 지정
토스트가 모바일에서 중앙으로 오지 않거나 너비가 짧은 경우가 있어서 스타일 변경, pc 버전에서는 기존과 동일

예시 영상

영상 촬영을 위해 토스트 시간을 10초로 변경하여 찍었습니다. 실제로는 변경 X

2023-11-09.12.34.18.mov

참고 자료

shadcn-ui/ui#75 (힌트를 얻은 사이트)
https://react-ko.dev/reference/react-dom/createPortal (리엑트 공식문서)
https://tanstack.com/query/v4/docs/react/guides/mutations ( 공식문서 mutate에서 async/await을 안붙이더라구요)

Drawer가 켜진다면 dialog 태그 내에서 생성되고, 닫히거나 열리지 않았다면 toast-content라는 별개의 div에서 생성되도록 구현

github-actions · 2023-11-09T03:42:26Z

⚡️ Lighthouse report!

Category	Score
🟠 Performance	75
🟠 Accessibilty	89
🟢 SEO	100
🟠 PWA	89

Category	Score
🟢 First Contentful Paint	0.6 s
🟠 Largest Contentful Paint	2.9 s
🔴 Total Blocking Time	840 ms
🟢 Cumulative Layout Shift	0.046
🟢 Speed Index	2.9 s

chsua · 2023-11-10T17:41:25Z

frontend/src/hooks/query/useReportApproveResult.tsx

+        const status = JSON.parse(fetchError.message).status;
+        if (status === 404) {
+          return false;
+        }


👍👍👍

chsua

우스! 해당 부분 저도 어떻게 해야할지 고민이 있었는데, 역시 실행력 갑입니다!

처음에 시도했던 방식이 createRoot로 기존 루트를 2개 만드는 거였는데, css로 처리가 가능하기에 굳이라고 생각되어 하지 않았거든요!
근데 이걸 creatPotal로 만들면서 다이아로그 위에 띄울 다른 방안을 고안하셨군요!!
해당 방법을 하면 개발자 도구에서 어떻게 보이는지 궁금해지네요
(top layer가 2개가 되는걸까요?🤔 아마 그냥 새로운 root가 만들어질 것 같기는 해요.)
아마 이걸로도 탑레이어 위에 쌓이지 않아서 dialog안에 root를 지정해주는 것을 따로 처리해주신 것 같기는 합니다! 혹시 맞을까요?

이번 계기를 통해서 레이어를 어떻게 쌓아 겹겹이 둬야 하는 지에 대해 고민해볼 수 있었습니다 bb

다만, 고민해보고 싶은 부분이 두 가지있습니다.

drawer에 토스트가 굉장히 깊게 관련되게 되는데 괜찮을까요? 해당 부분이 토스트 해결을 위한 어쩔 수 없는 부분이더라도 완전한 대책은 아니라고 생각이 듭니다. 토스트와 drawer는 독립적인 common으로 작동되는 것이 좋다고 생각이 들어요!
현재 뒷 배경의 인터렉션이 가능해서 여러개의 토스트가 발생할 수 있습니다! 근데 이 부분은 기존에 하나의 토스트 리스트로 관리가 되기 때문에 가능한 거였습니다. 이 로직이 3개의 root가 존재하게 되고, 이 root에 각 토스트가 삽입되는 것으로 바뀐 걸로 이해했습니다. 이로 인해 빠르게 한다면 사이드 바에서 즐겨찾기 오류를 발생시키고, 메인페이지에서 다른 알림 등의 오류를 발생시키면 중첩된 토스트가 발생할 것 같은데 혹시 해당 부분은 어떻게 처리되고 있는지 궁금합니다!
제 예상이 틀렸다면 혹시 어떤 이유로 이런 문제가 발생하지 않는지 알려주실 수 있으신가요?? 제가 코드를 잘못 이해했을 수도 있을 것 같아요!

고생하셨습니다 우스!
fe-리뷰완

inyeong-kang

createPortal 을 적용해주셨군요!
예전에 모달 처음 만들 때 createPortal 어떻게 쓸지 고민이 있었는데 Toast 에도 새로운 root를 열어서 사용해볼 수 있군요👍

다만 우스가 Drawer를 라이브러리로 빼낸다고 가정했을때, Toast 관련된 로직은 어떻게 처리하실지 궁금해지더라구요🤔

따라서 저도 수아처럼 우려해본 부분은, 지금 범용적인 컴포넌트인 Toast와 Drawer끼리 로직이 연관되어 있다는 점입니다
이건 좀더 고민해보고 리팩터링해봐야 감이 올거 같아요🤔🤔

inyeong-kang · 2023-11-12T04:42:18Z

frontend/src/hooks/context/toast.tsx

+    setToastList(toastList => [...toastList, { id, text: message }]);
+  };
+
+  const setElementId = useCallback((id: ToastContentId) => {


리렌더링 방지👍👍

Gilpop8663 · 2023-11-14T01:59:41Z

해당 방법을 하면 개발자 도구에서 어떻게 보이는지 궁금해지네요
(top layer가 2개가 되는걸까요?🤔 아마 그냥 새로운 root가 만들어질 것 같기는 해요.)
아마 이걸로도 탑레이어 위에 쌓이지 않아서 dialog안에 root를 지정해주는 것을 따로 처리해주신 것 같기는 합니다! 혹시 맞을까요?

네 맞아요 따로 html div를 만들어서 따로 관리해보려고 했지만 그래도 dialog 위로 보이지 않더라구요. 그래서 dialog 안에 특정 id를 가진 div 태그를 만들어놓은 후 토스트가 생길 때 dialog 내부 div 태그로 생성되도록 했어요. 새로운 root를 만든다기 보다 drawer가 열릴 때 id를 가진 div가 존재하고, 토스트가 div 내부로 생성되는 코드입니다. 그래서 toplayer는 dialog가 열렸을 때 1개만 존재해요

drawer에 토스트가 굉장히 깊게 관련되게 되는데 괜찮을까요? 해당 부분이 토스트 해결을 위한 어쩔 수 없는 부분이더라도 완전한 대책은 아니라고 생각이 듭니다. 토스트와 drawer는 독립적인 common으로 작동되는 것이 좋다고 생각이 들어요!

제로, 수아의 제안에 동의하여서 drawer 내부에 깊게 관여된 토스트 코드를 밖으로 걷어냈아요. drawer 함수를 사옹하는 측에서 serToastElementId를 통해 토스트가 생성될 아이디를 변경하도록 했습니다. 그리고 토스트가 생성될 div는 children으로 받도록 해서 무조건 사용되는 것이 아니라 사용하는 측에서 선택적으로 사용하도록 변경했습니다.

  const handleCategoryDrawerOpen = () => {
    openCategoryDrawer();
    setElementId('drawer-category-toast-content'); // 사용하는 측에서 토스트 아이디 변경
  };

  const handleCategoryDrawerClose = () => {
    closeCategoryDrawer();
    setElementId('toast-content'); // 사용하는 측에서 토스트 아이디 변경
  };

  <Drawer
            handleDrawerClose={handleCategoryDrawerClose}
            placement="left"
            width="225px"
            ref={categoryDrawerRdf}
          >
            <DrawerToastWrapper placement="left" id="drawer-category-toast-content" /> // 칠드런으로 받도록 함
            <Dashboard />
          </Drawer>

현재 뒷 배경의 인터렉션이 가능해서 여러개의 토스트가 발생할 수 있습니다! 근데 이 부분은 기존에 하나의 토스트 리스트로 관리가 되기 때문에 가능한 거였습니다. 이 로직이 3개의 root가 존재하게 되고, 이 root에 각 토스트가 삽입되는 것으로 바뀐 걸로 이해했습니다. 이로 인해 빠르게 한다면 사이드 바에서 즐겨찾기 오류를 발생시키고, 메인페이지에서 다른 알림 등의 오류를 발생시키면 중첩된 토스트가 발생할 것 같은데 혹시 해당 부분은 어떻게 처리되고 있는지 궁금합니다!

createPortal이 ToastContext에서 이루어지고 있기 때문에 id가 변경되면 toastList가 변경된 id로 보이게 재 렌더링이 되아요. 그래서 사이드바에 2 ~ 3개의 토스트가 있을 때 사이드바를 닫으면 메인 페이지로 그대로 보이게 됩니다. 중첩되지 않아요. 반대로 메인 페이지에서 2~3개의 토스트가 있는 상태로 사이드바를 열면 사이드바 내부로 토스트가 리렌더링 됩니다.

2023-11-14.10.57.22.mov

궁금한 점 있으시면 더 이야기 나눠보아요 😀

fe-리뷰반영

Gilpop8663 · 2023-11-14T02:00:41Z

drawer랑 toast 분리했습니당

fe-리뷰요청

Gilpop8663 · 2023-11-14T02:01:42Z

drawer랑 toast 분리했습니당

fe-리뷰요청

chsua

코드 잘 읽었습니다.!
똑똑한 우스..
이렇게 설계할 생각 못했는데 대단합니다!
토스트와 모달이 분리가 되면서도 본래 의도는 유지할 수 있어서 좋네요!

다만 코멘트에서 말한 것과 같이 직접 돔을 조작하는 것에 대해서는 한 번 생각해보고 싶습니다!
일단 토스트를 상단에 띄우는 것이 UX적으로 우선이니 머지해도 좋지만
가능하면 직접 돔 조작은 하고 싶지 않은 마음도 있습니다..
추후 수정해도 좋을 것 같아요!
고생하셨어요!

chsua · 2023-11-14T04:52:12Z

frontend/src/hooks/context/toast.tsx

 });

 export default function ToastProvider({ children }: PropsWithChildren) {
  const [toastList, setToastList] = useState<ToastInfo[]>([]);
+  const [toastElementId, setToastElementId] = useState<ToastContentId>('toast-content');
+  const toastContentEl = document.getElementById(toastElementId);


다시 읽으며 든 생각인데, document.getElementById로 dom을 직접 제어하는 것은 지양된다고 알고 있습니다.
real dom 과 가상돔이 달라지면서 돔 요소를 신뢰할 수 없어진다고 알고 있습니다.
위험부담을 감안하여 진행해야 할까요? 고민이 듭니다!

이 부분은 잘 모르겠네요.

다만 공식문서에서 있는 방법을 참고하여서 진행했기 때문에 괜찮지 않을까라는 생각이 듭니다

chsua · 2023-11-14T04:59:37Z

frontend/src/pages/HomePage/index.tsx

+  const handleCategoryDrawerClose = () => {
+    closeCategoryDrawer();
+    setElementId('toast-content');
+  };


여기까지 오기 위한 빌드업이었군요!
읽으면서 헷갈리는 부분도 있었는데, 이 부분을 읽으니 로직이 확 이해가 갔습니다 👍👍
설계하는 능력이 뛰어나시네요

chsua · 2023-11-14T05:03:12Z

fe-리뷰완

Gilpop8663 added 5 commits November 9, 2023 11:04

feat: (#738) createPortal을 이용하여 Drawer가 켜졌을 때와 닫힐 때 토스트가 생성될 위치를 변경

973d0c7

Drawer가 켜진다면 dialog 태그 내에서 생성되고, 닫히거나 열리지 않았다면 toast-content라는 별개의 div에서 생성되도록 구현

style: 모바일에서 토스트가 중앙으로 오도록 스타일 변경

dbde78c

feat: (#738) 토스트를 보여주는 엘리먼트 아이디가 중복되어서 각각 설정하도록 useDialog, Dialog 코드 변경

c67edd2

refactor: (#738) 없어도 무방한 async/await 코드 삭제 및 주석되어 있던 코드 복구

965f0ba

chore: (#738) 모킹 데이터로 실험하던 코드 복구

0bf256b

Gilpop8663 added FE 🐾 개발 🐛 버그 labels Nov 9, 2023

Gilpop8663 requested review from inyeong-kang and chsua November 9, 2023 03:37

Gilpop8663 self-assigned this Nov 9, 2023

chsua reviewed Nov 10, 2023

View reviewed changes

inyeong-kang approved these changes Nov 12, 2023

View reviewed changes

refactor: (#738) Drawer와 Toast의 연관 관계가 없도록 변경

d1a526b

chsua approved these changes Nov 14, 2023

View reviewed changes

Gilpop8663 merged commit b2ceead into dev Nov 20, 2023
1 check passed

woo-chang deleted the feat/#738 branch December 6, 2023 07:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

사이드바에 토스트가 가려지던 문제 해결 (feat.createPortal) #841

사이드바에 토스트가 가려지던 문제 해결 (feat.createPortal) #841

Gilpop8663 commented Nov 9, 2023

github-actions bot commented Nov 9, 2023 •

edited

Loading

chsua Nov 10, 2023

chsua left a comment •

edited

Loading

inyeong-kang left a comment

inyeong-kang Nov 12, 2023

Gilpop8663 commented Nov 14, 2023 •

edited

Loading

Gilpop8663 commented Nov 14, 2023 •

edited

Loading

Gilpop8663 commented Nov 14, 2023

chsua left a comment

chsua Nov 14, 2023

Gilpop8663 Nov 20, 2023

chsua Nov 14, 2023

chsua commented Nov 14, 2023

사이드바에 토스트가 가려지던 문제 해결 (feat.createPortal) #841

사이드바에 토스트가 가려지던 문제 해결 (feat.createPortal) #841

Conversation

Gilpop8663 commented Nov 9, 2023

🔥 연관 이슈

📝 작업 요약

⏰ 소요 시간

🔎 작업 상세 설명

예시 영상

참고 자료

github-actions bot commented Nov 9, 2023 • edited Loading

chsua Nov 10, 2023

Choose a reason for hiding this comment

chsua left a comment • edited Loading

Choose a reason for hiding this comment

inyeong-kang left a comment

Choose a reason for hiding this comment

inyeong-kang Nov 12, 2023

Choose a reason for hiding this comment

Gilpop8663 commented Nov 14, 2023 • edited Loading

Gilpop8663 commented Nov 14, 2023 • edited Loading

Gilpop8663 commented Nov 14, 2023

chsua left a comment

Choose a reason for hiding this comment

chsua Nov 14, 2023

Choose a reason for hiding this comment

Gilpop8663 Nov 20, 2023

Choose a reason for hiding this comment

chsua Nov 14, 2023

Choose a reason for hiding this comment

chsua commented Nov 14, 2023

github-actions bot commented Nov 9, 2023 •

edited

Loading

chsua left a comment •

edited

Loading

Gilpop8663 commented Nov 14, 2023 •

edited

Loading

Gilpop8663 commented Nov 14, 2023 •

edited

Loading