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

코드리뷰 반영 + 로그아웃 기능 추가 #129

Merged
merged 11 commits into from
Oct 30, 2024
Merged

Conversation

Diwoni
Copy link
Collaborator

@Diwoni Diwoni commented Oct 29, 2024

#️⃣ 연관된 이슈

ex) #124

📝 작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요.(이미지 첨부 가능)

로그아웃 API 연동

  • 로그아웃 API 연동하였습니다.

회원가입 후 리다이렉트 적용

  • 기존에 ROOT 로 해놓았던 임시 루트를 시니또 보호자 별로 홈으로 리다이렉트하게 변경하였습니다.

코드리뷰 반영

  • 일부 핸들러함수 이름 변경, custom hook 으로 분리할 수 있는 내용들은 분리하였습니다.

스크린샷 (선택)

💬 리뷰 요구사항(선택)

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

ex) 메서드 XXX의 이름을 더 잘 짓고 싶은데 혹시 좋은 명칭이 있을까요?

⏰ 현재 버그

✏ Git Close

close #124

@Diwoni Diwoni added ✨ Feature 새로운 기능 추가 및 구현하는 경우 📡 API 비동기 통신 코드를 짜는 경우, 백엔드와의 통신하는 경우 ♻️ Refactoring 코드 리팩토링 & 클린 코드 작업을 진행하는 경우 labels Oct 29, 2024
@Diwoni Diwoni requested review from Dobbymin and JYN523 October 29, 2024 20:05
@Diwoni Diwoni self-assigned this Oct 29, 2024
@Diwoni Diwoni linked an issue Oct 29, 2024 that may be closed by this pull request
@Diwoni
Copy link
Collaborator Author

Diwoni commented Oct 29, 2024

충돌 해결하는 과정에서, 로컬 브랜치랑 원격 브랜치 간 파일 하나가 달라져서 마지막 커밋에서 해결했습니다..

Copy link
Contributor

@Dobbymin Dobbymin left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

로그아웃 버튼의 위치를 위로 올리는건 어떨까요??
image

이 부분의 오른쪽 상단 혹은 마이페이지의 상단에 위치하면 조금 더 편할것 같다는 생각이 드네요.

그리고, 전체적으로 마이페이지가 답답해보여요 ㅠ margin 값을 좀 추가하는것도 좋아보이네요!

코드 한줄이라도 줄일려고 Box 대신 Flex로 수정했는데 다시 Box를 사용하면서 태초마을로 돌아가버렸네요 ㅠㅠ

Copy link
Contributor

Choose a reason for hiding this comment

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

로그아웃을 진행하고 로그아웃되었다는 알람창이 하나 떴으면 좋겠어요. 바로 첫페이지로 돌아가버리네요

Comment on lines 1 to 2
import { useLogout } from '@/shared/api/hooks/useLogout';
import { BasicButton } from '@/shared/components/common';
Copy link
Contributor

Choose a reason for hiding this comment

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

절대경로로 묶을 수 있지 않을까요?

Copy link
Collaborator

@JYN523 JYN523 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 +6 to +31
type GuidelineInfo = {
id: number;
type: string;
title: string;
content: string;
};

type UseGuidelineInfoProps = {
guideline: GuidelineInfo;
seniorId: number;
editMutation: UseMutationResult<string, Error, ModifyGuidelineRequest>;
deleteMutation: UseMutationResult<string, Error, number>;
};

type UseGuidelineInfoReturn = {
isMore: boolean;
isEditing: boolean;
guidelineTitle: string;
guidelineContent: string;
toggleContent: () => void;
setIsEditing: (value: boolean) => void;
setGuidelineTitle: (value: string) => void;
setGuidelineContent: (value: string) => void;
editGuideline: () => void;
deleteGuideline: () => void;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

type은 따로 분리하는 것도 괜찮을 것 같아요!

@Diwoni Diwoni merged commit cde90cc into Weekly Oct 30, 2024
1 check passed
@Diwoni Diwoni deleted the Feat/issue-#124 branch October 30, 2024 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📡 API 비동기 통신 코드를 짜는 경우, 백엔드와의 통신하는 경우 ✨ Feature 새로운 기능 추가 및 구현하는 경우 ♻️ Refactoring 코드 리팩토링 & 클린 코드 작업을 진행하는 경우
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8주차 코드리뷰 반영 및 보호자 홈 기능구현
3 participants