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

Conversation

joodongkim
Copy link
Collaborator

요구사항

기본

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

심화

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

주요 변경사항

스크린샷

image

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

withyj-codeit and others added 30 commits September 3, 2023 21:57
* First commit Sprint Mission5 on React

* Sprint mission 6 commit.

* Sprint Mission 6 Update 1

* update sprint mission 6

* Sprint mission 6 update commit.

* delete .bak files

* Update Comments

* modify ItemComment
@joodongkim joodongkim added the 순한맛🐑 마음이 많이 여립니다.. label Nov 2, 2024
@GANGYIKIM GANGYIKIM changed the title Next 김주동 sprint10 [김주동] sprint10 Nov 5, 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.

주동님 이번 스프린트 미션 고생하셨습니다~

try {
const query = new URLSearchParams(params).toString();
const response = await fetch(
`https://panda-market-api.vercel.app/articles/${articleId}/comments?${query}`
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
`https://panda-market-api.vercel.app/articles/${articleId}/comments?${query}`
`${BASE_URL}/articles/${articleId}/comments?${query}`


type ArticleItemProps = {
interface ArticleItemProps {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
interface로 바꿔주신것 좋습니다 👍

{article.writer.nickname} <Timestamp>{dateString}</Timestamp>
</ArticleInfoDiv>
<ArticleInfoWrapper>
<ArticleInfo article={article} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
기존 ArticleInfoDiv에서 ArticleInfo 태그명을 빼주신게 좋네요.

<CommentSectionTitle>댓글 달기</CommentSectionTitle>

<TextArea
placeholder={"댓글을 입력해 주세요."}
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
placeholder={"댓글을 입력해 주세요."}
placeholder="댓글을 입력해 주세요."

Comment on lines +19 to +21
const handleInputChange = (e: ChangeEvent<HTMLTextAreaElement>) => {
setComment(e.target.value);
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
이렇게 인풋값이 입력될때마다 state를 바꿔주는 경우 debounce를 통해 이벤트를 묶어 실행해주시는 것이 좋습니다.
그렇게 되면 리렌더링되는 횟수를 줄일 수 있습니다.

const ArticlePageCommentThread: React.FC<ArticlePageCommentThreadProps> = ({
articleId,
}) => {
const [comments, setComments] = useState<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:
배열의 타입 명시해준거 좋습니다 👍

Comment on lines +23 to +44
useEffect(() => {
if (!articleId) return;

const fetchComments = async () => {
setIsLoading(true);

try {
const response: CommentListResponse = await getArticleComments({
articleId,
});
setComments(response.list);
setError(null);
} catch (error) {
console.error("Error fetching comments:", error);
setError("게시글의 댓글을 불러오지 못했어요.");
} finally {
setIsLoading(false);
}
};

fetchComments();
}, [articleId]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
아래처럼 fetchComments 함수를 useEffect 외부로 빼셔도 됩니다.
이렇게 되면 로직이 분리되어 가독성에도 도움이 됩니다.

  const fetchComments = useCallback(async () => {
      setIsLoading(true);

      try {
        const response: CommentListResponse = await getArticleComments({
          articleId,
        });
        setComments(response.list);
        setError(null);
      } catch (error) {
        console.error("Error fetching comments:", error);
        setError("게시글의 댓글을 불러오지 못했어요.");
      } finally {
        setIsLoading(false);
      }
    });

  useEffect(() => {
    if (!articleId) return;

    fetchComments();
  }, [articleId, fetchComments]);

Comment on lines +54 to +67
if (comments && !comments.length) {
return (
<EmptyState text={`아직 댓글이 없어요,\n지금 댓글을 달아 보세요!`} />
);
} else {
return (
<ThreadContainer>
{comments.map((item) => (
<CommentItem item={item} key={`comment-${item.id}`} />
))}
</ThreadContainer>
);
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
이미 comments && !comments.length 조건문에서 return을 하고 있으니 else문은 사용하지 않으셔도 됩니다.

Suggested change
if (comments && !comments.length) {
return (
<EmptyState text={`아직 댓글이 없어요,\n지금 댓글을 달아 보세요!`} />
);
} else {
return (
<ThreadContainer>
{comments.map((item) => (
<CommentItem item={item} key={`comment-${item.id}`} />
))}
</ThreadContainer>
);
}
};
if (comments && !comments.length) {
return (
<EmptyState text={`아직 댓글이 없어요,\n지금 댓글을 달아 보세요!`} />
);
}
return (
<ThreadContainer>
{comments.map((item) => (
<CommentItem item={item} key={`comment-${item.id}`} />
))}
</ThreadContainer>
);
};

Comment on lines +19 to +22
const [comments, setComments] = useState<Comment[]>([]);
const [isLoading, setIsLoading] = useState(false);
const [error, setError] = useState<string | null>(null);

Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
data, isLoading, error와 같은 state들이 있고 이에 따른 로직들이 반복되니 custom hook으로 분리하시면 더 좋을 것 같아요~

id="image-upload"
type="file"
onChange={handleImageChange}
accept="image/*"
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
file의 accept 속성은 user에게 적힌 확장자를 제안할뿐 제한하는 것은 아닙니다.
특정 확장자만 받고 싶다면 handleImageChange함수에서 해당 file의 확장자를 확인하신 후 이에 따른 로직을 입력해주셔야 합니다.

@GANGYIKIM GANGYIKIM merged commit d0b2b5b 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.

4 participants