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

feat: 목표 생성, 목표 상세보기, 목표 삭제 후 현재 페이지 변경 로직 추가 #115

Merged
merged 9 commits into from
Jan 17, 2024

Conversation

deepbig
Copy link
Collaborator

@deepbig deepbig commented Jan 12, 2024

🤔 해결하려는 문제가 무엇인가요?

Goal 신규 생성 및 Goal Detail 조회 후 홈 화면 이동 시 해당 위치로 point 잡기

🎉 어떻게 해결했나요?

  • 목표 생성 시 생성된 목표 스라이드로 이동 (prefetch 적용)
  • 목표 상세 페이지 접근 후 뒤로 가기 수행 시, 같은 슬라이드로 이동
  • 목표 상세 페이지에서 목표 삭제 수행 시, 현재 시간 스라이드로 이동
Record_2024_01_13_03_52_02_13.mp4

📚 Attachment (Option)

  • N/A

Copy link

vercel bot commented Jan 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
amazing3-fe ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 17, 2024 4:32pm

@deepbig
Copy link
Collaborator Author

deepbig commented Jan 17, 2024

엑 요거 rebase하고 나서 비정상 동작하네여;; 고치는데 시간 좀 걸릴 듯..

Copy link

Copy link

Copy link
Member

@newminkyung newminkyung left a comment

Choose a reason for hiding this comment

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

👍
제꺼 리버트한 후에 머지하는게 좋을 것 같아요!

Copy link
Member

@Doeunnkimm Doeunnkimm left a comment

Choose a reason for hiding this comment

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

어메이징 류홍석 갓홍석 빛홍석 킹홍석 수고 많으셨으~~~ 💡✨👍

Copy link
Member

Choose a reason for hiding this comment

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

이건 무슨 페이지죠...???? /goal/detail/saved ..?
세부 목표 페이지에서 더 들어가는 게 있었나요 ..???

Copy link
Collaborator Author

@deepbig deepbig Jan 17, 2024

Choose a reason for hiding this comment

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

/goal/detail 레이아웃에 공통 부분이 있어서 같은 상위 directory에 위치하게 되었습니다. 나중에 리팩토링 대상인 페이지에요~ (두개의 링크가 달라질 필요가 없을 것 같아서..;;)
지금 구조는 /goal/detail/id인 경우 목표 상세 페이지 출력, goal/detail/saved인 경우에 저장된 목표 상세 페이지 출력합니다~

