-
Notifications
You must be signed in to change notification settings - Fork 6
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
[FE] refactor/#450 모아보기 기능 비회원도 가능하도록 수정 #516
Changes from all commits
7924374
85528fb
eb57e86
0cbb0dd
b30ce94
a1c7eb7
5003eda
bbfda89
d00ac55
07c91b2
a5d9a9f
f75aa7f
930261c
1173dc0
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 |
---|---|---|
|
@@ -8,6 +8,7 @@ import { Suspense, lazy, useContext, useEffect } from 'react'; | |
import { MarkerContext } from '../context/MarkerContext'; | ||
import TopicCardContainerSkeleton from '../components/Skeletons/TopicListSkeleton'; | ||
import { setFullScreenResponsive } from '../constants/responsive'; | ||
import { SeeTogetherContext } from '../context/SeeTogetherContext'; | ||
import SearchBar from '../components/SearchBar/SearchBar'; | ||
|
||
const TopicListContainer = lazy( | ||
|
@@ -18,13 +19,21 @@ const Home = () => { | |
const { routingHandlers } = useNavigator(); | ||
const { goToPopularTopics, goToLatestTopics, goToNearByMeTopics } = | ||
routingHandlers; | ||
|
||
const { seeTogetherTopics, setSeeTogetherTopics } = | ||
useContext(SeeTogetherContext); | ||
const { markers, removeMarkers, removeInfowindows } = | ||
useContext(MarkerContext); | ||
const accessToken = localStorage.getItem('userToken'); | ||
|
||
useSetLayoutWidth(FULLSCREEN); | ||
useSetNavbarHighlight('home'); | ||
|
||
useEffect(() => { | ||
if (accessToken === null && seeTogetherTopics?.length !== 0) { | ||
setSeeTogetherTopics([]); | ||
} | ||
}, []); | ||
|
||
Comment on lines
+31
to
+36
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. 요 로직이 모아보기 페이지에도 있어야할 것 같네요. 제가 원래 생각했던 것은 비회원에게는 모아보기가 일회성 장바구니처럼 기능을 제공하는 쪽이었습니다. 그래서 모아보기 리스트를 초기화하면서 로그인을 하면 모아보기 리스트가 유지돼요! 이런식의 토스트를 남겨둘 생각이었어요. (로그인 유도도 좀 할겸?) 그래도 홈에도 여전히 이 로직이 필요합니다. 왜그러냐면 비회원이 접속할 수 있는 페이지가 홈이랑 모아보기 둘 뿐이면 홈에는 필요가 없지만, 안타깝게도 저희에겐 askLogin 페이지도 있기 때문입니다 ㅋㅋㅋㅋ 그래서 홈 -> 모아보기 담고 -> 즐겨찾기(애스크 로그인으로 전환) -> 홈 이렇게 이동하면 모아보기 리스트는 그대로지만 UI는 초기화 되어있겠죠. 모아보기 페이지에서 초기화를 안해주면 결정적으로 topicInfo 의 모아보기 버튼과 UI가 차이가 나는 결정적인 문제가 있기도합니다. 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. 세인! 이 부분은 저희 금요일에 만나서 한번 더 얘기보는게 어떨까요?! 제가 잘 이해를 못하기도 했고 실제로 만나 화면을 보면서 말하면 금방 이해할 수 있을 것 같아서요!! 일단 이 전까지 코멘트는 모두 반영하였습니다! 이 부분 만나서 화면으로 보고 바로 반영해봐요!!! |
||
useEffect(() => { | ||
if (markers && markers.length > 0) { | ||
removeMarkers(); | ||
|
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.
이전에 전화로 전달해주셨을 때 그대로 써도 문제없을 것 같았는데 AddSeeTogether onChangeIsInAtlas 함수안에
요 부분 때문에 바꿔야만 하는군요 ㅋㅋㅋㅋ 좋습니다. 👍👍