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

[DETAIL] 상점 상세 페이지 URL 복사 및 북마크 구현 #124

Merged
merged 51 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
d3aa974
Merge commit '3dd27b16c21891eb16b950346806161ea3525988'
D0Dam Nov 1, 2023
36a3696
feat: shop 응답값 타입 정의
D0Dam Nov 1, 2023
cb2dedb
refactor: 오타 수정
D0Dam Nov 1, 2023
9a91cdf
feat: useCounter 구현
D0Dam Nov 1, 2023
1793744
feat: marker 클릭 이벤트 추가
D0Dam Nov 1, 2023
22ecaac
feat: ImageCarousel 구현
D0Dam Nov 1, 2023
5206053
feat: ShopDetail Page 구조 잡기
D0Dam Nov 1, 2023
11f58eb
refactor: typo error 수정
D0Dam Nov 2, 2023
0fa5ad5
refactor: imageCarousel css 수정
D0Dam Nov 2, 2023
6807ecd
feat: StarRatingPreview 구현
D0Dam Nov 3, 2023
bebc87a
feat: ShopDetail 에 기본 정보 ui 추가
D0Dam Nov 3, 2023
ea5f354
refactor: 불필요한 eslin 옵션 off
D0Dam Nov 7, 2023
e7e781a
refactor: detail 페이지 경로 수정
D0Dam Nov 7, 2023
422a4a5
feat: fetch 로직 및 타입 설계
D0Dam Nov 7, 2023
8ffe52c
feat: 데이터 확인을 위한 mock 추가
D0Dam Nov 7, 2023
b0c4c55
feat: not found svg 추가
D0Dam Nov 7, 2023
1e555b1
feat: SectionHeader 구현
D0Dam Nov 7, 2023
132f503
feat: ISO 형식 날짜 변환 함수 구현
D0Dam Nov 7, 2023
2e3a487
feat: Review 관련 컴포넌트 구현
D0Dam Nov 7, 2023
1b12a73
feat: Map 구현
D0Dam Nov 7, 2023
4c14198
feat: NotFound 컴포넌트 구현
D0Dam Nov 7, 2023
dac37e3
feat: ShopDetail에 컴포넌트 연결 및 grid 적용
D0Dam Nov 7, 2023
edcfc18
Squashed commit of the following: d0dam
D0Dam Nov 7, 2023
0376b56
fix: 충돌간 에러사항 수정
D0Dam Nov 7, 2023
a5c0e14
Merge branch 'develop' into feature/#56
D0Dam Nov 7, 2023
906f35c
fix: lint 에러 수정
D0Dam Nov 7, 2023
b891fd7
fix: lint 에러 수정
D0Dam Nov 7, 2023
2bceb21
refactor: bem 형식에 맞게 className 수정
D0Dam Nov 8, 2023
3b2915e
refactor: 불확실한 타입명 및 타입 재설정
D0Dam Nov 14, 2023
6b4b2d6
refactor:불필요한 타입 추론 제거
D0Dam Nov 14, 2023
3fb9d34
refactor: css 코드 최적화
D0Dam Nov 14, 2023
dd72fec
refactor: SectionHeader Button 관련 props 네이밍 변경
D0Dam Nov 14, 2023
cb74e48
refactor: 코드 이해를 위한 주석 추가
D0Dam Nov 14, 2023
dafe87a
refactor: 통일감을 위한 className 변경
D0Dam Nov 14, 2023
b4a9e15
hotfix: mock 에 사용되는 google api 제거
D0Dam Nov 14, 2023
9aef4d3
Merge remote-tracking branch 'origin/develop' into feature/#56
D0Dam Nov 15, 2023
7ac76f2
refactor: tanstack query 버전 변경에 따른 useQuery 문 재작성
D0Dam Nov 15, 2023
6fce323
refactor: className snake case로 변경
D0Dam Nov 15, 2023
82b3f73
Squashed commit of the following:
D0Dam Nov 20, 2023
e13bd6a
chore: lock 파일 변경 반영
D0Dam Nov 20, 2023
f766c83
feat: url 복사 로직 추가
D0Dam Nov 20, 2023
58e5c7a
feat: 북마크 기능 추가
D0Dam Nov 20, 2023
cad0299
refactor: ScrapButton 분리
D0Dam Nov 20, 2023
7cd9b20
Merge commit 'e17f28772a2fda795586b3dccc8e5ff2aff7ce31' into feature/#56
D0Dam Nov 20, 2023
60a81de
fix: lint 에러 수정
D0Dam Nov 20, 2023
1830df6
refactor: 불필요한 로직 제거, 토글 시 문구 변경
D0Dam Nov 21, 2023
25b9bab
refactor: 리뷰 반영
D0Dam Nov 26, 2023
4915264
Merge commit '50f20360b94a7d6f56ae097dccd514dc87b0e759' into feature/#56
D0Dam Dec 13, 2023
a7e5fde
refactor: 불필요한 타입 단언 제거 및 api 함수 형식 통일
D0Dam Dec 13, 2023
7f24746
refactor: api 이해를 위한 주석 추가
D0Dam Dec 13, 2023
fccf89a
refactor: 불필요한 vs setting 제거
D0Dam Dec 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions src/api/scrap/entity.ts
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;
}[];
Comment on lines +2 to +20
Copy link
Contributor

