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 #475

Conversation

hansaemkim38
Copy link
Collaborator

요구사항

기본

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

심화

  • 리퀘스트 헤더에 인증 토큰을 첨부할 때 axios interceptors 또는 이와 유사한 기능을 활용해 주세요.

주요 변경사항

  • 피드백 반영 했습니다.

스크린샷

스크린샷 2024-05-26 오후 7 37 28

멘토에게

@hansaemkim38 hansaemkim38 requested a review from clianor May 26, 2024 10:40
@hansaemkim38 hansaemkim38 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label May 26, 2024
@clianor
Copy link
Collaborator

clianor commented May 30, 2024

앗 제가 늦게 봤네요 죄송합니다ㅜㅜ

Copy link
Collaborator

Choose a reason for hiding this comment

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

src/components/Header 밑에 다른 컴포넌트가 위치하는 경우, 일반적으로 index.tsx 파일에 해당 컴포넌트들을 한데 모아두거나, src/components/index.ts 파일을 만들어서 header.tsx 파일을 export하는 방법을 주로 사용합니다. 이렇게 작성하면 import문이 너무 길어지는 상황을 피할 수 있습니다.

Comment on lines +11 to +13
interface setFolderDataId {
setFolderDataId?: Dispatch<SetStateAction<number>>;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Header의 props를 나타내는 타입을 작성할때는 HeaderProps와 같이 작성하는 것이 일반적입니다.
그리고 헤더의 입장에서 setFolderDataId라는 함수를 받아야할 이유가 없다고 생각합니다.
지금 헤더 내부에 작성된 비즈니스 로직은 헤더 보다 더 상위 요소에서 작성되고 헤더에서는 로그인 된 정보만 받으면 될것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

비즈니스 로직이 길어지게 되면 해당 로직을 커스텀 훅으로 별도로 분리 해보는 것도 하나의 방법이 될 수 있습니다.
먼저 하나의 커스텀 훅으로 분리를 해보고 해당 훅으로부터 연관된것들끼리 묶어 별도의 커스텀 훅으로 쪼개나가는 것도 좋은 리팩토링 방법이 될것으로 보여집니다.

} catch (e) {
if (e instanceof Error) {
alert(e.message);
}
}
};

export const loginFetchData = async () => {
export const checkEmailAvailability = async (email: string) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 함수명은 validateEmail로도 사용할 수 있을것 같습니다!

@clianor
Copy link
Collaborator

clianor commented May 30, 2024

저번에 말씀드렸던 내용들 위주로 개선이 된 것으로 보이네요.
약간 아쉬운 점은 타입들이 좀더 적극적으로 재사용되지 못한 점, 비즈니스 로직을 포함하지 않아도 될 컴포넌트 내에서 비즈니스 로직이 포함되는 것으로 보여집니다.
이런 부분들을 개선하면 좋을것 같네요!

@clianor clianor merged commit 0aca9a5 into codeit-bootcamp-frontend:part3-김한샘 May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants