-
Notifications
You must be signed in to change notification settings - Fork 5
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
[Feature] - 데모데이 피드백 반영(리버) #444
Changes from 7 commits
2c7830b
b74deda
7139021
45fbdb4
767d056
d6eaf39
39d8f45
cdda624
62d5aa7
62be1b9
a73e181
58ac69d
6fafcd6
49e292d
0010fd3
c666cab
eb6278c
ef79495
55dbc7f
59c9621
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ import { ROUTE_PATHS_MAP } from "@constants/route"; | |
|
||
import { extractLastPath } from "@utils/extractId"; | ||
import getDateRange from "@utils/getDateRange"; | ||
import getDaysAndNights from "@utils/getDaysAndNights"; | ||
import { isUUID } from "@utils/uuid"; | ||
|
||
import theme from "@styles/theme"; | ||
|
@@ -36,10 +37,7 @@ const TravelPlanDetailPage = () => { | |
|
||
const navigate = useNavigate(); | ||
|
||
const daysAndNights = | ||
data?.days.length && data?.days.length > 1 | ||
? `${data?.days.length - 1}박 ${data?.days.length}일` | ||
: "당일치기"; | ||
const daysAndNights = getDaysAndNights(data?.days); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. util로 분리한 거 좋네요 |
||
|
||
const { mutate: mutateDeleteTravelPlan, isPending: isDeletingPending } = useDeleteTravelPlan(); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ import { ERROR_MESSAGE_MAP } from "@constants/errorMessage"; | |
import { ROUTE_PATHS_MAP } from "@constants/route"; | ||
|
||
import { extractID } from "@utils/extractId"; | ||
import getDaysAndNights from "@utils/getDaysAndNights"; | ||
|
||
import theme from "@styles/theme"; | ||
import { SEMANTIC_COLORS } from "@styles/tokens"; | ||
|
@@ -42,10 +43,7 @@ const TravelogueDetailPage = () => { | |
|
||
const navigate = useNavigate(); | ||
|
||
const daysAndNights = | ||
data?.days.length && data?.days.length > 1 | ||
? `${data?.days.length - 1}박 ${data?.days.length}일` | ||
: "당일치기"; | ||
const daysAndNights = getDaysAndNights(data?.days); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 반복되는 로직 분리해주셔서 감사합니다 :) |
||
|
||
const { onTransformTravelDetail } = useTravelTransformDetailContext(); | ||
const { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import { useMemo } from "react"; | ||
|
||
import type { TravelTransformPlaces } from "@type/domain/travelTransform"; | ||
|
||
import { useUserProfile } from "@queries/useUserProfile"; | ||
|
||
import getDaysAndNights from "@utils/getDaysAndNights"; | ||
|
||
type TripRecordType = "travelogue" | "travelPlan"; | ||
|
||
type useInitialTripTitleProps = { | ||
days: TravelTransformPlaces[] | undefined; | ||
type: TripRecordType; | ||
}; | ||
|
||
const useInitialTripTitle = ({ days, type }: useInitialTripTitleProps) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사소한거지만 travel이라는 용어를 계속 사용해왔어서 그런지 통일하면 좋겠네요 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그렇네요~~ 반영했습니다! |
||
const { data, status } = useUserProfile(); | ||
|
||
return useMemo(() => { | ||
const daysAndNights = getDaysAndNights(days); | ||
|
||
const userPrefix = status === "success" ? `${data?.nickname}의 ` : ""; | ||
const tripType = type === "travelogue" ? "여행기" : "여행 계획"; | ||
|
||
return `${userPrefix}${daysAndNights} ${tripType}`; | ||
}, [days, data, status, type]); | ||
}; | ||
|
||
export default useInitialTripTitle; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
import { useEffect, useState } from "react"; | ||
import { useLocation, useNavigate } from "react-router-dom"; | ||
|
||
import { ROUTE_PATHS_MAP } from "@constants/route"; | ||
|
||
type PreviousPages = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 수정했습니다! |
||
[key: string]: string; | ||
}; | ||
|
||
const usePreviousPage = () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 궁금증 1 - previousPages의 경우 마이페이지, 로그인 페이지를 제외하곤 객체 내 하나의 프로퍼티만 들어오는데 혹시 이유를 알 수 있을까요 .. ? 혼자 찾아볼 땐 이유를 알기가 어렵네요 ㅜㅜㅜ... 궁금증 2 - useState로 관리하는 이유가 있을까요 ?! 위 사진 case에서 새로고침을 하니까 previousPages 데이터 내 프로퍼티가 하나만 존재하는 걸로 바뀌어서여(요건 그냥 localStorage에 데이터를 저장하고 useState에서 데이터를 받아오는 형태로 하면 어떨까 싶기도 합니다) 궁금증 3 - 의존성 배열에 pathname만 변경해도 충분하지 않을까 싶긴 한데 location 객체를 의존성 배열에 추가한 이유가 있을까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 지니랑 얘기한대로 state 불필요, 그전 페이지만 저장만으로 충분하다 생각해서 그렇게 수정했습니다! |
||
const navigate = useNavigate(); | ||
const location = useLocation(); | ||
const [previousPages, setPreviousPages] = useState<PreviousPages>({}); | ||
|
||
useEffect(() => { | ||
setPreviousPages((prev) => ({ | ||
...prev, | ||
[location.pathname]: document.referrer, | ||
})); | ||
}, [location]); | ||
|
||
const goBack = () => { | ||
const previousPage = previousPages[location.pathname]; | ||
const currentPage = location.pathname; | ||
const isPreviousPageMy = previousPage?.includes(ROUTE_PATHS_MAP.my); | ||
const isCurrentPageLogin = currentPage.includes(ROUTE_PATHS_MAP.login); | ||
|
||
if (isPreviousPageMy && isCurrentPageLogin) { | ||
navigate(ROUTE_PATHS_MAP.root); | ||
} else { | ||
navigate(ROUTE_PATHS_MAP.back); | ||
} | ||
}; | ||
Comment on lines
+12
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 개인적으로 얘를 좀 더 재사용성 높게 수정하면 좋을 거 같습니다 우리 계속 고민하던 부분이 이전 페이지가 등록 페이지인 경우 두 번 뒤로 가고 이런 고민들을 했어서 그런 것도 사용할 수 있는 방향이면 좋겠네요 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그런 부분은 이제 이 훅에 분기처리 추가해나가면되지 않을까용?? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이해했습니다 |
||
|
||
return goBack; | ||
}; | ||
|
||
export default usePreviousPage; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import type { TravelTransformPlaces } from "@type/domain/travelTransform"; | ||
|
||
const getDaysAndNights = (days: TravelTransformPlaces[] | undefined) => | ||
days?.length && days?.length > 1 ? `${days.length - 1}박 ${days.length}일` : "당일치기"; | ||
|
||
export default getDaysAndNights; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이전 페이지가 마이페이지고 현재 페이지가 로그인 페이지라면 goBack을 통해 뒤로가기 버튼을 눌러도 마이 페이지로 가는 것이 아닌 루트 페이지로 이동하는 방식이군여 좋습니다 ! !