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

[조연아] week15 #538

Merged
merged 7 commits into from
Dec 20, 2023
Merged

[조연아] week15 #538

merged 7 commits into from
Dec 20, 2023

Conversation

yunajoe
Copy link
Collaborator

@yunajoe yunajoe commented Dec 16, 2023

요구사항

기본

  • [링크 공유 페이지] 링크 공유 페이지의 url path를 ‘/shared’에서 ‘/shared/{folderId}’로 변경했나요?
  • [링크 공유 페이지] 폴더의 정보는 ‘/api/folders/{folderId}’, 폴더 소유자의 정보는 ‘/api/users/{userId}’를 활용했나요?
  • [링크 공유 페이지] 링크 공유 페이지에서 폴더의 링크 데이터는 ‘/api/users/{userId}/links?folderId={folderId}’를 사용했나요?
  • [폴더 페이지] 폴더 페이지에서 유저가 access token이 없는 경우 ‘/signin’페이지로 이동하나요?
  • [폴더 페이지] 폴더 페이지에서 전체 링크 데이터를 받아올 때 ‘/api/links’, 특정 폴더의 링크를 받아올 때 ‘/api/links?folderId=1’를 활용했나요?
  • [폴더 페이지] 폴더 페이지의 url path가 ‘/folder’일 경우 폴더 목록에서 “전체” 가 선택되어 있고, ‘/folder/{folderId}’일 경우 폴더 목록에서 {folderId} 에 해당하는 폴더가 선택되어 있고 폴더에 있는 링크들을 볼 수 있나요?
  • [폴더 페이지] 폴더 페이지에서 현재 유저의 폴더 목록 데이터를 받아올 때 ‘/api/folders’를 활용했나요?
  • [상단 네비게이션] 유효한 access token이 있는 경우 ‘/api/users’로 현재 로그인한 유저 정보를 받아 상단 네비게이션 유저 프로필을 보여주나요?

심화

  • [심화] 리퀘스트 헤더에 인증 토큰을 첨부할 때 axios interceptors 또는 이와 유사한 기능을 활용 했나요?

주요 변경사항

  • 카카오 소셜 공유 이미지가 자꾸 경로를 못찾아서, build시 에러가 발생해서 일단 삭제했습니다(vsc에서 build할때는 괜찮은데 vercel에서는 자꾸 에러가 나네요). 프로젝트 이후, 이 부분은 fix하도록 하겠습니다

배포도메인

https://firstmyvercel.vercel.app/

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

Copy link
Collaborator

@kimjngyun kimjngyun left a comment

Choose a reason for hiding this comment

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

연아님 고생하셨습니다! 이번에 axios interceptor 도입하면서 기존 fetch 대신 axios로 통일하고, axios<Type>.get() 을 이용하여 response 타입을 미리 정해두면 개발이 조금 더 편해질 것 같습니다. (~~Data 타입도 안만들어줘도 되구요)
고생하셨습니다!

@@ -10,14 +10,14 @@ import Profile from "@/components/profile/Profile";
export default function ShareNav({
data,
}: {
data: UserProfile[] | ShareUser;
data: UserProfile[] | ShareUser | AuthShareUser | undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 컴포넌트에서 사용하는 정보는 data의 email값 뿐으로 보이는데 data의 타입이 조금 과한 것 같습니다. email 값을 직접 받는식으로 수정해보는 것은 어떨까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

그렇게 한다면 FolderNav/ShareNav 를 하나의 컴포넌트를 이용할 수 있을 것 같습니다.

import SearchContext from "../../contexts/SearchContext";
import { Folder } from "@/types/folderMenuListTypes";
export type Data = Folder[] | [] | undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

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

[] 타입이 들어오는 경우가 있나요?

};

export default function DataList({ folderIdKey }: DataListProps) {
const { LinkSDataArr } = useContext(LocaleContext);
export default function DataList({ folderIdKey, data }: DataListProps) {
const { inputValue, handleInputFunc } = useContext(SearchContext);
Copy link
Collaborator

Choose a reason for hiding this comment

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

handleInputFunc은 사용안해서 inputValue 만 꺼내쓰시면 좋을 것 같습니다.

Comment on lines +11 to 15
}: {
folderMenu: FolderMenuListProps[] | undefined;
folderId: string | undefined;
}) {
const router = useRouter();
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit

Suggested change
}: {
folderMenu: FolderMenuListProps[] | undefined;
folderId: string | undefined;
}) {
const router = useRouter();
}: {
folderMenu?: FolderMenuListProps[];
folderId?: string;
}) {

user_id: 25,
links: [],
},
...copyfolderMenuList,
Copy link
Collaborator

Choose a reason for hiding this comment

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

...로 얕은복사가 이루어질 것 같아서 L30의 folderMenuList.slice()는 불필요해 보입니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

[folderId].tsx 파일과 거의 유사한데 따로 만드신 이유가 있을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional Catch-all Segments 을 활용해볼 수 있을것 같아요.

@@ -164,9 +160,9 @@ export default function SingUpPage() {
<Link href="https://www.google.com">
<Image src={google} alt="google" />
</Link>
<Link href="https://www.kakaocorp.com/page">
{/* <Link href="https://www.kakaocorp.com/page">
Copy link
Collaborator

Choose a reason for hiding this comment

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

외부 링크는 a 태그를 사용해주셔야 합니다.

Comment on lines +56 to +63
if (sharedFolderData.data.length === 0) {
return {
props: {
errorCode: 404,
},
};
}
const folderData = sharedFolderData.data[sharedFolderData.data.length - 1];
Copy link
Collaborator

Choose a reason for hiding this comment

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

데이터의 길이가 0인 것을 404 에러로 처리하는 것이 맞을까요? 404에러는 사용자가 사이트에서 존재하지 않는 URL을 탐색했을 때 발생합니다. 이 상황에서는 404 에러로 처리하기보다는 데이터가 없음을 알려주는 것이 좋아보입니다.


// https://bootcamp-api.codeit.kr/api/links?folderId=1

// export const AuthFolderLinks = async (
Copy link
Collaborator

Choose a reason for hiding this comment

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

안쓰는 주석은 지워주세요!

@kimjngyun kimjngyun merged commit 73870f5 into codeit-bootcamp-frontend:part3-조연아 Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants