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

[PR] 공통 UI 컴포넌트 조정 #43

Merged
merged 5 commits into from
May 10, 2024
Merged

[PR] 공통 UI 컴포넌트 조정 #43

merged 5 commits into from
May 10, 2024

Conversation

gamjatan9
Copy link
Member

@gamjatan9 gamjatan9 commented May 10, 2024

📋 Checklist

  • 🔀 PR 제목의 형식을 잘 작성했나요? (e.g. feat: 유저 조회 기능 구현)
  • 🏷️ 라벨, 프로젝트는 등록했나요?
  • 🧹 코드 스멜은 해결했나요?

🧩 이슈 번호


🎨 완성 이미지

작업 이미지 image

👩‍💻 공유 포인트 및 논의 사항

  • 폰트 크기나 굵기를 styles/typography.ts 정의해둔 변수로 통일합시다
  • 버튼 크기 동일하게 적용함
  • 마이페이지 메인페이지 레이아웃과 통일시킴

gamjatan9 added 4 commits May 10, 2024 09:30
버튼 크기 통일, 폰트 크기 통일, 마이페이지 및 nav바 ui 수정
baseurl 수정, 유저관련  api 추가
@gamjatan9 gamjatan9 added documentation Improvements or additions to documentation enhancement New feature or request labels May 10, 2024
@gamjatan9 gamjatan9 requested a review from kangsinbeom May 10, 2024 00:40
@gamjatan9 gamjatan9 self-assigned this May 10, 2024
@gamjatan9 gamjatan9 linked an issue May 10, 2024 that may be closed by this pull request
background: ${({ background = colors.yellow }) => background};
color: ${({ color = 'white' }) => color};
font-size: 20px;
font-weight: 700;
${({ typography = 't1' }) => typographyMap[typography]}
Copy link
Contributor

Choose a reason for hiding this comment

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

button style 정의할 때 styles/button.ts 여기 ts 파일이 있는데 여기서 새롭게 정의를 하죠 typography 사용하는 거는 span이나 p tag에서만 쓰면 좋을 것 같습니다.


import styled from '@emotion/styled';
import { colors } from '../../styles/colorPalette';
import { typographyMap } from '@/styles/typography';

export const NavContainer = styled.div`
Copy link
Contributor

Choose a reason for hiding this comment

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

이거 styled component들 네이밍 컨벤션을 좀 정했으면 좋겠어요 Container로 저는 정의하는데 앞에 관련 컴포넌트 이름 붙일까요?

@@ -11,7 +11,7 @@ export const getAuthAxios = (accessToken?: string) => {
};

const authAxios = axios.create({
baseURL: 'http://211.206.94.24:9999',
baseURL: 'http://13.124.54.157:8080',
Copy link
Contributor

Choose a reason for hiding this comment

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

이부분 .env로 빼서 관리합시다.

@@ -0,0 +1,19 @@
import { useInfiniteQuery } from '@tanstack/react-query';
Copy link
Contributor

Choose a reason for hiding this comment

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

hooks file 안에는 2개 export하고 있는 것보다 하나의 기능만 넣어주세요

const response = await fetch(`${api}?page=${pageParam}&limit=${limit}`);
const data = await response.json();
// 다음 페이지 계산: 'done'이 false면 다음 페이지 번호, true면 undefined
const nextPage = data.pageInfo.done ? undefined : pageParam + 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

nexPage에 undefined보단 hasNext에 null or false와 원시 값을 사용하는 게 좋을 것 같습니다 undefined는 개발자가 정의하는 값이 아니라고 들었습니다.

Copy link
Collaborator

@j2an777 j2an777 left a comment

Choose a reason for hiding this comment

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

yeah

@gamjatan9 gamjatan9 merged commit e0bc91e into dev May 10, 2024
@gamjatan9 gamjatan9 deleted the develop/reshape branch May 10, 2024 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[develop/reshape] 공통 UI 컴포넌트 조정
3 participants