Notice 페이지 API 연동 및 컴포넌트 교체 #284
Conversation
auto-assign
bot
requested review from
Eugene-A-01,
ParkSohyunee,
kanglocal and
seoyoung-min
|
@Nahyun-Kang is attempting to deploy a commit to the Eujin Ahn's projects Team on Vercel. A member of the Team first needs to authorize it. |
ParkSohyunee
left a comment
ParkSohyunee
left a comment
There was a problem hiding this comment.
나현님 공지페이지 구현해주셔서 감사합니다. 💕LGTM💕 고생많으셨습니다!!
+. PR에 올려주신것처럼 다크모드에서는 검은화면이지만 라이트모드에서는 콘텐츠블록이 흰색으로 보여집니다. 어드민 페이지에서는 배경이 흰색이어서 미처 발견하지 못하였는데요, 해당 PR에 스타일 수정 반영 부탁드립니당 🥹💛💛 (NoticeDetailContents 컴포넌트 마크다운, 유의사항)
|
|
||
| const getNotices = async () => { | ||
| const result = await axiosInstance.get<NoticeListItemType[]>('/admin/notices'); | ||
|
|
||
| return result.data; | ||
| }; | ||
|
|
||
| export default getNotices; |
There was a problem hiding this comment.
나현님, 해당 API 엔드포인트는 어드민 공지 조회로 보이는데 API가 잘못 연결된게 맞을까요?
어드민, 사용자에 따라 필요한 필드가 다르기 때문에 사용자용 공지 조회는 GET "/notices"를 사용하는 것으로 알고있습니다~! 참고로 어드민은 공개, 비공개에 상관없이 모든 게시물이 노출됩니다!
src/app/notice/mockdata.ts
Outdated
There was a problem hiding this comment.
📌 사용하지 않는 목업데이터는 삭제하면 좋을 것 같아요~!
src/app/notice/NoticeList.tsx
Outdated
| {notices?.map((item: NoticeListItemType, index) => ( | ||
| <li key={index}> | ||
| <NoticeListItem item={item} /> |
There was a problem hiding this comment.
key를 index로 하신 이유가 있을까요?! 👀 사용자만큼은 아니지만 게시물도 생성 및 수정이 빈번하게 될 수 있기 때문에 id를 key값으로 지정하는 것이 더 안전해보입니다!
| const { data: notices } = useQuery<NoticeListItemType[]>({ | ||
| queryKey: [QUERY_KEYS.getAllNotices], | ||
| queryFn: getNotices, | ||
| staleTime: 1000 * 60 * 30, | ||
| }); |
There was a problem hiding this comment.
한가지 궁금한점이 NoticeListItemType 타입은 itemImageUrl 필드를 가지고 있는데, 해당 필드는 게시물을 생성할 때 콘텐츠블록에 이미지 블록을 의미하는 것이 맞을까요? 아니면 대표 이미지가 있는 것인지 해당 이미지가 UI상 노출되는 부분을 확인할 수 없어서 여쭤봅니다~!
| function NoticeDetailComponent() { | ||
| const pathname = usePathname(); | ||
| const noticeId = pathname?.split('/').pop(); | ||
| const noticeIdNumber = noticeId ? Number(noticeId) : null; | ||
| const router = useRouter(); | ||
|
|
||
| const { | ||
| data: notice, | ||
| isLoading, | ||
| isError, | ||
| } = useQuery<NoticeDetailType>({ | ||
| queryKey: [QUERY_KEYS.getNoticeDetail, noticeIdNumber], | ||
| queryFn: () => getNoticeDetail(noticeIdNumber as number), | ||
| enabled: noticeIdNumber !== null, // noticeIdNumber가 유효한 경우에만 실행 | ||
| }); |
There was a problem hiding this comment.
나현님, Next.js의 Dynamic Routes의 경우에는 라우트(경로) 파라미터(slug)를 params라는 props로 받아올 수 있습니다~!
이 방법을 사용하면 path를 얻기위해 use client를 사용하지 않아도 되고, 따로 가공하지 않아도 되므로 코드를 간결하게 유지할 수 있다는 장점이 있습니다.
function NoticeDetailComponent({ params }: { params: { noticeId: number } }) {
const noticeId = params.noticeId;
// useQuery
enabled: !!noticeId,
...
| export const contents = style({ | ||
| paddingTop: '2rem', | ||
| padding: '2rem 1.6rem 0 1.6rem', | ||
|
|
There was a problem hiding this comment.
(의견) 나현님, 이 부분은 contents 자체에 패딩을 주기 보다는 NoticeDetailInfo 컴포넌트를 사용하는 곳에 div태그로 감싸고 여기에 패딩을 주면 어떨까요?!
There was a problem hiding this comment.
📌 public 안에있는 변경사항 파일은 PR에 포함되는 파일일까요?!
3 participants
개요
작업 사항
참고 사항 (optional)
관련 이슈 (optional)
스크린샷
리뷰어에게