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

[이승현] Sprint10 #298

Conversation

leesh7048
Copy link
Collaborator

@leesh7048 leesh7048 commented Aug 16, 2024

요구사항

  • Github에 PR(Pull Request)을 만들어서 미션을 제출합니다.
  • 피그마 디자인에 맞게 페이지를 만들어 주세요.
  • 기존의 React, Typescript로 구현한 프로젝트와 별도로 진행합니다.
    ��- Next.js를 사용합니다

기본

상품 등록 페이지

  • 상품 등록 페이지 주소는 “/addboard” 입니다.
  • 게시판 이미지는 최대 한개 업로드가 가능합니다.
  • 각 input의 placeholder 값을 정확히 입력해주세요.
  • 이미지를 제외하고 input 에 모든 값을 입력하면 ‘등록' 버튼이 활성화 됩니다.
  • 회원가입, 로그인 api를 사용하여 받은accessToken을 사용하여 게시물 등록을 합니다.
  • 등록’ 버튼을 누르면 게시물 상세 페이지로 이동합니다.

상품 상세 페이지

  • 상품 상세 페이지 주소는 “/board/{id}” 입니다.
  • 댓글 input 값을 입력하면 ‘등록' 버튼이 활성화 됩니다.
  • 활성화된 ‘등록' 버튼을 누르면 댓글이 등록됩니다

스크린샷

멘토에게

아직 별로 만든게 없습니다... 주말동안 완성해보겠습니다!
이번에도 merge 천천히 해주시면 감사하겠습니다!

@leesh7048 leesh7048 requested a review from 201steve August 16, 2024 14:28
@leesh7048 leesh7048 added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Aug 16, 2024
Copy link
Collaborator

@201steve 201steve left a comment

Choose a reason for hiding this comment

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

수고 많으셨습니다!
api 호출 함수 분리도 잘 해주셨고, 타입 분리도 깔끔하게 되어있어서 보기 좋네요!
api 호출 할때에 에러핸들링도 신경써주시면 더할나위없이 좋을것같습니다 :-)

Comment on lines +22 to +27
export const getArticle = async (articleId: number) => {
const res = await axios.get(`/articles/${articleId}`);
if (res.status !== 200) throw new Error("getArticle api 오류");
const result = res.data;
return result;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

export const getArticle = async (articleId: number) => {
  try{
    const res = await axios.get(`/articles/${articleId}`);
  if (res.status !== 200) throw new Error("getArticle api 오류");
  const result = res.data;
  return result;
  }
  catch(error){
    console.error(error.message)
    //에러 상황에 해야할 행동
  }
};

에러 핸들링 해주는 코드가 추가되면 resolve, reject 상황에 좀더 보기 편할 것같아요
네트워크 호출, 응답은 항상 resolve 된다는 보장이 없어서 try,catch가 항상 따라다닌다고 생각하시면 좀더 습관화 하시기 좋을꺼에요!

Comment on lines +15 to +24
const [isDisabledSubmit, setIsDisabledSubmit] = useState(true);

const isDisabledCheck = useCallback(() => {
const { title, content } = values;
if (title && content) {
setIsDisabledSubmit(false);
} else {
setIsDisabledSubmit(true);
}
}, [values]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

이미 관리하고있는 상태값으로 계산할 수 있는 영역이어서, disabled를 다룰때에 쓰일 state를 하나 더 만들지 않아도 될것같아요.

state 구조를 생각할 때에 state 생성 원칙이 공식문서에 있어요 여길 한번 참고 해 보세요.

Suggested change
const [isDisabledSubmit, setIsDisabledSubmit] = useState(true);
const isDisabledCheck = useCallback(() => {
const { title, content } = values;
if (title && content) {
setIsDisabledSubmit(false);
} else {
setIsDisabledSubmit(true);
}
}, [values]);
const {title,content} = values;
const canSubmit = title&&content
// ...
// ...(중략)
<button className={styles.addBoardBtn} disabled={!canSubmit}>

Comment on lines +30 to +33
setValues((preValues) => ({
...preValues,
[name]: value,
}));
Copy link
Collaborator

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 +35
useEffect(() => {
id && fetchComments(Number(id), 5);
}, [id]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
useEffect(() => {
id && fetchComments(Number(id), 5);
}, [id]);
useEffect(() => {
if(id){
fetchArticle(Number(id));
fetchComments(Number(id), 5);
}
}, [id]);

같은 조건에 동작하는 useEffect인것같아요. 하나로 통합해보는건 어떨까요? useEffect 둘 다 UI 렌더 이후에 동작해서 같은 조건을 보고 함수 호출하는 역할만 할꺼라 합쳐도 무방하고, 코드 보는게 좀더 깔끔해질것같아요. :-)

Comment on lines +37 to +43
const dateFormat = (date: Date) => {
const newDate = new Date(date);
const formatDate = `${newDate.getFullYear()}.${String(
newDate.getMonth() + 1
).padStart(2, "0")}.${String(newDate.getDate()).padStart(2, "0")}`;
return formatDate;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

날짜 포매팅 잘 만들어주셨습니다.
new Data 객체로 만들어보는 경험도 중요 해요.

좀더 빠르게 만들고싶다면 라이브러리도 추천드립니다.
dayjs 설치하셔서

const dateFormat = (date: Date) => {
  return dayjs(date).format('YYYY.MM.DD');
};

로 하면 연,월,일 포맷도 간편하게 만들 수 있어요 참고 해보세요~

Comment on lines +42 to +46
export interface CommentWriter {
id: number;
nickname: string;
image: string;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
export interface CommentWriter {
id: number;
nickname: string;
image: string;
}
export interface Writer {
id: number;
nickname: string;
}
export interface CommentWriter extends Writer{
image: string;
}

비슷한 구조를 가진 간단한 interface끼리 확장해서 쓸 수 있어요

pageSize,
orderBy,
keyword,
}: GetArticles) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 함수는 반환은 하고있는데, 반환 타입이 없네요

getArticle, getComments도 반환타입을 추가 해 보세요! :-)

@201steve 201steve merged commit 0768221 into codeit-bootcamp-frontend:Next-이승현 Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants