Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/관리자 전체 유저 알림 모달 개선 #650 #676

Merged

Conversation

Wilbur0306
Copy link
Contributor

@Wilbur0306 Wilbur0306 commented Feb 22, 2023

📌 개요

-전체 유저 알림 모달과 toast message 연결

💻 작업사항

  • 전체 유저 알림 모달과 toast message 연결
  • toast message를 독립적으로 사용할 수 있게 코드 개선

정다인 and others added 21 commits February 14, 2023 12:59
@Wilbur0306 Wilbur0306 added enhancement New feature or request admin 관리자 페이지에서 발생한 issue labels Feb 22, 2023
@Wilbur0306 Wilbur0306 self-assigned this Feb 22, 2023
@Wilbur0306 Wilbur0306 requested a review from a team as a code owner February 22, 2023 08:20
@Wilbur0306 Wilbur0306 requested review from mike2ox, raehy19 and 42sungwook and removed request for a team February 22, 2023 08:20
Copy link
Contributor

@mike2ox mike2ox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

토스트랑 모달 개선하느라 고생하셨습니다!
몇가지만 수정하시면 될 거 같아요

components/modal/admin/AdminNotiAllModal.tsx Outdated Show resolved Hide resolved
components/modal/admin/AdminNotiAllModal.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@raehy19 raehy19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다 ! 토스트 메세지 리코일로 잘 만들어주신것 같네요 close 눌렀을때 내용이 없어진 다음 토스트 메시지가 없어지는거같은데 리코일을 써서 어쩔수 없이 느린것같기도 하네요! 모송님이 적어주신 리팩토링할만한것 말고는 크게 문제되는건 없어보입니다 ! ㅣLGTM

Copy link
Contributor

@42sungwook 42sungwook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다 잘되네요!

Comment on lines +12 to +13
return <MuiAlert elevation={6} ref={ref} variant='filled' {...props} />;
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

뭐에 대한 설정인지 궁금해요!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

조사해보니 흥미로운 내용이 있어서 공유할게요

forwardRef

  • 다른 컴포넌트의 내부 html 요소에 접근할 때(ref 사용) 사용하는 react built-in API (ref : 일반적인 상황에서 쓰이는 prop은 아님)

MuiAlert

  • MUI에서 제공해주는 Alert 컴포넌트
  • elevation : 컴포넌트 뒤에 나오는 shadow 정도
  • variant : 어떤 디자인의 Alert인지 정해주는 prop

- 불필요한 useState, 함수들 제거
- useRef를 사용해 불필요한 render현상 막음
- 다음 미션 주석 작성
@mike2ox
Copy link
Contributor

mike2ox commented Feb 25, 2023

@raehy19 리뷰 감사합니다 :)

close 눌렀을때 내용이 없어진 다음 토스트 메시지가 없어지는거같은데 리코일을 써서 어쩔수 없이 느린것같기도 하네요!

요거 같은 경우 toast와 modal는 별개로 동작해야하는 컴포넌트들이라 동작이 느리게 보이는 겁니다.
기존에 다인님이 만드신 형태는 modal이 종료시 toast까지 종료하는 방식이었습니다.
하지만 해당 modal이 잘 동작했는지 안했는지를 확인할 수 없는 불편함이 있어서 독립적으로 동작하도록 개선한게 현 상태입니다.
혹시 다른 동작으로 해야할 것같으면 피드백 남겨주세요!

@Wilbur0306 Wilbur0306 merged commit 0af4662 into main Feb 27, 2023
@Wilbur0306 Wilbur0306 deleted the Feat/관리자-전체-유저-알림-모달-개선-#650 branch February 27, 2023 02:50
@mike2ox mike2ox linked an issue Feb 28, 2023 that may be closed by this pull request
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
admin 관리자 페이지에서 발생한 issue enhancement New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Feat] toast message 기능 추가
4 participants