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

[이진우] sprint11 #170

Open
wants to merge 17 commits into
base: next-이진우
Choose a base branch
from

Conversation

ajantang
Copy link
Collaborator

@ajantang ajantang commented Nov 11, 2024

요구사항

기본

  • Github에 위클리 미션 PR을 만들어 주세요.
  • React 및 Express를 사용해 진행합니다.
  • TypeScript를 활용해 프로젝트의 필요한 곳에 타입을 명시해 주세요.
  • any 타입의 사용은 최소화해 주세요.
  • 복잡한 객체 구조나 배열 구조를 가진 변수에 인터페이스 또는 타입 별칭을 사용하세요.
  • Union, Intersection, Generics 등 고급 타입을 적극적으로 사용해 주세요.
  • 타입 별칭 또는 유틸리티 타입을 사용해 타입 복잡성을 줄여주세요.
  • 타입스크립트 컴파일러가 에러 없이 정상적으로 작동해야 합니다.

프론트엔드

  • 기존 React(혹은 Next) 프로젝트를 타입스크립트 프로젝트로 마이그레이션 해주세요.
  • TypeScript를 활용해 프로젝트의 필요한 곳에 타입을 명시해 주세요.

주요 변경사항

  • 초기 업로드

스크린샷

image

멘토에게

  • 현재 TS 적용 중입니다
  • 기존의 코드에 (미완성 || 만족 못하는) 부분들이 많아서 수정 중입니다

BE를 sprint10 미션용으로 수정

참고 : user, product 등 다른 api도 수정 예정
article -> post, title -> name

참고 : ts 변환 과정이라 수정 부분 테스트 추후 진행 예정
@ajantang ajantang added the 진행 중 🏃 아직 스프린트 미션 제출일이 아닙니다. 새로 커밋된 내용에 대해 코드리뷰 해주세요! label Nov 11, 2024
BE 변경 사항에 맞춰 수정
@chash0517 chash0517 added the 제출일 이후 제출한PR 제출일에 늦은 PR입니다. label Nov 11, 2024
일단 실행 후 코드 / 로직 수정해야할 부분이 보여서 임시로 작성한 ts 코드 사용
홈페이지를 구성하는 component 추가

참고 : 반응형 적용 안함. tailwind css 반응형 설정. 기존의 방법 이외에 선호되는 방법을 찾아서 적용 예정
일부 새로 생성한 css 파일 삭제
반응형 css 적용을 위해 3개지 선택 사항 중 1안 선택
1. tsx에 직접 tailwind css 사용
2. css module에 tailwind + vanilla(@media) 조합 사용
3. tailwind.config.js만 사용
일부 새로 생성한 css 파일 삭제
Copy link
Collaborator

@wildCodingWarrior wildCodingWarrior left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

Comment on lines +33 to +39
const postPreviewClass = `${style["post-preview"]}`;
const postPreviewTopBarClass = `flex flex-row justify-between ${style["top-bar"]}`;
const postPreviewNameClass = `font-semibold ${style.name}`;
const postPreviewBottomBarClass = `flex flex-row justify-between ${style["bottom-bar"]}`;
const postPreviewBottomBarNicknameDateClass = `flex flex-row items-center ${style["bottom-bar-nickname-date"]}`;
const postPreviewBottomNicknameClass = `font-normal ${style["nickname-name"]}`;

Copy link
Collaborator

Choose a reason for hiding this comment

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

이런식으로 정리하시면 tailwind CSS intellisense 혜택을 못보지 않나요?

차라리 조금 공부하더라도 tailwind-merge랑 clsx를 활용하는 cn유틸을 사용해보는 것이 어떤가 싶습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아직 tailwind를 잘 못 쓰고 있습니다. 정확히는 tailwind 적용한 코드를 돌아보니 tailwind.config.js에 몰아서 정의하다보니, 오히려 관리의 어려움을 느껴, 현재는 tsx 파일에 직접 tailwind를 사용하는 방식으로 수정 중입니다.
tailwind-merge랑 clsx를 활용하는 cn유틸은 자료를 찾아보겠습니다.

참고 : 아마 위의 코드는 바닐라로 만들고 아직 tailwind로 완전히 적용 안 한 부분일 겁니다.

Comment on lines 1 to 21
import { ReactNode } from "react";

export interface MiddleBannerProps {
isLeftImage: boolean;
imagePath: string;
textSetWidth: string;
topText: string;
middleText: string;
bottomText: string;
}

export interface SignBottomTextProps {
message: string;
linkText: string;
linkPath: string;
}

export interface DropdownProps {
dropdwonClass?: string;
minimise?: string | boolean;
children: ReactNode;
Copy link
Collaborator

Choose a reason for hiding this comment

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

컴포넌트에 해당하는 Props는 해당 컴포넌트에 작성하는게 더 응집도 있는 코드라고 생각합니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

알겠습니다. 수정해두겠습니다.

package.json Outdated
"dev": "cross-env NODE_OPTIONS='--inspect' next dev",
"build": "next build",
"dev": "cross-env NODE_OPTIONS=development next dev",
"build": "next build && tsc",
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 tsc는 없어도 무방하겠습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

수정해두겠습니다

"react": "^18",
"react-dom": "^18",
"react-hook-form": "^7.53.0",
"react-modal": "^3.16.1",
"react-query": "^3.39.3",
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 패키지 옛날 버전이라서 @tanstack/react-query를 사용해보세요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"@tanstack/react-query": "^5.56.2", 를 사용 중입니다

회원 가입에 사용되는 컴포넌트 분리
ts에 적합한 코드로 정리
@ajantang ajantang removed the 제출일 이후 제출한PR 제출일에 늦은 PR입니다. label Nov 14, 2024
@chash0517 chash0517 added the 제출일 이후 제출한PR 제출일에 늦은 PR입니다. label Nov 14, 2024
Copy link

vercel bot commented Nov 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
1-sprint-mission-fe ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 14, 2024 6:21am

SSR에서 호출하는 API가 로컬스토리지에 토큰에 접근하는 것을 방지

참고 : 토큰을 로컬스토리지에 저장한 것은 임시 코드여서 이 부분을 어떻게 수정할지 고민 필요
임시로 상품 리스트 / 상품 상세 조회 페이지에 적용할 API 코드 작성

참고 : 테스트 과정에서 일단 문제가 없어보여 커밋. 다른 API 함수 전부 수정해야 됨
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
제출일 이후 제출한PR 제출일에 늦은 PR입니다. 진행 중 🏃 아직 스프린트 미션 제출일이 아닙니다. 새로 커밋된 내용에 대해 코드리뷰 해주세요!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants