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

[정해찬] sprint 8 #117

Conversation

just-codingbaby
Copy link
Collaborator

@just-codingbaby just-codingbaby commented Dec 8, 2024

요구사항

기본

  • [x]
  • []
  • []

심화

  • [x]
  • []

주요 변경사항

스크린샷

스크린샷 2024-12-09 00 55 07

멘토에게

  • tailwind css도 같이 적용해보았습니다, 백엔드파트는 초급 프로젝트 진행으로 미션 6,7를 진행하지 않아서 모범답안을 활용하여 데이터 렌더링을 진행하였습니다
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@just-codingbaby just-codingbaby added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 완성은 아니지만 제출합니다... 진행 중 🏃 스프린트 미션 진행중입니다. labels Dec 8, 2024
@just-codingbaby just-codingbaby added 제출일 이후 제출한PR 제출 마감일(일요일) 이후에 제출하는 PR입니다. 최종 제출 스프린트미션 최종 제출본입니다. and removed 미완성🫠 완성은 아니지만 제출합니다... 진행 중 🏃 스프린트 미션 진행중입니다. labels Dec 19, 2024
@orlein
Copy link
Collaborator

orlein commented Dec 25, 2024

최종 제출 라벨을 붙이시면 꼭 디스코드에 말씀 부탁드립니다...... 알림이 안와요................

**/._.DS_Store

pandamarket/
.vscode/
Copy link
Collaborator

Choose a reason for hiding this comment

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

.vscode 폴더 내용은 프로젝트의 성격에 따라 공유되어야할때도 있긴 합니다.

Comment on lines +1 to +11

export default function Button({ children, handleClick, disabled = false}) {
return (
<>
<button onClick={handleClick} disabled={disabled}
className={`${ disabled ? "bg-gray-400" : "bg-custom_blue"} w-[88px] h-[42px] rounded-lg px-[23px] py-3 text-base font-semibold leading-[19.09px] text-white`}>
{children}
</button>
</>
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

생각보다 Button은 다양한 형태가 있고, 단 하나의 형태를 위해 컴포넌트를 사용하실거라면 이렇게 파일로 만드실 필요는 없습니다. 게다가 width, height도 픽셀고정이라 좋은 컴포넌트 형태가 아닙니다.

Comment on lines +8 to +9
isOpen,
setIsOpen,
Copy link
Collaborator

Choose a reason for hiding this comment

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

나중에 Dropdown 컴포넌트가 깊어지거나 할 가능성이 있는데요, 그럴땐 이렇게 props drilling을 유발할 수 있는 prop보다는 Context 사용을 고려해보세요.

selectedOption,
isOpen,
setIsOpen,
isKebab,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dropdown의 성질이 kebab만 있는게 아니고 다른 형태도 있을 수 있습니다. type={"kebab"} 이런식으로 prop을 받게끔 한다면 좀 더 명확하고 확장성있겠습니다.

Comment on lines +16 to +37
{isKebab ? (
<button onClick={toggleOpen} className="pl-[155px]">
<Image src="/img/btn_dropDown.png" width={3} height={13} alt="드롭다운 케밥 아이콘" />
</button>
) : (
<button
onClick={toggleOpen}
className="w-full text-left px-4 py-2 border rounded-lg bg-white focus:outline-none"
>
{selectedOption}
</button>
)}

{!isKebab && (
<Image
src="/img/drop_arrow.png"
alt="drop-arrow"
width={15.7}
height={14.2}
className="absolute right-5 top-4"
/>
)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

isKebab 조건이 둘로 나뉩니다. 이런 경우, 차라리 if(isKebab) return (... ) 이후 return (...) 하는 느낌으로 아예 isKebab일 때의 모습을 한쪽으로 몰아놓는게 가독성과 Testability가 좋습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

pages 아래의 파일들은 신중하게 만들고 사용하셔야합니다.

import Button from "@/components/Button";
import { useRouter } from "next/router";

export async function getServerSideProps(context) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

getServerSidePropsrequest time에 실행되는 함수입니다. 그렇기때문에 이 함수의 실행비용은 굉장히 큽니다. 사용에 주의하세요.

Comment on lines +23 to +28
} catch (err) {
console.error("Error fetching data:", err);
return {
notFound: true,
};
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

모든 에러를 notFound로 처리해선 안됩니다. 어떤 에러는 에러의 정체를 밝혀야할 때가 있는데 https://nextjs.org/docs/pages/building-your-application/routing/custom-error#500-page 를 참고하세요

try {
const res = await axios.post(`articles/${data.id}/comments`, commentData);
if (res.status === 201) {
alert("댓글 등록 완료!");
Copy link
Collaborator

Choose a reason for hiding this comment

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

React를 사용하신다면 alert사용은 자제하세요. UI/UX를 크게 해칩니다.

Comment on lines +40 to +45
const toggleDropdown = (id) => {
setIsOpenMap((prev) => ({
...prev,
[id]: !prev[id],
}));
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

[id]로 key설정 하려면 id가 key에 들어갈 수 있는 값인지 명확히 validation하세요.

@orlein orlein merged commit 1670bbf into codeit-sprint-fullstack:next-정해찬 Dec 25, 2024
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.

2 participants