@@ -16,14 +17,15 @@ interface DeleteGoalButtomSheetProps {

export const DeleteGoalButtomSheet = ({ open, onClose, goalId }: DeleteGoalButtomSheetProps) => {
Copy link
Member

Choose a reason for hiding this comment

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

ButtonSheet !?! ㅋㅋㅋㅋㅋㅋㅋ

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

에휴 리팩토링할 때까지 발견되지 않기를 바랐는데 걸려버렸다~~~ 바로 수정~~~

Comment on lines 24 to 26
if (isSuccess) {
onClose();
router.back();
router.push(`/home/${memberData?.username}`);
Copy link
Member

Choose a reason for hiding this comment

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

홍석넴 요거 onSuccess로 처리해줘도 될 거 같아영

// useDeleteGoal

...
export const useDeleteGoal = () => {
  ...
  const router = useRouter();

  return useMutation({
    ...
    onSuccess: () => {
      queryClient.invalidateQueries({ queryKey: ['goals'] });
      const memberData = queryClient.getQueryData(['memberData']) as MemberProps;
      router.push(`/home/${memberData.username}`);
    },
  });
};

Copy link
Member

@Doeunnkimm Doeunnkimm Jan 17, 2024

Choose a reason for hiding this comment

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

아 그리고 삭제 버튼을 누르면 pending 상태일 때 버튼에 스피너 넣는 게 좋을 거 같아용
잠깐 화면이 멈춰있는데 내가 안 누른건가 해서 한번더 눌러서 서버 에러 뜸요 😭

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아주 좋은 피드백 최고최고

@@ -104,8 +125,8 @@ export const LifeMapContent = ({ goalsData, memberData, downloadSectionRef, isPu
) : (
<MapCardPositioner type="B" goals={goals} isLast={page === LAST_PAGE - 1} />
)}
{position && currentPage === page && (
<CurrentPositionCover currentPosition={position % TOTAL_CURRENT_POSITIONS} />
{positionState.positionPage === page && positionState.position && (
Copy link
Member

Choose a reason for hiding this comment

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

positionState.positionPage === page && positionState.position

이거 뭘 의미하는지 한번 상수로 만들어준 다음에 넣어주실 수 있나영 ? 나중에 여기 리팩터링 제가 맡으면 눈물 한번 흘리고 시작할 거 같아서 😭

Copy link
Collaborator Author

@deepbig deepbig Jan 17, 2024

Choose a reason for hiding this comment

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

positionSate.position 체크 로직은 뒤의 컴포넌트 출력 시 null 체크를 먼저 하려고 추가된 거에요. 별도 함수로 빼서 로직을 생성하면 결국은 앞의 positionState.positionPage === page만 남는데 이게 의미가 있을까~??

[도은 피드백 적용 후]

  const isCurrentPage = (page: number) => {
    return positionState.positionPage === page
  };

Copy link
Collaborator Author

@deepbig deepbig Jan 17, 2024

Choose a reason for hiding this comment

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

일단 나중에 도은이가 리팩토링 작업할 때 이해 안될 수도 있으니까 comment로 설명 적어뒀어요~~~~!

{/** 현재 위치에 별 위치 시키기 위해 1) 현재 날짜가 포함된 페이지를 찾아서, 2) 포지션 위치에 별을 출력함. */}
{positionState.positionPage === page && positionState.position && (
  <CurrentPositionCover currentPosition={positionState.position} />

}
}, [currentPosition]);

export const MapSwiper = ({ currentPage, children }: MapSwiperProps) => {
Copy link
Member

Choose a reason for hiding this comment

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

홍석넴 이거 위에서 PropsWithChildren 상속 받기보다

Suggested change
export const MapSwiper = ({ currentPage, children }: MapSwiperProps) => {
export const MapSwiper = ({ currentPage, children }: PropsWithChildren<MapSwiperProps>) => {

도 가능하답니다잇 (별건 아니여서 시간 있을 때 😉

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

반영하긴 했는데 이렇게 하면 좋은 점이 있나요~?? 👀 잘모르겠어서 👀👀

Comment on lines +30 to +36
export const usePrefetchGoals = (): void => {
const queryClient = useQueryClient();
queryClient.prefetchQuery({
queryKey: ['goals'],
queryFn: () => api.get<GoalResponse>('/life-map'),
});
};
Copy link
Member

Choose a reason for hiding this comment

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

요거 민경넴이 작업하신 거랑 중복되어서 메인 페이지에서 goals 관련된 요청이 2번 되고 있는데

한번 체크 부탹 😉 글고 prefetch 안 되고 있는 거 같은데 요것도 확인 한번 부탁 🙏

Copy link
Collaborator

@hjy0951 hjy0951 left a comment

Choose a reason for hiding this comment

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

리뷰가 늦어 죄송...
너무 고생했슴다~! 🫡

const [currentPage, setCurrentPage] = useState<number | null>(null);
const paramId = Number(useSearchParams().get('id'));
Copy link
Collaborator

Choose a reason for hiding this comment

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

크게 불편한건 아니지만 paramGoalId 같이 어떤 ID인지 넣어주면 좋을것 같아!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

반영 완~

@deepbig deepbig merged commit 78d925e into main Jan 17, 2024
2 checks passed
@deepbig deepbig deleted the feat/home-position-after-goal-detail-saved branch January 17, 2024 16:36
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.

4 participants