[Feat/#20] 사용자 / 관리자 알림 페이지 퍼블리싱 #21
Conversation
src/constants/notificationStatus.ts
Outdated
|
|
||
| export const USER_NOTIFICATION_STATUS = { | ||
| USER_RENTAL_APPLY: 'USER_RENTAL_APPLY', | ||
| USER_RENTAL_APPROVED: 'USER_RENTAL_APPROVED ', |
There was a problem hiding this comment.
'USER_RENTAL_APPROVED ' -> 여기 맨뒤에 공백이 들어갔습니당
| export type AdminNotificationTypes = keyof typeof AdminNotificationText; | ||
| export type UserNotificationTypes = keyof typeof UserNotificationText; |
There was a problem hiding this comment.
이미 AdminNotificationStatus 타입이 AdminNotificationText의 키값과 동일해서 아래와 같이 적어도 되지 않을까용? (user도!)
export type AdminNotificationTypes = AdminNotificationStatus;
export type UserNotificationTypes = UserNotificationStatus;
There was a problem hiding this comment.
허거덩 가독성 측면에서는 이렇게 수정하는 게 더 좋을 수도 있겠네요!!
다만 제가 걱정하는 부분은 수민 오빠가 물론 잘 처리해 주었겠지만... 만약 새로운 상태가 추가된다면
export type AdminNotificationTypes = AdminNotificationStatus;
export type UserNotificationTypes = UserNotificationStatus;
방식으로 처리한다면 존재하지 않는 키가 타입으로 포함될 위험이 있다고 생각했어요! 때문에 keyof typeof를 사용해서 실제로 존재하는 키만 정의하고 혹여나 추가되거나 삭제될 때 자동으로 타입을 맞췄으면... 해서 저렇게 정의했습니당!
그리구 AdminNotificationText는 텍스트 매핑에 사용되는 키 값들의 타입이구 AdminNotificationStatus는 알림의 상태값 자체라서 지금대로 유지하는 게 개인적으로 더 명시적이라구 생각해요!
쪼금 뜨거운 감자...... 가 될 수 잇는 여지가 잇긴 하네용... 생각해보니까 굳이? 인 거 같기도 하고... 넘 어렵따 좋은 의견 있으면 남겨주세요!!
There was a problem hiding this comment.
아하 그렇군용😮
저는 상태가 정해져있고, AdminNotificationStatus 타입을 기반으로 AdminNotificationText가 만들어 졌기 때문에 keyof typeof 연산을 추가로 하지 않아도 되지 않을까하는 생각이었답니다 히히
근데 새로운 상태가 추가 될 수 있고 그에 따른 위험이 생길 수 있다는 점을 생각하면 기존의 코드대로 진행하는 것이 좋을 거 같습니다!!
신지 완전 야무진 머쨍이 개발자😎👍
|
너무너무 잘하는 거 같아요 최고십니당😍 |
📌 관련 이슈번호
🎟️ PR 유형
어떤 변경 사항이 있나요?
Check List
✅ Key Changes
사용자 / 관리자 알림 페이지 퍼블리싱
전반적인 디자인이 같고 내용만 달라서
NotificationItem공통 컴포넌트로 빼서 작업했습니다!enum 관리할 type 추가
공통 컴포넌트로 작업하다보니 enum 관리할 type 정리가 필요해서 constants 폴더 내에서 작업했습니다. 또 공통으로 사용하는데 Type을 파일 별로 작성하는 게 비효율적인 것 같아서 notificationType 파일을 추가했습니당~ API 명세서 보고 작업했는데 혹시 같은 타입 사용하는 개발 파트 있으면 사용해도 돼요!
📢 To Reviewers
그리고 [ Feat ] Header 공통 컴포넌트 구현 #17 에서는 svgr이 잘 먹히는데 이 브랜치에서는 요상하게 안 먹혀서 17번 브랜치 먼저 머지하고 얘 머지한 후에 svg 새로 브랜치 파서 작업하겠습니다! 일단은 주석 처리해뒀어요!-> 해결 후 적용했습니다!헤더 PR이 아직 머지가 안 되어서 헤더 적용 안 했습니다! [Feat/#17] 모바일 Header 공통 컴포넌트 추가 #19 머지되면 작업할게요-> 헤더 작업 완료했습니다!타입 단언 피하려고 쌩쇼를 햇는데... 코드가 마음에 들지는 모루겟네요 next에서 이렇게 작업해도 되나 싶기도 하고... 코리 와방 남겨주세여
📸 스크린샷
사용자 뷰 알림
관리자 뷰 알림
