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

Conversation

rladmswo1715
Copy link
Collaborator

요구사항

기본

  • 링크 공유 페이지의 url path를 ‘/shared/{folderId}’로 변경하고, {folderId}에 해당하는 폴더 데이터가 화면에 보이게 해주세요.
  • 폴더 페이지의 url path를 전체 링크를 보는 경우 ‘/folder’로, 특정 폴더를 보는 경우 ‘/folder/{folderId}’로 변경하고, {folderId}에 해당하는 폴더 데이터가 화면에 보이게 해주세요.

심화

  • 리퀘스트 헤더에 인증 토큰을 첨부할 때 axios interceptors를 활용해 주세요. (axios를 사용하지 않는다면 이와 유사한 기능을 활용해 주세요.)리퀘스트 헤더에 인증 토큰을 첨부할 때 axios interceptors를 활용해 주세요. (axios를 사용하지 않는다면 이와 유사한 기능을 활용해 주세요.)

주요 변경사항

스크린샷

멘토에게

@rladmswo1715 rladmswo1715 requested a review from choinashil May 24, 2024 09:45
@O-daeun O-daeun added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label May 27, 2024
Copy link
Collaborator

@choinashil choinashil 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 +19 to +20
navId: string | string[] | undefined,
userToken: string | undefined
Copy link
Collaborator

Choose a reason for hiding this comment

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

undefined 인 경우 아래와 같이 선택적 프로퍼티로 작성할 수도 있습니다!

Suggested change
navId: string | string[] | undefined,
userToken: string | undefined
navId?: string | string[],
userToken?: string

) => {
let query = "/api/links";
if (navId) {
query += `?folderId=${navId}`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 넣는다면 navId 는 string[] 타입인 경우보다는 string 타입인 경우만 해당될 것 같네요
19라인에서는 string | string[] 으로 정의되어있는데 string[] 으로 들어오는 케이스가 있는지 확인 후, 없다면 타입을 좁혀주시면 좋을 것 같습니다!

다른 부분도 마찬가지

@@ -39,6 +43,7 @@ const LinkAddContent = () => {
<S.Content>
<S.FolderList>
{navList.map((navItem: INavItem, i) => {
console.log(navItem);
Copy link
Collaborator

Choose a reason for hiding this comment

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

PR 올린 후 불필요한 코드가 있는지 꼭 확인해주세요

Suggested change
console.log(navItem);

Comment on lines +70 to +74
isCurrentNav={
pageNavId && navItem.id?.toString() === pageNavId[0]
? true
: false
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

조건문 자체가 boolean 이므로 간단하게 변경해볼수있겠네요

Suggested change
isCurrentNav={
pageNavId && navItem.id?.toString() === pageNavId[0]
? true
: false
}
isCurrentNav={
pageNavId && navItem.id?.toString() === pageNavId[0]
}


interface NavItemProps {
navName?: string;
navId?: number;
onClick: (navId: number) => void;
//onClick: (navId: number) => void;
Copy link
Collaborator

Choose a reason for hiding this comment

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

미사용 코드 삭제!

const handleLogout = () => {
window.localStorage.removeItem("userToken");

router.reload();
Copy link
Collaborator

Choose a reason for hiding this comment

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

새로고침 할 수도 있고,
전역에서 로그인 여부를 관리하는 상태를 참조하도록 해도 될 것 같아요
로그인 할 때 토큰 세팅하면서 해당 상태도 같이 true 로 변경해주고,
로그아웃할 때는 토큰 삭제하면서 같이 false 로 변경

이러면 새로고침 하지 않아도 상태 변경에 따라 새로고침 없이도 화면에 반영될 수 있을거고요

꼭 수정해야한다는 의미는 아니고 다양한 방법이 있다는 정도로 이해하시면 됩니다

Comment on lines +39 to +42
id: userInfo.id,
name: userInfo.name,
image_source: userInfo.image_source,
email: userInfo.email,
Copy link
Collaborator

Choose a reason for hiding this comment

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

userInfo 에 id, name, image_source, email 만 포함되어있다면 아래처럼 작성할 수도 있겠네요

Suggested change
id: userInfo.id,
name: userInfo.name,
image_source: userInfo.image_source,
email: userInfo.email,
...userInfo

Comment on lines +68 to +70
// run build 에러때문에 잠시 작성
const handleSearchChange = (value: string) => {};
const handleDeleteBtn = () => {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

SearchBar 에서 꼭 필요한 prop 맞나요?? 아니라면 SearchBar 의 prop 타입에서 두 함수를 옵셔널로 처리해도 될 것 같아요

@@ -60,6 +52,11 @@ const GlobalStyle = createGlobalStyle<GlobalStyleProps>`
cursor: pointer;
background: none;
}

a{
Copy link
Collaborator

Choose a reason for hiding this comment

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

작은 여백도 꼼꼼하게 챙기기!

Suggested change
a{
a {

}
`;

export const EmailBox = styled.div`
margin-top: 24px;
`;

export const PassWrodBox = styled.div`
export const PassWordBox = styled.div`
Copy link
Collaborator

Choose a reason for hiding this comment

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

password 는 한 단어이므로 중간에 대문자 대신 전체 소문자로 작성해야해요

Suggested change
export const PassWordBox = styled.div`
export const PasswordBox = styled.div`

@choinashil choinashil merged commit f22912f into codeit-bootcamp-frontend:part3-김은재 May 27, 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.

3 participants