-
Notifications
You must be signed in to change notification settings - Fork 0
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
[DETAIL] 상점 상세 페이지 URL 복사 및 북마크 구현 #124
Changes from all commits
d3aa974
36a3696
cb2dedb
9a91cdf
1793744
22ecaac
5206053
11f58eb
0fa5ad5
6807ecd
bebc87a
ea5f354
e7e781a
422a4a5
8ffe52c
b0c4c55
1e555b1
132f503
2e3a487
1b12a73
4c14198
dac37e3
edcfc18
0376b56
a5c0e14
906f35c
b891fd7
2bceb21
3b2915e
6b4b2d6
3fb9d34
dd72fec
cb74e48
dafe87a
b4a9e15
9aef4d3
7ac76f2
6fce323
82b3f73
e13bd6a
f766c83
58e5c7a
cad0299
7cd9b20
60a81de
1830df6
25b9bab
4915264
a7e5fde
7f24746
fccf89a
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 |
---|---|---|
@@ -0,0 +1,46 @@ | ||
export interface GetMyScrapShopResponse { | ||
content: { | ||
address: string; | ||
category: string; | ||
createdAt: string; | ||
directory: { | ||
createdAt: string; | ||
id: number; | ||
name: string; | ||
scrapCount: number; | ||
updatedAt: string; | ||
}; | ||
name: string; | ||
photo: string; | ||
placeId: string; | ||
ratingCount: number; | ||
scrapId: number; | ||
totalRating: number; | ||
updatedAt: string; | ||
}[]; | ||
empty: boolean; | ||
first: boolean; | ||
last: boolean; | ||
number: number; | ||
numberOfElements: number; | ||
pageable: { | ||
offset: number; | ||
pageNumber: number; | ||
pageSize: number; | ||
paged: boolean; | ||
sort: { | ||
empty: boolean; | ||
sorted: boolean; | ||
unsorted: boolean; | ||
}; | ||
unpaged: boolean; | ||
}; | ||
Comment on lines
+26
to
+37
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. 위의 코멘트로 대체하겠습니다~! |
||
size: number; | ||
sort: { | ||
empty: boolean; | ||
Comment on lines
+24
to
+40
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. 아마 이런 페이지 관련 프로퍼티들은 서버에서도 동일한 DTO로 사용중일텐데, 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. 저도 인지를 어느정도 하고 있긴 합니다만.. 바꾸려고 보니 이미 관련 타입들을 동일하게 여러개 만들어두고 사용하고 있더라구요. ex) InquiryPageable, PostPageable 그래서 이 타입을 따로 분리한다면 위 타입들까지 포함해서 전부 바꿔주고 싶은데, 그러기에는 너무 많은 파일들을 건드려서 일단 두었습니다. 이 부분은 가능하면 따로 이슈를 파서 작업을 해보는 방향으로 가볼게요! 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. 쪼아영 타입들 한번 싹 손대기 고고 |
||
sorted: boolean; | ||
unsorted: boolean; | ||
}; | ||
totalElements: number; | ||
totalPages: number; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import scrapApi from './scrapApiClient'; | ||
import type { GetMyScrapShopResponse } from './entity'; | ||
|
||
export const postScrapShop = (shopId: string) => | ||
// post된 음식점이 들어가는 최상위 directoryId는 0입니다. | ||
scrapApi.post('/scraps', { directoryId: 0, placeId: shopId }); | ||
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. 으흠.. 주석을 웬만하면 정말 안쓰려고 하다보니 살짝 낯서네요 ㅎㅎ.. 한 번 추가해볼게요~! |
||
|
||
export const getMyScrapShop = () => scrapApi.get<GetMyScrapShopResponse>('/scraps'); | ||
|
||
export const deleteScrapShop = (scrapId: number) => scrapApi.delete(`/scraps/${scrapId}`); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
import axios from 'axios'; | ||
import { API_PATH } from 'config/constants'; | ||
|
||
const scrapApi = axios.create({ | ||
baseURL: `${API_PATH}`, | ||
timeout: 2000, | ||
}); | ||
|
||
scrapApi.interceptors.request.use((config) => { | ||
const accessToken = sessionStorage.getItem('accessToken'); | ||
// eslint-disable-next-line no-param-reassign | ||
if (config.headers && accessToken) config.headers.Authorization = `Bearer ${accessToken}`; | ||
return config; | ||
}); | ||
|
||
export default scrapApi; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import useNaverMap from 'pages/Home/components/Map/hooks/useNaverMap'; | ||
import makeToast from 'utils/ts/makeToast'; | ||
import SectionHeader from '../SectionHeader'; | ||
import styles from './Map.module.scss'; | ||
|
||
|
@@ -9,14 +10,24 @@ interface Props { | |
} | ||
|
||
function Map({ formattedAddress, latitude, longitude }: Props) { | ||
const copyURL = () => { | ||
const urlToCopy = window.location.href; | ||
|
||
navigator.clipboard.writeText(urlToCopy).then(() => { | ||
makeToast('success', 'URL을 클립보드에 복사하였습니다.'); | ||
}).catch(() => { | ||
makeToast('error', 'URL을 복사하는데 실패했습니다.'); | ||
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. 너무 지당한 말입니다. 사실 어느정도의 ux 를 포기했습니다 허허.. 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. 간단한 방법으로는 대부분의 크롬 버전 / 브라우저에서 지원될 테니깐 간단하게 |
||
}); | ||
}; | ||
|
||
useNaverMap(latitude, longitude); | ||
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. useMarker로 위치 표시하는 것은 어떨까요? 축소하니까 위치 파악이 안 되는 것 같아서요!(물론 선택 사항입니닿ㅎㅎ) 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. 이건... 저도 마커를 넣는게 더 좋다고 생각은 했지만, ui 디자인 상에서 마커가 없었기 때문에 일부러 넣지 않았습니다! 그리고 useMarker는 마커를 찍기 위해 상점 이름, 대표사진이 필요해요.(아마 마커를 표시하는데, 상점이름과 이름을 넣어주고 싶었던 거겠죠?) |
||
|
||
return ( | ||
<section className={styles.container}> | ||
<SectionHeader | ||
title="지도" | ||
description={formattedAddress} | ||
button={{ content: 'URL 복사', onClick: () => {} }} | ||
button={{ content: 'URL 복사', onClick: copyURL }} | ||
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. 저도 조합을 좀 생각하기는 했는데(사실 지금도 조합이 좀 고프지만..) |
||
/> | ||
<div id="map" className={styles.map} /> | ||
</section> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
.scrap-button { | ||
display: flex; | ||
gap: 2px; | ||
align-items: center; | ||
justify-content: center; | ||
background: #f8f8f8; | ||
padding: 16px 24px; | ||
border-radius: 4px; | ||
font-size: 16px; | ||
color: #666; | ||
transition: all 0.2s ease; | ||
|
||
&--active { | ||
background: #ff7f23; | ||
color: #fff; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import useScrap from 'utils/hooks/useScrap'; | ||
import cn from 'utils/ts/classNames'; | ||
import { ReactComponent as BookMarkIcon } from 'assets/svg/shop/book-mark.svg'; | ||
import styles from './ScrapButton.module.scss'; | ||
|
||
interface Props { | ||
placeId: string; | ||
initialScrapId?: number | null; | ||
} | ||
|
||
function ScrapButton({ placeId, initialScrapId = null }: Props) { | ||
const { scrapId, toggleScrap, isPending } = useScrap(placeId, initialScrapId); | ||
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. scrapId를 받는 인터페이스도 특이한 것 같은데, 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.
초기에 음식점 상세 데이터를 불러오면 scrap 여부를 number | null 형식으로 주더라구요. 그래서 데이터 형식을 좀 일관화 하고 싶어서, 이걸 그대로 따라갔습니다. 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. isScrap등의 불린값으로 써야한다고 생각했고, |
||
|
||
return ( | ||
<button | ||
type="button" | ||
onClick={toggleScrap} | ||
disabled={isPending} | ||
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. pending 상태일때 disabled로 동작을 막는 디테일은 생각해보지 못했네요.. 배워갑니다ㅎㅎ |
||
className={cn({ | ||
[styles['scrap-button']]: true, | ||
[styles['scrap-button--active']]: scrapId !== null, | ||
})} | ||
> | ||
{scrapId ? <BookMarkIcon fill="#fff" stroke="#fff" /> : <BookMarkIcon stroke="#666" />} | ||
<span>{scrapId ? '북마크된 장소' : '북마크 하기'}</span> | ||
</button> | ||
); | ||
} | ||
|
||
export default ScrapButton; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import { useMutation } from '@tanstack/react-query'; | ||
import { deleteScrapShop, postScrapShop } from 'api/scrap'; | ||
import { useState } from 'react'; | ||
|
||
const useScrap = (placeId: string, initialScrapId: number | null) => { | ||
const [scrapId, setScrapId] = useState<number | null>(initialScrapId); | ||
|
||
const { mutate: postScrap, isPending: postPending } = useMutation({ | ||
mutationFn: postScrapShop, | ||
onSuccess: (res) => setScrapId(res.data.id), | ||
}); | ||
|
||
const { mutate: deleteScrap, isPending: deletePending } = useMutation({ | ||
mutationFn: deleteScrapShop, | ||
}); | ||
|
||
const toggleScrap = () => { | ||
if (scrapId) { | ||
deleteScrap(scrapId); | ||
setScrapId(null); | ||
} else { | ||
postScrap(placeId); | ||
} | ||
}; | ||
|
||
const isPending = postPending || deletePending; | ||
|
||
return { scrapId, toggleScrap, isPending }; | ||
}; | ||
|
||
export default useScrap; |
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.
API 가독성을 위해 밖으로 빼는 것은 어떨까요?
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.
배열 요소를 외부로 빼면 그래도 살짝은 가독성이 좋아지긴 하겠지만, 이것 때문에 분리하는 것은 살짝 반대입니다..
이 타입 자체에서 명시되어 있듯이 스크랩한 상점의 응답값인 것을 알 수 있기 때문에 어노정도 유추할 수 있다는 생각도 해봅니다..!
그리고 타입을 새로 만들 때는 그래도 외부에서 쓰일 수 있을 때 분리하는 편입니다. 이미 해당 타입에 맞는 정보가 타입으로 정의되어 있는 상태라면 가져다 쓰면 좋겠지만, 쩝박에서는 상점 정보의 타입이 상점 조회, 스크랩된 상점 조회 등에서 다 다른 형식으로 주더라구요. 그러다보니 아직 해당 배열 요소를 특정해서 외부에서 쓰지 않고 있으니 크게 분리의 필요성을 느끼지 못하겠더라구요.. 물론 묶어주고 싶은 그 느낌(?)은 이해합니다..! 추후에 이 요소가 세부적으로 사용될 일이 생기면 그 때 분리를 생각해도 괜찮지 않나 싶습니다!
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.
넵ㅋㅋㅋ확인했습니다! 설명감사합니다!