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 #126

Conversation

swim-kim
Copy link
Collaborator

@swim-kim swim-kim commented Nov 2, 2024

요구사항

기본

  • 상품 등록 페이지 주소는 "/addboard" 입니다.
  • 게시판 이미지는 최대 한개 업로드가 가능합니다
  • 각 input의 placeholder 값을 정확히 입력해주세요.
  • 이미지를 제외하고 input 에 모든 값을 입력하면 '등록' 버튼이 활성화 됩니다.
  • 상품 상세 페이지 주소는 "/board/{id}" 입니다.
  • 댓글 input 값을 입력하면 '등록' 버튼이 활성화 됩니다.
  • 활성화된 '등록' 버튼을 누르면 댓글이 등록됩니다

심화

  • 회원가입, 로그인 api를 사용하여 받은accessToken을 사용하여 게시물 등록을 합니다.
  • '등록' 버튼을 누르면 게시물 상세 페이지로 이동합니다.

멘토에게

  • board/[id] 에서 다른페이지로 이동했다가 뒤로가기를 했을때 id 값을 못가져오는 문제가 있습니다 ..

@swim-kim swim-kim requested a review from GANGYIKIM November 2, 2024 10:18
@swim-kim swim-kim self-assigned this Nov 2, 2024
@swim-kim swim-kim added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Nov 2, 2024
Copy link
Collaborator

@GANGYIKIM GANGYIKIM left a comment

Choose a reason for hiding this comment

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

수영님 스프린트 미션 고생하셨습니다~
board/[id] 에서 다른페이지로 이동했다가 뒤로가기를 했을때 id 값을 못가져오는 문제가 있습니다 .. 라고 말씀해주신것은 코멘트와 멘토링 시간에 같이 논의했으니 잘 해결하실 수 있을거에요~

Comment on lines +7 to +11
export interface CommentItemProps {
item:Comment;
}

const CommentItem: React.FC<CommentItemProps> = ({ item }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
item으로 묶어 전달 받기보다 content, writer, createdAt로 분해해 받는 게 더 좋아보여요~
item 객체로 받으면 꼭 다른 값도 받기위해서 묶어서 받는 것처럼 보이니, 필요한 값만 전달 받으시면 더 좋을 것 같아요

list: Comment[];
}

export default function Comment() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
코멘트 다는 부분과 코멘트 리스트를 분리하면 더 좋을 것 같아요. 로직들이 섞여 있어서 파악하기가 좀 어렵네요.

const [value, setValue] = useState("");
const [isValid, setIsValid] = useState(false);
const router = useRouter();
const id = router.query['id'];
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
next에서 query 객체를 통해 데이터를 불러올 경우 아래 글을 참고해서 코드를 수정해주셔야 에러가 나지 않습니다.

https://velog.io/@jiwonyyy/NextJs-router.query%EA%B0%80-undefined%EC%9D%BC-%EB%95%8C

Comment on lines +45 to +47
if (!item) {
return ;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
item이 없을때 null을 반환하면 해당 컴포넌트가 공간을 차지하지 않게 되어서
가능하면 데이터가 없는 빈 컴포넌트가 렌더링될 수 있게 item에 default value로 빈 객체, 혹은 적절한 값을 넘겨주시는 것을 추천드립니다.

export default function Article() {
const router = useRouter();
const id = router.query['id'];
const [item, setItem] = useState<Article | undefined>(undefined);
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
undefined는 명시해주지 않으셔도 됩니다.

Suggested change
const [item, setItem] = useState<Article | undefined>(undefined);
const [item, setItem] = useState<Article>();

Comment on lines +40 to +43
params: {
limit: params.limit,
cursor: params.cursor
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
키값이 같기 때문에 아래처럼 하셔도 됩니다.

Suggested change
params: {
limit: params.limit,
cursor: params.cursor
}
params


export async function getArticleComment(articleId: number, params = { limit: 10, cursor: 0 }) {
try {
const response = await axios.get<CommentList>(`${BASE_URL}/articles/${articleId}/comments`, {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
👍

Comment on lines +7 to +10
export async function getArticles(params = {}) {
const allowedParams = ["orderBy", "pageSize", "page", "keyword"];
const invalidParams = Object.keys(params)
.filter(key => !allowedParams.includes(key));
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:
params 타입을 명시해주시면 좋을 것 같아요~ 그렇게 해주시면 아래에 어떤 값들이 param에 들어왔는지 확인하는 로직이 없어도 될 것 같습니다~

<AS.InputLabel>이미지</AS.InputLabel>
<AS.ImgBox>
<AS.CustomButton onClick={() => {inputRef.current?.click();}}>
<AS.ContentInput
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
file input의 경우 대부분 정해진 확장자의 파일만을 받게 되는데 이를 위해 input의 accept 속성을 통해 원하는 확장자를 제안하고
파일이 들어오면 handleImageChange 같은 함수에서 해당 파일의 확장자를 확인하는 로직이 필요합니다~
나중에 시간이 되실때 추가해보세요

https://developer.mozilla.org/ko/docs/Web/HTML/Element/input/file

@GANGYIKIM GANGYIKIM merged commit 9a96ed5 into codeit-bootcamp-frontend:Next-김수영 Nov 5, 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