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

feat: 서랍장 YLS 연결 작업 #236

Merged
merged 11 commits into from
Jun 26, 2024
Merged

feat: 서랍장 YLS 연결 작업 #236

merged 11 commits into from
Jun 26, 2024

Conversation

seocylucky
Copy link
Member

@seocylucky seocylucky commented Jun 23, 2024

1️⃣ 어떤 작업을 했나요? (Summary)

서랍장 랭킹(메인) 화면 속 서비스를 클릭했을 때 yls 로그가 찍히도록 로깅시스템을 붙였습니다.

Jun-23-2024 21-21-50

기존 코드에 영향을 미치지 않는 변경사항

DetectClick에서 해당 서비스의 drawerServiceOwner을 제공하기 위해 useGetDetectProvider 훅을 만들어서 provider만 데이터로 따로 빼오도록 하였습니다.

버그(였던 것) 픽스

ranking 페이지의 BigDrawerCard에서 LogClick이 먹히지 않는 현상
-> onClick이 제대로 전달되지 못했던 것이 문제!!!! LogClick의 onClick을 최상위로 전달되게 하기 위해 BigDrawerCard와 BigDrawerCard의 하위 컴포넌트인 Card에 onClick 속성을 추가했습니다. 메인에 있는 BigDrawerCard가 아니면 해당 onClick 전달을 안하기 때문에 onClick 속성은 옵셔널로 설정하였습니다.

2️⃣ 궁금한 점🤔

첫번째

현재 테스트해보니까 localStorage의 timestamp는 실제로 찍힌 시간에 맞게 잘 나오는데 키바나에 나오는 로그는 시간이 이상하게 적용되는 것 같습니다. 확인해보니 시간이 9시간 후로 적용됩니다. UTC 시간으로 적용된거겠죠..?

로컬 스토리지의 timestamp

image

키바나의 timestamp

image

두번째

10번 로컬스토리지에 로그를 찍고 난 후 다음 로그를 찍을 때 yls-web 로컬스토리지가 빈값으로 시작되는 점
10번 로컬스토리지에 로그를 찍고 나서 다음 로그를 찍으면 키바나가 정상적으로 업데이트되고 timestamp는 첫번째 궁금한 점에서처럼 9시간 이른 시간인거 빼고 정상적으로 잘 나옵니다!

2024-06-24.2.19.37.mov

3️⃣ 추후 작업

리뷰 반영!

4️⃣ 체크리스트 (Checklist)

  • main 브랜치의 최신 코드를 pull 받았나요?

Copy link
Collaborator

@minai621 minai621 left a comment

Choose a reason for hiding this comment

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

체리 수고하셨습니당!


