Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

4-3 [FE] [메인 - 인기 검색어] 1~10위까지의 인기검색어 데이터를 1초간격으로 슬라이딩 방식으로 보여준다. #23

Merged
merged 8 commits into from
Nov 17, 2022

Conversation

Palwol
Copy link
Collaborator

@Palwol Palwol commented Nov 16, 2022

개요

인기검색어 슬라이딩 애니메이션 구현

스크린샷

인기검색어

작업사항

  • 인기검색어를 1위~10위까지 슬라이딩 애니메이션으로 보여주도록 구현했습니다.
  • dev server api를 연결하여 redis에 캐시된 인기검색어 데이터를 받아옵니다. CORS 이슈로 임시로 proxy를 설정했습니다.
  • 로고 파일을 svg로 변경하고, favicon과 페이지 title을 수정했습니다.
  • hooks의 dependency를 놓치지 않기 위해 react-hooks/exhaustive-deps lint rule을 추가했습니다.

리뷰 요청사항

  • 인기검색어 슬라이딩 애니메이션을 무한루프로 구현하기 위해 DAN의 useInterval hook을 사용했습니다. 로직에 문제가 없는지, 혹은 더 좋은 방법이 있을지 조언 부탁드립니다. (구현 과정 이슈 작성 예정입니다.)

  • 인기검색어 데이터 fetch시 예외처리를 우선 간단하게 에러 메시지를 나타내는 것으로 구현해두었습니다. 생각해둔 방법으로는

    1. 곧바로 에러 페이지로 라우팅한다.
    2. 일정 시간 이후 재요청을 보낸다. (재요청 보낼때까지 로딩스피너를 보여줌)
    3. 인기검색어 컴포넌트만 에러 표시를 한다. (인기검색어만 사용 못하는거고, 다른 기능은 사용 가능)

    이정도입니다. 선호하시는 방식이 있는지, 혹은 더 좋은 방법이 있을지 조언 부탁드립니다^^ (to. 예윤)

import styled from 'styled-components';
import { SLIDE_DELAY, TRANSITION_SETTING, TRANSITION_TIME } from '../../../constants/ranking';

Choose a reason for hiding this comment

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

공통으로 계속 쓰이는 constanst 가 아니고 특정 컴포넌트 만 사용되면 컴포넌트 레벨로 가져가도 될 것 같네요.
이 컴포넌트는 components 레벨로 옮기는 것이 좋을 것 같아요.

Comment on lines 49 to 53
{newRankingData.map((data, index) => (
<SlideItem key={`${index}${data.keyword}`}>
<span>{index === dataSize - 1 ? 1 : index + 1}</span>
<span>{data.keyword}</span>
</SlideItem>

Choose a reason for hiding this comment

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

만약 API 호출이 안되거나, 오류가 있을 때 사용자에게 어떻게 보여주나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

상위 컴포넌트 (KeywordRanking)에서 API를 호출하는데, 우선 오류가 있을 시 RankingSlide 컴포넌트를 보여주지 않고 에러 메시지를 보여주도록 처리해두었습니다. 더 자세한 예외처리로 보완할 예정입니다.

Copy link
Member

@leesungbin leesungbin left a comment

Choose a reason for hiding this comment

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

코드 보느라 시간가는줄 몰랐습니다.

const RankingSlide = ({ rankingData }: IRankingSlideProps) => {
const [keywordIndex, setKeywordIndex] = useState(0);
const [transition, setTransition] = useState('');
const newRankingData = [...rankingData.slice(0, 10), rankingData[0]];
Copy link
Member

Choose a reason for hiding this comment

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

와우... 아름답네요.
api에서 제공되는 rankingData가 10개 이상일 수도 있음이 고려된 것 같은데, 너무 좋네요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

실제로 10개 이상이 왔기 때문에 고려할 수 있었습니다.^^

Copy link
Collaborator

Choose a reason for hiding this comment

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

ㅎㅎㅎㅎ..

if (keywordIndex === dataSize - 2) {
replaceSlide();
}
};
Copy link
Member

Choose a reason for hiding this comment

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

useInterval을 쓰지 않고 비슷하게 하는 방법을 제시해봅니다.

const replaceSlide = () => {
    setTransition('');
    setKeywordIndex(0);
};

const moveSlide = () => {
    return new Promise((resolve) => {
        setTimeout(() => {
            setTransition(TRANSITION_SETTING);
            setKeywordIndex((prev) => prev + 1);
            resolve('');
        }, SLIDE_DELAY);
    });
};
useEffect(() => {
    moveSlide().then(() => {
        if (keywordIndex === dataSize - 1) {
            replaceSlide();
        }
    });
}, [keywordIndex]);

위에 코드는 6위에서 1위로 돌아왔을 때에는, 2위로 넘어가기까지 SLIDE_DELAY*2 만큼의 시간이 걸립니당.
keywordIndex === dataSize - 2 가 아닌 -1 을 한다는 점에서 머리가 조금 자유로운(?) 것 같습니다. ㅎㅎ..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오오 Promise를 사용하는건 생각 못했는데 역시 성빈님이십니다! 👍

<g fill="#2D2D29FF" stroke="#2D2D29FF">
<path d="M 37.297 135.408 C 40.119 138.008 44.594 139.739 45.682 138.651 C 46.006 138.327 45.438 136.811 44.420 135.281 C 42.896 132.992 42.506 132.809 42.211 134.250 C 41.767 136.421 40.179 136.471 36.973 134.415 C 34.796 133.020 34.834 133.138 37.297 135.408 " />
</g>
</svg>
Copy link
Member

Choose a reason for hiding this comment

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

와우,.. svg로... 어떻게.. 하신거죠? ㅋㅋ...
사과드립니다.🙇‍♂️

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

갓성빈님의 로고를 변환한거밖에 없습니다,,ㅋㅋ

@yeynii
Copy link
Member

yeynii commented Nov 16, 2022

저는 일단 에러 발생시 재요청 (max 5회) 로 하고있는데, 재요청 횟수가 max를 넘어갈 때를 따로 정의하진 않았네용
저라면 2번 + 재요청 한도 초과시 3번으로 처리할 듯 합니다

Copy link
Collaborator

@JunYupK JunYupK left a comment

Choose a reason for hiding this comment

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

좋습니다. 예외처리가 인상깊네요

setRankingData(data);
try {
const response = await fetch('/keyword-ranking');
if (!response.ok) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

FE 쪽 예외처리 너무 좋습니다. BE도 신경쓰도록 하겠습니다.

const RankingSlide = ({ rankingData }: IRankingSlideProps) => {
const [keywordIndex, setKeywordIndex] = useState(0);
const [transition, setTransition] = useState('');
const newRankingData = [...rankingData.slice(0, 10), rankingData[0]];
Copy link
Collaborator

Choose a reason for hiding this comment

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

ㅎㅎㅎㅎ..

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FE] 인기검색어 슬라이딩 애니메이션이 루프가 되지 않는 이슈
5 participants