-
Notifications
You must be signed in to change notification settings - Fork 1
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: 2차, 톡앤픽 플레이스 페이지 api 연결 및 컴포넌트 수정 #166
Conversation
}, | ||
}); | ||
|
||
export const barStyling = (varPercentage: number) => keyframes` |
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.
keyframe을 사용해본적이 없었는데 이번 기회에 아름님이 구현하신 코드 보고 한 번 또 배워갑니다 너무 너무 수고많으셨습니당
@@ -14,14 +14,14 @@ export type TalkPickDetail = { | |||
myBookmark: boolean; | |||
votedOption: 'A' | 'B' | null; | |||
writer: string; | |||
lastModifiedAt: string; | |||
createdAt: string; | |||
isUpdated: boolean; | |||
}; | |||
|
|||
export type TalkPickSummary = { |
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.
type 파일을 따로 만들 때의 기준이 궁금합니다! 일반적으로 컴포넌트 내에서 직접 사용하는 것과 이 폴더 내에 작성한 type을 구분하는 기준은 api의 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.
네! 현재 api 연결하여 받아오는 데이터의 type들을 types 폴더 내에 정의하여 사용하고 있습니다!! 😊
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 관련 type들은 모두 폴더로 이동시켜야겠네용ㅎㅎ
empty: true, | ||
}; | ||
useEffect(() => { | ||
window.scrollTo(0, 0); |
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.
window.scrollTo로 자동 스크롤 로직 구현하신거 아주 좋은 생각인 것 같습니다ㅎㅎ 추가적으로 스크롤 위치가 최상단 (0,0)이 아닌 TalkPickListSection
상단 부분으로 이동시키는건 어떤지 여쭙고싶습니다! 유저 입장에서 최상단으로 올라가면 어쨋든 리스트 부분으로 추가적인 스크롤이 필요하다고 생각합니당🧐
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.
useRef로 ref 연결하는 방법으로 시도해보았는데, 페이지가 바뀔 때마다 데이터를 새로 받아와야 해서 데이터가 렌더링이 된 이후 스크롤 이동이 가능하여 query hook에 isLoading을 추가해야 하더라구요..!
useEffect문을 사용할 경우 isLoading 값이 바뀌자마자 스크롤이 작동하여 페이지를 열자마자 TalkPickListSection으로 스크롤이 이동되는 문제가 발생합니다..🥲
이 부분은 제가 좀 더 공부해보고 추후에 이슈 파서 진행해보겠습니다!!! 🥹🙇♀️
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.
혹시 documnet.querySelector()
을 사용해보시는건 어떨까요?! 어제 scrollTo에 관하여 저도 검색해보다가 위치를 querySelector로 찾아와서 scrollTo로 이동시켜주는 로직을 보았거든요! 도움이 될까 싶어 링크 첨부하겠습니당😚
스크롤을 이동시키는 주문, Window.scrollTo()
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 연결 및 컴포넌트 수정하시느라 수고많으셨습니다👏 voteBar 결국 구현된거 너무 감사드려요🙏!!
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 및 api 연동 고생 많으셨습니다! 스토리북에서 애니메이션도 잘 작동하는 거 같아요(너무 이뻐요 ㅎㅎ)
쓰다보니 리뷰를 많이 남기게 되었네요.. 지금 이슈 발생한 부분에 코멘트 달아놨으니 확인 부탁드립니다. 고생 많으셨습니다!!
@@ -25,6 +25,7 @@ const ToggleGroup = ({ items, selectedValue, onClick }: ToggleGroupProps) => ( | |||
items.map((item, idx) => ( | |||
<button | |||
type="button" | |||
key={item.value} |
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.
mapping 시 key 를 item의 값으로 처리하는 로직 확인했습니다 😄
export const rotate = keyframes({ | ||
'100%': { | ||
transform: 'rotate(360deg)', | ||
}, | ||
}); |
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.
svg 파일을 rotate 애니메이션을 통해 360도 회전 시키는 방식이군요..! 360deg 와 같이 양수로 지정해주면 시계 방향, -360 와 같이 음수로 지정해주면 반시계 방향으로 회전되는 것을 이번에 알게 되었네요 👍
export const barStyling = (varPercentage: number) => keyframes` | ||
0% { | ||
width: 0%; | ||
} | ||
60% { | ||
width: 100%; | ||
} | ||
100% { | ||
width: ${varPercentage}%; | ||
} | ||
`; |
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%로 채워졌다가 다시 varPercentage에 해당하는 값으로 바가 표현되는거군요! storybook에서도 잘 작동하네요 고생하셨습니다 😄 😄
@@ -1,7 +1,7 @@ | |||
export type Pageable = { | |||
page: number; | |||
size?: number; | |||
sort?: string[]; | |||
sort?: string; |
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.
페이지네이션 변동 사항 숙지했습니다. 해당 사항 반영해서 페이지네이션 fix 하도록 하겠습니다 ㅎㅎ
votesStyle, | ||
neutralBarStyle, | ||
} from './VoteBar.style'; | ||
import * as S from './VoteBar.style'; |
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 하는 방식으로 저도 사용하는데 기존보다 코드가 깔끔해진거 같네요 ㅎㅎ
const TodayTalkPickBanner = ({ talkPick }: TodayTalkPickBannerProps) => { | ||
const navigate = useNavigate(); | ||
|
||
const onClickBanner = () => { | ||
navigate('/todaytalkpick', { state: { talkPickId: talkPick?.id } }); | ||
}; |
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.
molecules/CategoryBox 에서는 link를 쓰셨는데, 해당 컴포넌트에는 navigate가 사용되어서 각각 어떤 case 일 떄 사용을 의도하신건지 궁금합니다 ㅎㅎ
+) 직관적으로 보면 단순한 이동에는 link, id나 추가적인 params를 받아서 동적으로 이동해야하는 경우 navigate를 사용하신거 같다는 생각이 드는데 맞나욥??
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.
단순한 페이지 이동에는 Link를 사용하고, 몇 초 후 페이지 이동, 조건에 맞을 경우 페이지 이동 등의 특별한 로직과 연관된 경우에 navigate를 사용하고 있습니다!! 👍
@@ -13,15 +13,15 @@ import { | |||
} from './TopBanner.style'; | |||
|
|||
export interface TopBannerProps { | |||
todayTalkPick: TodayTalkPick; | |||
todayTalkPick: TodayTalkPick | undefined; |
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.
todayTalkPick? : TodayTalkPick 이 아니라 없는 case에는 undefiend로 선언을 해줘야하는군요... ⭐
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.
원준님의 코멘트 보고 한번 찾아봤는데, 받아오는 데이터가 없어도 잘 동작해야 한다면 ?:
를, 데이터가 제대로 들어올 때와 undefined일 때의 처리가 다르다면 | undefined
를 사용하는게 좋다고 하네요!!! 둘다 사용 가능하지만 ?:
를 사용하는 쪽이 더 적합하다고 판단되어 코드 수정하려 합니다 코멘트 주셔서 감사합니다 🙇♀️🫢
export interface TalkPickListProps { | ||
talkPickList: TalkPickListPagination; | ||
talkPickList: TalkPickListPagination | undefined; | ||
toggleItem: ToggleGroupItem[]; | ||
selectedValue: string; | ||
setToggleValue: React.Dispatch<React.SetStateAction<string>>; | ||
selectedPage: number; | ||
handlePageChange: React.Dispatch<React.SetStateAction<number>>; | ||
} |
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 연동이 이루어 지는 페이지에서는 types 에 props를 선언해주고 하위 organisms, molecules, atoms 에는 각각의 type을 내부에 지정해주는 걸까요??
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.
구현하면서 해당 props 관련하여 고민을 많이 했었는데요..! 하위 컴포넌트를 미리 구현해두고 페이지 파일 내에서 데이터를 가져와 넘겨주다보니, 하위 컴포넌트 내에서 데이터 제어 (인기순/최신순 토글 등)가 필요한 경우 함수 연결이 애매하더라구요..!
아토믹 디자인 구조 상 페이지에서 데이터 연결이 이루어지기도 하고, query 함수가 연결된 컴포넌트는 스토리북에서의 확인이 어렵다고 알고있어 해당 페이지에 하위 컴포넌트 내에 필요한 값들 및 함수를 선언해주고 넘겨주는 방식을 선택했습니다 😶
TALKPICK: (talkPickId: Id) => `/talks/${talkPickId}`, | ||
CREATE_TALKPICK: '/talks', | ||
TALKPICK_LIST: '/talks', | ||
TODAY_TALKPICK: '/talks/today', | ||
BEST_TALKPICK: '/talks/best', |
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.
/talks 와 같이 공통적으로 선언되는 엔드포인트의 경우 baseUrl로 먼저 선언해주는게 더 깔끔할 거 같습니다!
useEffect(() => { | ||
window.scrollTo(0, 0); | ||
}, [selectedPage]); | ||
|
||
useEffect(() => { | ||
setSelectedPage(1); | ||
}, [selectedValue]); |
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.
해당 부분에 관한 의견은 아름님과 비슷합니다! 웹사이트 몇군데에서 실제로 동작을 해봤는데, 보통은 맨 위로 가는 로직이 일반적이더라구요. 다만, 저희 페이지의 경우 최상단으로 이동 시 TalkPickListSection
부분이 거의 가려지기 때문에 TalkPickListSection
위치로 이동하는 로직이 더 사용자 입장에서 편리해 보인다고 생각합니다.
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.
저도 해당 부분 검색해봤는데, useRef를 사용하게되면 query hook에 isLoading를 추가하거나, useEffect의 의존성 배열에 isLoading의 boolean 값을 추가해야하더라구요!
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.
useEffect문을 사용할 경우 isLoading 값이 바뀌자마자 스크롤이 작동하는 문제의 경우, TalkPickListSection 컴포넌트 내부에 useEffect를 사용하여 렌더링 시점에 스크롤 제한을 설정하는 방법이 있는거 같습니다 😄
추가로 인위적으로 setTimeout를 거는 방법이 있는거 같은데 이 방식은 사용자 경험을 의도적으로 저해하는 거 같아 추천드리지는 않습니닷!
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.
의견 감사합니다!! 아무래도 페이지, 인기순/최신순이 바뀔 때 데이터를 새로 받아오다보니 톡픽 리스트로의 이동을 구현하려면 isLoading 추가가 필수적인 것 같아요,, 🥹
💡 작업 내용
💡 자세한 설명
✅ TextModal
smallText
를 추가하여 크기가 작은 텍스트도 추가할 수 있도록 수정했습니다! 스토리북에 추가해두었으니 확인 부탁드립니다.✅ SummaryBox
✅ VoteBar
✅ TodayTalkPickBanner
talkPick?.id
로 넘겨지게 되어있습니다.)✅ TalkPickPlacePage
📗 참고 자료 (선택)
📢 리뷰 요구 사항 (선택)
🚩 후속 작업 (선택)
✅ 셀프 체크리스트
closes #163