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/#60] 메인 페이지 뉴스 조회 API 연결 #62

Merged
merged 6 commits into from
Nov 29, 2024

Conversation

imddoy
Copy link
Collaborator

@imddoy imddoy commented Nov 28, 2024

📌 관련 이슈번호

🎟️ PR 유형

어떤 변경 사항이 있나요?

  • 새 기능 추가
  • 버그 수정
  • CSS 등 사용자 UI 디자인 변경
  • 리팩토링

✅ Key Changes

이번 PR에서 작업한 내용을 간략히 설명해주세요

  1. get API 연결했습니다.

📢 To Reviewers

  • 파라미터 cursor 값을 쿼리키로 추가했습니다.
  • 이미지는 클라에서 더미로 관리하기로 했기 때문에 계속 반복해서 매칭해주었습니다.
  • 기존에 스켈레톤이 p에 값이 없으면 height가 없던 부분 해결했습니다.
  • isLoading이랑 기존 스켈레톤을 연결해주었습니다.

📸 스크린샷

image

🔗 참고 자료

@imddoy imddoy self-assigned this Nov 28, 2024
Copy link
Member

@gwagjiug gwagjiug left a comment

Choose a reason for hiding this comment

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

수고하셨어요~ 최고~ 궁금해서 여쭤본게 많은데 시간날 때 답변 부탁드려요 ㅎㅎ

@@ -2,7 +2,6 @@ import axios from 'axios';

export const instance = axios.create({
baseURL: import.meta.env.VITE_API_BASE_URL,
withCredentials: true,
Copy link
Member

Choose a reason for hiding this comment

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

디버깅의 흔적인가요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

로그인 토큰 넣을 때 필요한데, 이번 합세에서는 헤더에 넣는 요청들이 없더라구요! 그래서 지웠습니다...ㅎ

try {
const response: AxiosResponse<getNewsResponse> = await get('/news');
const response: AxiosResponse<getNewsResponse> = await get(
`/news${cursor ? `?cursor=${cursor}` : ''}`,
Copy link
Member

Choose a reason for hiding this comment

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

👍

queryFn: () => getNews(cursor),
staleTime: 1000 * 60 * 60,
gcTime: 1000 * 60 * 60 * 12,
retry: 3,
Copy link
Member

Choose a reason for hiding this comment

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

이 쿼리 세팅 값들은 그냥 보편적으로 사용되는 세팅값인가요? 아니면 고려하고 설정한게 있나요?? 궁금해서 여쭤보는 것...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

개발자가 쿼리마다 적절하게 지정해주는 것으로 알고 있습니다!!
저는 이렇게 지정해준 이유는, 맥도날드 홍보용 카드뉴스(?)는 사실 올라오는 주기가 그렇게 많지도 않고 한번 올라왔을 때 수정이 발생하지 않는다고 생각했습니다.
그래서 1시간 정도를 staletime으로 주어도 괜찮을 것 같다고 판단했고, staleTime보다 gcTime이 길게 설정되어야 하고, 적절한 시간을 고민하다가 12시간으로 설정해두었습니다!

@@ -48,6 +48,7 @@ export const TitleStyle = (isThreeLines: boolean) => (theme: Theme) => css`
color: ${theme.colors.gray500};
${theme.fonts.title07};
overflow: auto;
word-break: keep-all;
Copy link
Member

Choose a reason for hiding this comment

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

미친거니

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

왜그러니~~~~!

Copy link
Collaborator

@youtheyeon youtheyeon left a comment

Choose a reason for hiding this comment

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

LGTM!


color: transparent;
${theme.fonts.title07};
overflow: auto;
}

p::after {
content: '';
content: 'dd';
Copy link
Collaborator

Choose a reason for hiding this comment

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

디버깅의 흔적인가요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ㄷㄷ 야무지다

@imddoy imddoy merged commit d8162c4 into develop Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ Feat ] 뉴스 조회 API 연결
3 participants