-
Notifications
You must be signed in to change notification settings - Fork 8
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
템플릿 좋아요 기능 구현 #686
템플릿 좋아요 기능 구현 #686
Conversation
frontend/src/api/like.ts
Outdated
url: `${LIKE_API_URL}/${templateId}`, | ||
}); | ||
|
||
if (response.status >= 400) { |
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.
기존 조건식이 200번대 응답을 에러로 판단하는 문제가 존재하여 이와 같은 조건식을 사용하였습니다.
typeof response === 'object' && response !== null && 'status' in response
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.
오.. 그런데 저의 이해로는 이미 customFetch
내부에서 !response.ok
일경우 CustomError 를 리턴해주고 있어서 이 부분이 없어도 될 것 같은데, 이 부분이 없으면 문제가 생겼었나요..?
@@ -54,7 +56,7 @@ export const getTemplateList = async ({ | |||
queryParams.append('tagIds', tagIds.toString()); | |||
} | |||
|
|||
const url = `${TEMPLATE_API_URL}?${queryParams.toString()}`; | |||
const url = `${TEMPLATE_API_URL}${memberId ? '/login' : ''}?${queryParams.toString()}`; |
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 명세에 따른 반영 (현재파일 이하 변경사항 동일)
#680
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.
'/login' 를 붙여야하는 히스토리를 켬미에게 들어서 참고하시라고 기술해둡니다.
[기본 전제]
- 로그인 여부와 member 정보는 쿠키로 받는다.
- 로그인 정보가 필요한경우, 백엔드에서는 요청을 인터셉터로 가로채서 확인하고, 로그인하지 않은 사용자라고 판별이 나면 예외처리가 된다!!
[원래 상황]
- 조회의 경우에는 로그인 여부에 상관없이 아무나 다 가능했기 때문에 인터셉터 X
[좋아요로 인해 변경된 상황]
- 좋아요 여부를 확인하기 위해 조회에서도 로그인 정보가 필요해졌다!
- 따라서 조회시에도 인터셉터를 하는데, 로그인하지 않은 사용자는 예외처리가 되어버린다. (기본 전제 참고)
[결론]
- 로그인 여부에 따라 다르게 요청을 처리해주어야 해서 임시적으로 'login' 을 붙이기로 했다.
- 추후 백엔드에서 인터셉터 로직을 변경할 예정이다.
frontend/src/assets/images/like.tsx
Outdated
const LikeIcon = ({ state, size }: Props) => { | ||
const fillColor = { | ||
like: '#FF5936', | ||
unlike: 'none', | ||
unClickable: '#393E46', | ||
}[state]; | ||
|
||
const strokeColor = { | ||
like: '#FF5936', | ||
unlike: '#393E46', | ||
unClickable: '#393E46', | ||
}[state]; |
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.
세 가지 타입의 'Like' 아이콘을 컴포넌트화 하여 구현하였습니다.
const LikeWidget = ({ likeCount, isLiked, clickable = false, onLikeWidgetClick }: Props) => ( | ||
<S.LikeButtonWidgetContainer isLiked={isLiked} clickable={clickable} onClick={onLikeWidgetClick}> | ||
<LikeIcon state={isLiked ? 'like' : clickable ? 'unlike' : 'unClickable'} size={14} /> | ||
<Text.Small color={theme.color.light.secondary_800}>{formatWithK(likeCount)}</Text.Small> |
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.
(단순 질문) 좋아요가 1000k 가 넘어가면 그 다음 단위도 변경해 줄 것인가요?? 함께 논의해보면 좋을 것 같아요!ㅎㅎ
(검색해본 트위터의 기준)
1,000 === 1K
1,000,000 === 1000k === 1M
1,000,000,000 ==== 1000m === 1B
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.
10만 미만을 가정하고 있습니다. 쉽게 확장할 수 있고, 좋아요 수 100만이 넘을 가능성은 지금 고려하지 않아도 될거라 생각했어용
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.
사실 그렇긴 하네요 🤣 사용자 수 100만 넘으면 바로 고려하러 가는 걸로!!ㅎㅎ
export const LikeButtonWidgetContainer = styled.div<{ isLiked: boolean; clickable: boolean }>` | ||
cursor: ${({ clickable }) => (clickable ? 'pointer' : 'default')}; | ||
|
||
display: flex; | ||
align-items: center; | ||
justify-content: space-between; | ||
|
||
width: 4.25rem; | ||
height: 1.5rem; | ||
padding: 0 0.5rem; | ||
|
||
border: 1px solid | ||
${({ isLiked }) => (isLiked ? theme.color.light.analogous_primary_400 : theme.color.light.secondary_800)}; | ||
border-radius: 1rem; | ||
`; |
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.
좋아요 수 10만 미만(소수자리 포함 3자리)을 가정한 레이아웃 디자인 입니다.
<Flex gap='0.75rem'> | ||
<Flex align='center' gap='0.25rem'> | ||
<PersonIcon width={14} /> | ||
<Text.Small color={theme.mode === 'dark' ? theme.color.dark.primary_300 : theme.color.light.primary_500}> | ||
{member.name} | ||
</Text.Small> | ||
</Flex> | ||
<Flex align='center' gap='0.25rem'> | ||
<ClockIcon width={14} /> | ||
<S.NoWrapTextWrapper> | ||
<Text.Small color={theme.color.light.primary_500}>{formatRelativeTime(modifiedAt)}</Text.Small> | ||
</S.NoWrapTextWrapper> | ||
</Flex> | ||
</Flex> | ||
<Flex align='center'> | ||
<LikeWidget likeCount={template.likeCount} isLiked={template.isLiked} /> |
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.
템플릿 카드의 레이아웃을 변경하였습니다. (#671)
<Heading.Large color={theme.mode === 'dark' ? theme.color.dark.white : theme.color.light.black}> | ||
{template.title} | ||
</Heading.Large> |
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.
템플릿 제목의 사이즈를 변경하였습니다. (Medium > Large)
<Flex gap='0.5rem' align='center'> | ||
<Flex align='center' gap='0.125rem'> | ||
<PersonIcon width={14} /> | ||
<Text.Small | ||
color={theme.mode === 'dark' ? theme.color.dark.primary_300 : theme.color.light.primary_500} | ||
> | ||
{template?.member?.name || ''} | ||
{template.member.name} |
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.
테스트를 위한 기존의 코드를 수정했습니다.
(Mock 데이터에서 template의 member가 없었을 때의 소스코드)
import { useMutation } from '@tanstack/react-query'; | ||
|
||
import { deleteLike } from '@/api'; | ||
|
||
export const useDislikeMutation = () => | ||
useMutation({ | ||
mutationFn: (templateId: number) => deleteLike({ templateId }), | ||
}); |
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.
현재 요청 실패 시, 사용자에게 알리고 있지 않습니다. (좋아요 낙관적업데이트만 롤백) (useLikeMutation 동일)
사용자에게 요청 실패를 전달할 방법에 대한 논의가 필요합니다.
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.
음... 좋아요 또는 좋아요 취소 요청 실패할 경우, 저는 2가지 정도 생각이 납니다!
-
토스트를 통해 요청 실패를 전달한다. (단, 토스트로 실패를 알려줄 경우 해당 토스트에서 어떤 템플릿에 대한 요청이 실패한건지 함께 알려주어야 한다고 생각해요...!)
-
사용자에게 알리지 않는다. (좋아요 라는 기능 자체가 실패했을 때 사용자에게 반드시 알려주어야 할 만큼 치명적인 작업이 아니라고 판단된다면 굳이 알리지 않아도 될 것 같습니다!)
export const useTemplateExploreQuery = ({ sort = DEFAULT_SORTING_OPTION.key, page = 1, size = PAGE_SIZE }: Props) => { | ||
const { | ||
memberInfo: { memberId }, | ||
} = useAuth(); | ||
|
||
return useQuery<TemplateListResponse, Error>({ | ||
queryKey: [QUERY_KEY.TEMPLATE_LIST, sort, page, size, memberId], | ||
queryFn: () => getTemplateExplore({ sort, page, size, memberId }), |
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 명세에 따른 반영 (useTemplateQuery 동일)
#680
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 가 수정되어 자잘하게 수정된 부분이 많았는데
월하가 코멘트 세심하게 미리 달아주신 덕분에 캐치를 잘 할 수 있었던 것 같습니다.
현재 네이밍과 같이 사소한 코멘트 몇개와 useLike
내부에서 사용하는 상태 및 toggle 함수에 대한 코멘트 남겨두었으니 확인 부탁드립니다!
frontend/src/api/like.ts
Outdated
url: `${LIKE_API_URL}/${templateId}`, | ||
}); | ||
|
||
if (response.status >= 400) { |
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.
오.. 그런데 저의 이해로는 이미 customFetch
내부에서 !response.ok
일경우 CustomError 를 리턴해주고 있어서 이 부분이 없어도 될 것 같은데, 이 부분이 없으면 문제가 생겼었나요..?
frontend/src/assets/images/like.tsx
Outdated
const fillColor = { | ||
like: '#FF5936', | ||
unlike: 'none', | ||
unClickable: '#393E46', | ||
}[state]; | ||
|
||
const strokeColor = { | ||
like: '#FF5936', | ||
unlike: '#393E46', | ||
unClickable: '#393E46', | ||
}[state]; |
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.
사용되는 컬러 theme 적용해주시면 좋을 것 같아요!!
const LikeWidget = ({ likeCount, isLiked, clickable = false, onLikeWidgetClick }: Props) => ( | ||
<S.LikeButtonWidgetContainer isLiked={isLiked} clickable={clickable} onClick={onLikeWidgetClick}> | ||
<LikeIcon state={isLiked ? 'like' : clickable ? 'unlike' : 'unClickable'} size={14} /> | ||
<Text.Small color={theme.color.light.secondary_800}>{formatWithK(likeCount)}</Text.Small> |
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.
(단순 질문) 좋아요가 1000k 가 넘어가면 그 다음 단위도 변경해 줄 것인가요?? 함께 논의해보면 좋을 것 같아요!ㅎㅎ
(검색해본 트위터의 기준)
1,000 === 1K
1,000,000 === 1000k === 1M
1,000,000,000 ==== 1000m === 1B
@@ -54,7 +56,7 @@ export const getTemplateList = async ({ | |||
queryParams.append('tagIds', tagIds.toString()); | |||
} | |||
|
|||
const url = `${TEMPLATE_API_URL}?${queryParams.toString()}`; | |||
const url = `${TEMPLATE_API_URL}${memberId ? '/login' : ''}?${queryParams.toString()}`; |
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.
'/login' 를 붙여야하는 히스토리를 켬미에게 들어서 참고하시라고 기술해둡니다.
[기본 전제]
- 로그인 여부와 member 정보는 쿠키로 받는다.
- 로그인 정보가 필요한경우, 백엔드에서는 요청을 인터셉터로 가로채서 확인하고, 로그인하지 않은 사용자라고 판별이 나면 예외처리가 된다!!
[원래 상황]
- 조회의 경우에는 로그인 여부에 상관없이 아무나 다 가능했기 때문에 인터셉터 X
[좋아요로 인해 변경된 상황]
- 좋아요 여부를 확인하기 위해 조회에서도 로그인 정보가 필요해졌다!
- 따라서 조회시에도 인터셉터를 하는데, 로그인하지 않은 사용자는 예외처리가 되어버린다. (기본 전제 참고)
[결론]
- 로그인 여부에 따라 다르게 요청을 처리해주어야 해서 임시적으로 'login' 을 붙이기로 했다.
- 추후 백엔드에서 인터셉터 로직을 변경할 예정이다.
import { useMutation } from '@tanstack/react-query'; | ||
|
||
import { deleteLike } from '@/api'; | ||
|
||
export const useDislikeMutation = () => | ||
useMutation({ | ||
mutationFn: (templateId: number) => deleteLike({ templateId }), | ||
}); |
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.
음... 좋아요 또는 좋아요 취소 요청 실패할 경우, 저는 2가지 정도 생각이 납니다!
-
토스트를 통해 요청 실패를 전달한다. (단, 토스트로 실패를 알려줄 경우 해당 토스트에서 어떤 템플릿에 대한 요청이 실패한건지 함께 알려주어야 한다고 생각해요...!)
-
사용자에게 알리지 않는다. (좋아요 라는 기능 자체가 실패했을 때 사용자에게 반드시 알려주어야 할 만큼 치명적인 작업이 아니라고 판단된다면 굳이 알리지 않아도 될 것 같습니다!)
export const useLike = ({ templateId, initialLikeCount, initialIsLiked }: UseLikeProps) => { | ||
const [likeCount, setLikeCount] = useState(initialLikeCount); | ||
const [isLiked, setIsLiked] = useState(initialIsLiked); | ||
const [isProcessing, setIsProcessing] = useState(false); |
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.
별도의 상태를 두기보다는 기존 Mutation 에서 제공하는 isPending
을 사용해보는건 어떨까요?
if (!isLiked) { | ||
setIsLiked(true); | ||
setLikeCount((prev) => prev + 1); | ||
await likeTemplate(templateId); | ||
} else { | ||
setIsLiked(false); | ||
setLikeCount((prev) => prev - 1); | ||
await dislikeTemplate(templateId); | ||
} |
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.
저희 컨벤션대로 얼리 리턴을 활용해보면 어떨까요?
|
||
import { useDislikeMutation, useLikeMutation } from '@/queries/likes'; | ||
|
||
interface UseLikeProps { |
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.
저는 export 하지 않는 경우에는 그냥 Props
로 사용하고있는데요, 통일성을 위해 Props
로 통일하거나 UseLikeProps
처럼 이름으로 통일하거나 하나로 통일해보면 좋을 것 같네요! 별것 아니지만 이런 사소한 부분도 통일성이 지켜지면 좋을 것 같아서 이것도 함께 논의해봅시다!ㅎㅎ
if (isProcessing) { | ||
return; | ||
} |
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.
현재 toggleLike
함수를 보면 하나의 요청이 모두 처리될 때까지 다른 요청을 막고 있는 것 같아요!
만약 좋아요를 빠르게 타닥 하고 누르면 좋아요 요청 뒤에 좋아요 요청 취소 요청이 날아가거나 애초에 처음 보낸 좋아요 요청을 취소해야할 것 같은데,
지금 코드상으로는 좋아요를 빠르게 타닥 하고 누르면 좋아요 처리만 될 것 같아요..!
이러한 부분에 대한 처리가 필요해보입니다!!
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.
헤인과 겹치는 코멘트가 많네요. 추가로는 현재 좋아요 UI가 아래 그림처럼 되어있는데, 한자리 일때 꽤 비어보여서 center에 두거나 width를 줄일지 논의해보면 좋겠습니다.
두번째로 MSW로 확인을 해보고자 했는데, 다른 handler에도 '/login'을 붙여야 동작을 하네요. 이것도 해당 PR에서 변경사항이라면 반영해야할까? 싶었습니다.
마지막으루 로컬에서 확인했을 때 템플릿 수정, 삭제 버튼 UI 변경이 확인되지 않아서, UI 변경은 현재는 스토리북을 쓰고 있지 않으니 스크린샷으로 공유하는 것도 좋을 것 같습니당~
짧은 기간인데 고생했어요 월ha~

import { useAuth } from '../../hooks/authentication/useAuth'; | ||
|
||
export const useTemplateQuery = (id: number): UseQueryResult<Template, Error> => { | ||
const { | ||
memberInfo: { memberId }, | ||
} = useAuth(); | ||
|
||
return useQuery<Template, Error>({ | ||
queryKey: [QUERY_KEY.TEMPLATE, id, memberId], | ||
queryFn: () => getTemplate({ id, memberId }), |
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.
백엔드에서 interceptor 로직을 어떻게 바꾼다고는 아직 듣지 못했는데,
당장 프론트에서는 memberId가 요청 URI에 필요할 때마다 query 훅에서 전역 변수를 불러오는 것보다 추후 apiClient 같은 방식으로 한단계 추상화해도 좋겠다는 생각이 드네요.
그럼 spec 파일들에서도 AuthProvider를 wrap 해주지 않아도 될 것 같구요!
지금은 해당 구현만으로도 충분해보입니다!👍
<PersonIcon width={14} /> | ||
<Text.Small color={theme.mode === 'dark' ? theme.color.dark.primary_300 : theme.color.light.primary_500}> | ||
{member.name} | ||
</Text.Small> | ||
</Flex> | ||
<Flex align='center' gap='0.25rem'> | ||
<ClockIcon width={14} /> |
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.
요 icon들 14로 거의 쓸거 같다면 기본값을 14로 해도 좋을 것 같네요!
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.
onLikeButtonClick: () => void; | ||
} | ||
|
||
const LikeCounter = ({ likesCount, isLiked, onLikeButtonClick }: Props) => ( |
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.
여기 이름이 LikeCounter
인것은 의도하신걸까요..? 파일 이름과 동일하게 LikeButton
으로 바꿔보면 어떨까요??
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.
default export라서 import하는 이름만 변경되고 원본의 이름이 변경되지 않았었네요!
주의해야겠어용 감사합니다 (- -)(_ _)
@@ -41,15 +34,12 @@ export const useLike = ({ templateId, initialLikesCount, initialIsLiked }: UseLi | |||
} catch (error) { | |||
setLikesCount((prev) => prev + (isLiked ? -1 : 1)); |
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.
아주 사소한데 아래처럼 여기도 initialLikesCount
로 초기화해줘도 좋을 것 같네요!!
setLikesCount(initialLikesCount + 1 - +initialIsLiked); | ||
await likeTemplate(templateId); | ||
} else { | ||
setIsLiked(false); | ||
setLikesCount((prev) => prev - 1); | ||
setLikesCount(initialLikesCount - 1 + +!initialIsLiked); |
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.
혹시 이 부분은 그냥 이전처럼 아래와 같이 업데이트 해 주는 것이 더 안전하고 명시적이라고 생각하는데 월하 생각은 어떤가요...?
setLikesCount((prev) => prev + 1);
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.
지금과 같이 변경한 이유는 like와 dislike의 요청 순서가 뒤섞였을 경우에도 정확한 값을 보장할 수 있기 때문입니다!
기존 방법이 명시적이라는 의견에 동의하지만, 안전성 측면에서는 0에서 싫어요가 두 번 카운트되어 -1(클라리언트)이 될 가능성을 고려하였습니당 자바스크립트에서 비동기 요청과 응답의 순서를 보장하지 않기 때문입니다!
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.
자잘한 코멘트들이 계속 있었는데, 단기간에 고생많으셨습니다! 👏👏
좋아요 버튼 디자인 및 색상에 대해서는 QA 기간에 백엔드 팀원들과 다함께 상의해봐요!
⚡️ 관련 이슈
#668
📍주요 변경 사항