-
Notifications
You must be signed in to change notification settings - Fork 2
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] 팔로우 기능 및 페이지 추가 #436
Conversation
b99aabd
to
ee5eaa4
Compare
팔로우 기능 제작하고 계셨군요! 👍 |
디자인 괜찮나요? 칭호, 레벨 같은걸 넣을까 고민중인데 흠.. 고칠 부분이 많진 않은데 예주님한테 물어볼 사항이 있어서 그 부분까지 반영되면 다시 알려드릴게요! |
6a5d7c6
to
a268d70
Compare
수정할게 있어 잠시 닫아놓겠습니다 |
const { handleFollow, followStatus } = useFollow(id); | ||
|
||
const followStatusText = (followStatus: boolean | undefined) => { | ||
switch (followStatus) { | ||
case true: | ||
return 'Unfollow'; | ||
case false: | ||
return 'Follow'; | ||
default: | ||
return 'Unfollow'; | ||
} | ||
}; | ||
|
||
return ( | ||
<FollowButtonLayout color={color} onClick={handleFollow}> | ||
<H3BoldText | ||
color={ | ||
color === 'absoluteBlack' | ||
? theme.colors.mono.absolute.white | ||
: theme.colors.mono.black | ||
} | ||
> | ||
{followStatusText(followStatus)} | ||
</H3BoldText> | ||
</FollowButtonLayout> |
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.
- handleFollow : useFollow
- followStatus : 지엽적인 로직
followStatusText, FollowButton : 뷰 이기 때문에 수동적인 컴포넌트로 분리
최대한 중복 로직은 통합하는 방식으로 수정하시면 좋을 것 같아요.
app/src/Profile/index.tsx
Outdated
let tabsToRender = null; | ||
|
||
if ( | ||
pathname.startsWith(ROUTES.PROFILE_FOLLOWERS_OF(login)) || | ||
pathname.startsWith(ROUTES.PROFILE_FOLLOWING_OF(login)) | ||
) { | ||
tabsToRender = ( | ||
<> | ||
<Tab | ||
selected={pathname.startsWith(ROUTES.PROFILE_FOLLOWERS_OF(login))} | ||
link={ROUTES.PROFILE_FOLLOWERS_OF(login)} | ||
> | ||
팔로워 | ||
</Tab> | ||
<Tab | ||
selected={pathname.startsWith(ROUTES.PROFILE_FOLLOWING_OF(login))} | ||
link={ROUTES.PROFILE_FOLLOWING_OF(login)} | ||
> | ||
팔로잉 | ||
</Tab> | ||
</> | ||
); | ||
} else { | ||
tabsToRender = ( | ||
<> | ||
<Tab | ||
selected={pathname.startsWith(ROUTES.PROFILE_GENERAL_OF(login))} | ||
link={ROUTES.PROFILE_GENERAL_OF(login)} | ||
> | ||
일반 | ||
</Tab> | ||
<Tab | ||
selected={pathname.startsWith( | ||
ROUTES.PROFILE_LOGTIME_AND_PROJECT_OF(login), | ||
)} | ||
link={ROUTES.PROFILE_LOGTIME_AND_PROJECT_OF(login)} | ||
> | ||
접속 · 과제 | ||
</Tab> | ||
<Tab | ||
selected={pathname.startsWith(ROUTES.PROFILE_EVAL_OF(login))} | ||
link={ROUTES.PROFILE_EVAL_OF(login)} | ||
> | ||
평가 | ||
</Tab> | ||
{login !== user.login && ( | ||
<Tab | ||
selected={pathname.startsWith(ROUTES.PROFILE_VERSUS_OF(login))} | ||
link={ROUTES.PROFILE_VERSUS_OF(login)} | ||
> | ||
나와 비교 | ||
</Tab> | ||
)} | ||
</> | ||
); | ||
} |
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.
팔로우 / 팔로잉 페이지로 이동했을 때 다시 일반 탭으로 이동하는 방법이 없어서 UX적으로 좋지 않은 것 같아요.
해결법
- 팔로우 / 팔로잉 탭 삭제
- 팔로우 / 팔로잉 탭 남기고, 뒤로가기 버튼
apollo client의 useQuery는 기본으로 cach-first 기능이 있었네요. no-cache로 변경했습니다. |
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.
뭔가 기능에 문제가 있을 것 같지는 않네요.
그냥 오랜만이라 주저리주저리 적어봤습니다. 편하게 스킵하실 건 스킵하시고 그냥 공부한다 생각하고 코멘트해주십쇼 😇😇
라고 말하려다가...
이건 UX에 좋지 않을 것 같... 아닙니다. 제가 디자인하고 수정할 여력은 안되어서... Approve 드리겠습니다. 하하하
2024-04-01.9.47.33.mov
const loading = loadingFollow || loadingUnfollow; | ||
const error = errorFollow || errorUnfollow; |
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.
무언가 사고의 과정이,
Follow
버튼을 누르면Unfollow
버튼으로 변경된다.- 따라서,
Follow
버튼을 연속으로 두 번 누르면 Follow Mutation이 로딩 중인데Unfollow
요청을 또 보내버릴 수도 있다. - 따라서, useHitFollow 훅을 만들고 handleHitFollow, handleHitUnfollow 함수를 바깥으로 꺼내 사용하는 방식으로 loading일 때 요청을 보내지 못하도록 막는다.
같은데 맞나요?
- loading 상태일 때 버튼을 disable 시켜야하는데, 이 코드를 보면 현재 그렇게 하지 못하고 있을 것 같은데 맞나요?
- React Query에서는 POST 요청 성공 시 해당 queryKey를 구독하던 모든 컴포넌트를 대상으로 상태를 다시 fetching 해오는 기능이 있는데요..!
Ref. https://pozafly.github.io/react-query/mutation-after-data-update/
저도 예전에 할 때는 적용해보진 않았지만 찾아보니 Apollo Client (React)에도 useMutation의 onSuccess 동작으로 invalidateQuery와 비슷한 기능이 있는 것 같아 링크 걸어둡니다.
Ref. https://www.apollographql.com/docs/react/data/mutations/
정확히 코드를 뜯어본 건 아니지만, 위에서 no-cache로 해결하신 문제도, 이 invalidateQuery 관련하여 찾아보시고 적용하면 훨씬 깔끔한 방식으로 해결할 수 있지 않을까라는 생각 (진짜 그냥 생각임. 망상일수도) 이 있습니다.
뭐, 제가 다시 들어갈 때쯤엔 아예 Next로 리팩토링을 해버리지 않을까 싶어서 기능만 잘 작동한다면 OK입니다.
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.
이 부분은 좀 고민중이던 부분인데 일단 지금 처리된 방법을 알려드리면
- FollowButton은 UI적으로 disable된 상태를 보여주지 않고 있어요. useHitFollow의 hitFollow로직에서 loading일 때 return을 하고 있기 때문에 요청은 보내지 않기 때문에 사용자에게 보여지는지의 차이만 있습니다. 버튼 자체가 안눌려야된다고 생각하시면 수정하겠습니다.
- invalidateQuery처럼 사용할 수 있는걸 알고 있지만 그렇게 처리하는게 개인적으로 좋은지 모르겠어서 사용하지 않았습니다. 지금 팔로워/팔로잉 페이지에서는 isFollowing의 리스트를 불러와서 컴포넌트들을 띄워주는데 invalidateQuery를 사용하게 되면 한 사람에 대한 followStatus를 불러오는게 아니라 리스트에 대한 api요청을 다시 하게 됩니다. 그래서 한 사람에 대한 follow상태를 바꾸는 hitFollow/hitUnfollow mutation이 성공하면 임의로 로컬에서 팔로우 상태를 바꿔줍니다. 단점은 이렇게 되면 이렇게 쓰면 리스트 자체를 다시 불러오지 않았기 때문에 캐싱된 상태를 사용하지 못하고 no-cache로 설정해놔서 팔로우 페이지를 들어올 때는 매번 리스트에 대한 api를 요청하게 됩니다. (한 줄 요약: 팔로우 리스트 api 요청할 때 팔로우 버튼을 누를 때 마다 요청할지 페이지에 들어올 때 요청할지 선택이 필요한데 페이지에 들어올 때 불러오는 것을 선택했습니다)
{followRowList.map((chunk, col) => ( | ||
<DashboardRow key={col}> | ||
{chunk.map((user) => ( | ||
<DashboardRowItem | ||
key={user.userPreview.id} | ||
rowSpan={1} | ||
colSpan={1} | ||
content={() => <FollowItem user={user} />} | ||
></DashboardRowItem> | ||
))} | ||
</DashboardRow> | ||
))} |
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.
ProfileFollowTabFollowList
, ProfileFollowTabFollowListItem
이런 식으로 하위 컴포넌트로 빼는 게 좋아보입니다.
뭐.. 예전의 저도 제가 지금 말하는 내용을 지키진 않았습니다. 그냥 오랜만에 보니까 예전 코드를 리팩토링하고 싶은 생각이 조금씩 드는군요.
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.
이거는 어떤 의미인지 제대로 이해 못했어요.
follow 리스트를 받아와서 대쉬보드 카드로 바로 띄워주고 있는데 리스트랑 아이템 하위 컴포넌트로 만든다는게 어떤 의미인가요?
아 이것도 관심사 분리하라는 뜻으로 이해할게요. 한번 리팩토링 해보겠습니다
생각해보니까 이미 followItem을 따로 만들어 쓰고 있네요. 이해 못했어요,,,ㅋㅋ
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.
그냥 쪼개면 좋겠다는 얘기였어요! 넘기셔도 됩니다
@yoopark 우선 PR 남겨주신 내용 invalidateQuery 관련 제외하고는 다 고쳤어요. 기존 대쉬보드는 Row랑 Content를 Frame에 넣어서 사용했는데
dev에 목데이터도 올라갔어요~ |
확인했습니다! 일단 배포해보고 버그 발생 시 수정해도 괜찮을 듯합니다. 수고 많으십니다 😄😄😄 |
Summary
팔로우 기능 및 페이지 추가
Describe your changes
추가
todo
Issue number and link