Choose a reason for hiding this comment

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

API 가독성을 위해 밖으로 빼는 것은 어떨까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

배열 요소를 외부로 빼면 그래도 살짝은 가독성이 좋아지긴 하겠지만, 이것 때문에 분리하는 것은 살짝 반대입니다..
이 타입 자체에서 명시되어 있듯이 스크랩한 상점의 응답값인 것을 알 수 있기 때문에 어노정도 유추할 수 있다는 생각도 해봅니다..!

그리고 타입을 새로 만들 때는 그래도 외부에서 쓰일 수 있을 때 분리하는 편입니다. 이미 해당 타입에 맞는 정보가 타입으로 정의되어 있는 상태라면 가져다 쓰면 좋겠지만, 쩝박에서는 상점 정보의 타입이 상점 조회, 스크랩된 상점 조회 등에서 다 다른 형식으로 주더라구요. 그러다보니 아직 해당 배열 요소를 특정해서 외부에서 쓰지 않고 있으니 크게 분리의 필요성을 느끼지 못하겠더라구요.. 물론 묶어주고 싶은 그 느낌(?)은 이해합니다..! 추후에 이 요소가 세부적으로 사용될 일이 생기면 그 때 분리를 생각해도 괜찮지 않나 싶습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

넵ㅋㅋㅋ확인했습니다! 설명감사합니다!

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
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
Contributor Author

Choose a reason for hiding this comment

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

위의 코멘트로 대체하겠습니다~!

size: number;
sort: {
empty: boolean;
Comment on lines +24 to +40
Copy link
Member

Choose a reason for hiding this comment

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

아마 이런 페이지 관련 프로퍼티들은 서버에서도 동일한 DTO로 사용중일텐데,
클라이언트에서도 재사용하기 좋게끔 PaginationableResponse 정도의 이름으로 모아두면 extends를 적절히 활용해 쓰기 좋게 만들 수 있을 것 같아여
공지사항쪽에도 동일한 프로퍼티를 봤던 것 같은 느낌이 있네여

Copy link
Contributor Author

Choose a reason for hiding this comment

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

저도 인지를 어느정도 하고 있긴 합니다만.. 바꾸려고 보니 이미 관련 타입들을 동일하게 여러개 만들어두고 사용하고 있더라구요.

ex) InquiryPageable, PostPageable

그래서 이 타입을 따로 분리한다면 위 타입들까지 포함해서 전부 바꿔주고 싶은데, 그러기에는 너무 많은 파일들을 건드려서 일단 두었습니다. 이 부분은 가능하면 따로 이슈를 파서 작업을 해보는 방향으로 가볼게요!

Copy link
Member

Choose a reason for hiding this comment

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

쪼아영 타입들 한번 싹 손대기 고고