interface DetectClickProps {
product: ProductResult;
children: React.ReactElement;
Copy link
Collaborator

Choose a reason for hiding this comment

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

요기 ReactNode가 아니라 ReactElement로 타입 좁혀놓으신 이유가 있을까용?

Copy link
Member Author

@seocylucky seocylucky Jun 24, 2024

Choose a reason for hiding this comment

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

현재 DetectClick의 children은 원시타입은 존재하지 않고 함수형 컴포넌트로 반환된 커스텀 컴포넌트이기에 넓은 범위의 ReactNode보다는 ReactElement로 사용하는게 낫다 생각하여 사용하게 되었습니당

유연하게 가는게 더 나을까요?
보통 children 타입은 ReactNode를 사용한다곤 하지만 해당 DetectClick 사이에 원시타입이 들어갈 일이 없어서 허헣

link={`/drawer/services/${product.productNo}`}
title={product.productTitle}
body={product.productSubTitle}
bookmarkCount={product.count}
isBookmarked={product.isBookmarked}
bigImgSrc={product.introductionImage[0]}
smallImgSrc={product.mainImage}
onClick={() => {}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

optional인데 props에 빈 익명함수를 넣어두신 이유가 있을까요?

Copy link
Collaborator

@nijuy nijuy Jun 24, 2024

Choose a reason for hiding this comment

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

왜냐면......
안 넣으면 로깅이 안되기 때문입니다
로깅이 필요한 곳에만 빈 함수를 하나 넣어줘야 한대요

(Card의 경우 onClick이 CardContainer에 정의되어있는 상태여서 걍 빈 함수 주는거고 꼭 빈 함수를 넣어줘야 하는 건 아닐걸.........요 아마~~~ 근데 저도 다른 방법은 없을까 쫌 더 고민해보고 싶어요 🥺)

버그(였던 것) 픽스
ranking 페이지의 BigDrawerCard에서 LogClick이 먹히지 않는 현상
-> onClick이 제대로 전달되지 못했던 것이 문제!!!! LogClick의 onClick을 최상위로 전달되게 하기 위해 BigDrawerCard와 BigDrawerCard의 하위 컴포넌트인 Card에 onClick 속성을 추가했습니다. 메인에 있는 BigDrawerCard가 아니면 해당 onClick 전달을 안하기 때문에 onClick 속성은 옵셔널로 설정하였습니다.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

보리말대로 OnClick을 BigDrawerCard에 넣어야 LogClick이 최상위에 놓여있는 onClick을 가로채서 작동되는 방식이라 일단 저렇게 해놨는데 음 빈 값 들어간게 좀 보기 글킨하네요🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

아니면 일단 이렇게 해두고 리팩토링 이슈로 넘겨도 조아요~~! 시간이 많이 없을 거 같아서 ㅠ.ㅠ

.slice(0, 3)
.map((product) => (
newReleases.pages[0].slice(0, 3).map((product) => (
<DetectClick key={product.productNo} product={product}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

DetectClick 안에서는 useGetDetectProvider를 호출하고 해당 쿼리는 useSuspenseQuery인데 컴포넌트를 호출하는 부분에서는 Suspense로 감싸지 않고 있네요.
이러면 가장 최상단의 Suspense의 fallback에 잡힐텐데 Ranking 컴포넌트에는 Suspense가 없어서 router.tsx의 Suspense에 잡혀서 해당 컴포넌트에서의 promise가 페이지 전체를 blocking하게 될 것 같아요 ㅠㅠ..

Copy link
Member Author

@seocylucky seocylucky Jun 25, 2024

Choose a reason for hiding this comment

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

Ranking 컴포넌트 속 suspense가 DetectClick을 감싸야 한다는 걸 간과했네요. 당장 응급처치하겠습니다 useQuery로 적용시켰습니다! 날카롭다 엠제이👍🏻✨

@nijuy nijuy requested a review from minai621 June 25, 2024 15:50
@Hanna922
Copy link
Member

10번 로컬스토리지에 로그를 찍고 나서 다음 로그를 찍으면 키바나가 정상적으로 업데이트 된다는 건 빈 값으로 시작하는 문제가 발생하지 않는다는 의미이실까요?? timestamp는 저렇게 찍히는 게 정상입니당

@seocylucky
Copy link
Member Author

@Hanna922 영상대로 10번 찍고 다음 로그를 찍을 때 빈값인 상태 -> 이때 키바나에 들어가보면 전에 찍혔던 10개의 로그가 업데이트 됨! 으로 이해하시면 될 것 같아요! 영상대로 빈값이 바로 찍히는게 맞는건지 궁금해서 여쭤봤습니당

@Hanna922
Copy link
Member

Hanna922 commented Jun 26, 2024

10개 쌓인 후 로그 보낼 때 누르는 클릭, 즉 11번째 로그는 손실되는 문제가 있다는 걸로 이해했는데 요거 말씀하시는 거 맞죵?

@seocylucky
Copy link
Member Author

@Hanna922 맞습니다!!!!!!

@Hanna922
Copy link
Member

조아요👍🏻👍🏻 다음 버전에서 해결할게요!

@seocylucky
Copy link
Member Author

image

@seocylucky seocylucky merged commit af7b1d3 into develop Jun 26, 2024
@seocylucky seocylucky deleted the feat/#228-drawer-yls branch June 26, 2024 17:38
@seocylucky seocylucky mentioned this pull request Jun 26, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
drawer feat New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] 서랍장 YLS 연결 작업
4 participants