sorted: boolean;
unsorted: boolean;
};
totalElements: number;
totalPages: number;
}
10 changes: 10 additions & 0 deletions src/api/scrap/index.ts
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 });
Copy link
Member

Choose a reason for hiding this comment

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

directoryId: 0 인 이유를 주석이나 PR 코멘트에 추가되어있으면 좋을 것 같아요~

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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}`);
16 changes: 16 additions & 0 deletions src/api/scrap/scrapApiClient.ts
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;
2 changes: 1 addition & 1 deletion src/assets/svg/shop/book-mark.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 0 additions & 12 deletions src/pages/ShopDetail/ShopDetail.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -80,18 +80,6 @@
font-size: 36px;
font-weight: normal;
}

& > button {
display: flex;
gap: 2px;
align-items: center;
justify-content: center;
background: #f8f8f8;
padding: 16px 24px;
border-radius: 4px;
font-size: 16px;
color: #666;
}
}

&__info-name {
Expand Down
13 changes: 12 additions & 1 deletion src/pages/ShopDetail/components/Map/index.tsx
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';

Expand All @@ -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을 복사하는데 실패했습니다.');
Copy link
Member

Choose a reason for hiding this comment

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

사용하는 입장에서는 실패했으니 어떻게 하라는거지? 정도의 텐션을 보이기때문에
실패한 원인에 대한 해결책을 제시해주거나, 사용자 입장에서 납득할 만한 태도를 보이는것도
프론트도 도울 수 있는 UX 향상이 가능한 부분이예요ㅎ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

너무 지당한 말입니다. 사실 어느정도의 ux 를 포기했습니다 허허..
사실 그 정도의 에러에 대한 ux 를 맞춰주려고 할 때에는 저는 백엔드와 에러코드 정도를 맞춰 둔 정도의 레벨에서 고려를 했었거든요.
물론 지금 더 프론트가 도울 수 있는 ux 측면이 있는 것은 확실하지만, 일단.. 우선순위에서는 차후로 두겠습니다.
다른 것까지 다 하고 좀 더 고도화 해볼게요!

Copy link
Member

Choose a reason for hiding this comment

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

간단한 방법으로는 대부분의 크롬 버전 / 브라우저에서 지원될 테니깐 간단하게 현재 브라우저 버전에서는 지원하지 않는 기능입니다. 최신 버전의 Chrome을 사용해주세요. 정도면 UI공수없이 적절할 것 같네여 ㅋㅋ

});
};

useNaverMap(latitude, longitude);
Copy link
Contributor

Choose a reason for hiding this comment

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

useMarker로 위치 표시하는 것은 어떨까요? 축소하니까 위치 파악이 안 되는 것 같아서요!(물론 선택 사항입니닿ㅎㅎ)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이건... 저도 마커를 넣는게 더 좋다고 생각은 했지만, ui 디자인 상에서 마커가 없었기 때문에 일부러 넣지 않았습니다!

그리고 useMarker는 마커를 찍기 위해 상점 이름, 대표사진이 필요해요.(아마 마커를 표시하는데, 상점이름과 이름을 넣어주고 싶었던 거겠죠?)
만약 마커 표시를 하도록 시안이 바뀐다면 이 상세 페이지에서는 마커를 그냥 위치 기반으로 찍어주면 되기 때문에 useMarker에서 위치만 받았을 때의 로직을 재설계 해서 넣어두도록 하겠습니다.


return (
<section className={styles.container}>
<SectionHeader
title="지도"
description={formattedAddress}
button={{ content: 'URL 복사', onClick: () => {} }}
button={{ content: 'URL 복사', onClick: copyURL }}
Copy link
Member

Choose a reason for hiding this comment

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

요건 좀 익숙하지 않은 인터페이스네요.. ㄷㄷ
onButtonClick 정도의 인터페이스로 두거나, 아예 조합 컴포넌트로 바꾸는것도 좋아보이는데
사실 그렇게 중요한 부분은 아니니 마이너한 리뷰로 봐주세영

Copy link
Contributor Author

Choose a reason for hiding this comment

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

저도 조합을 좀 생각하기는 했는데(사실 지금도 조합이 좀 고프지만..)
이 정도 까지는 하나의 컴포넌트로 만들어도 괜찮겠다 생각이 들어서요.
더이상 디자인 이외에 확장되는 기능이 나오기 힘들다라고 봤거든요. 그렇다고 그렇게 복잡도가 높은 컴포넌트도 아니어서, 혹여 진짜 나중에 확장할 필요가 있다고 하면 조합으로 함 써보고 싶네요. 이 김에 컴파운드 패턴을 좀 다시 복기 해볼까..

/>
<div id="map" className={styles.map} />
</section>
Expand Down
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;
}
}
30 changes: 30 additions & 0 deletions src/pages/ShopDetail/components/ScrapButton/index.tsx
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);
Copy link
Member

Choose a reason for hiding this comment

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

scrapId를 받는 인터페이스도 특이한 것 같은데,
스크랩중인 여부를 불린값으로 반환하지 못하는 이유가 있나요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

스크랩중인 여부 라는 게 isPending 을 말하는 걸까요? 아니면 그 initialScrapId 를 number | null 대신 isScrap과 같은 props를 만들어 boolean 값으로 받을 수는 없을까 일까요?

초기에 음식점 상세 데이터를 불러오면 scrap 여부를 number | null 형식으로 주더라구요. 그래서 데이터 형식을 좀 일관화 하고 싶어서, 이걸 그대로 따라갔습니다.
그렇다고 boolean 여부를 생각 안해본 것은 아닌데, 이렇게 되면 button 에 data attribute를 사용해서 비제어 형식으로는 만들어 볼 수 있겠지만 코드량도 늘고 비제어 형식보다는 제어 형식으로 작성한게 깔끔하다고 느낌을 받아서 이렇게 구상을 했습니다~!

Copy link
Member

Choose a reason for hiding this comment

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

isScrap등의 불린값으로 써야한다고 생각했고,
이런 프로퍼티의 정보는 서버에서 갖고있어야 한다고 생각해요.
북마크의 id가 클라이언트에서 필요한 정보도 아닐 뿐더러, 저희가 필요한 정보는 해당 상점을 등록했는지 / 내가 어떤 상점들을 등록했는지 뿐이니깐 정보의 위치가 불편하다는 느낌이 드네영


return (
<button
type="button"
onClick={toggleScrap}
disabled={isPending}
Copy link
Contributor

Choose a reason for hiding this comment

The 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;
11 changes: 4 additions & 7 deletions src/pages/ShopDetail/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import { useQuery } from '@tanstack/react-query';
import AuthTopNavigation from 'components/Auth/AuthTopNavigation';
import { fetchShop } from 'api/shop';
import StarRatingPreview from 'components/StarRating/StarRatingPreview';
import { ReactComponent as BookMarkIcon } from 'assets/svg/shop/book-mark.svg';
import { ReactComponent as InfoIcon } from 'assets/svg/shop/info.svg';
import styles from './ShopDetail.module.scss';
import ImageCarousel from './components/ImageCarousel';
import FriendReviewList from './components/ReviewList/FriendReviewList';
import MyReviewList from './components/ReviewList/MyReviewList';
import ScrapButton from './components/ScrapButton';
import Map from './components/Map';
// import mock from './mock';

Expand All @@ -26,11 +26,11 @@ function ShopDetail() {
if (data) {
const {
// shopId,
// placeId,
// periods,
// scrap,
// openNow,
// category,
// placeId,
scrap,
name,
formattedAddress,
lat,
Expand Down Expand Up @@ -60,10 +60,7 @@ function ShopDetail() {
</div>
<div className={styles['detail-main__name']}>
<h1>{name}</h1>
<button type="button" onClick={() => {}}>
<BookMarkIcon />
<span>북마크 하기</span>
</button>
{placeId && <ScrapButton placeId={placeId} initialScrapId={scrap} />}
</div>
</div>

Expand Down
31 changes: 31 additions & 0 deletions src/utils/hooks/useScrap.ts
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;
Loading