-
Notifications
You must be signed in to change notification settings - Fork 79
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 #674
The head ref may contain hidden characters: "Next.js-\uC774\uC601\uD6C8-sprint10"
[이영훈] sprint11 #674
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
과제 하느라 고생하셨습니다! 잘 만들어주셨네요 👍
@@ -1,7 +1,7 @@ | |||
const BASE_URL = "https://panda-market-api.vercel.app"; | |||
const BASE_URL = process.env.NEXT_PUBLIC_API; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.env 적용해주신거 좋네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
적용 했습니다 감사합니다!
|
||
return ( | ||
<Link href={`/addboard/${id}`}> | ||
<div className={styles.ArticleCard}> | ||
<div className={styles.section_top}> | ||
<div className={styles.title}>{title}</div> | ||
{image ? ( | ||
{image && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
&& 적용해주신거 좋아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이것도 적용 했습니다 감사합니다!
)} | ||
</div> | ||
<div className={styles.section_bottom}> | ||
<div className={styles.section_left}> | ||
<Image src="../images/logo.svg" alt="" width={24} height={24} /> | ||
<div className={styles.profile_name}>{nickname}</div> | ||
<div className={styles.date}>{formattedDate}</div> | ||
<div className={styles.date}>{getFormattedDate(createdAt)}</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
자주 사용하는 로직 함수로 변환해주신거 좋네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
말씀해주신대로 적용 해봤습니다 감사합니다!
comment: Comment; | ||
handleDeleteComment: (id: number) => void; | ||
}) => { | ||
const [kebabButton, setKebabButton] = useState(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
케밥버튼을 껐다 키기 위한 상태 같은데, 이런 경우는 좀 더 명시적으로 visible이나 open 같은 이름을 붙여주시는 것도 추천드려요. kebabButtonVisible, kebabButtonOpen 이런식으로요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 넵 참고해서 수정하겠습니다!
@@ -60,7 +72,11 @@ const ArticleFeedComments = ({ id }: { id: string }) => { | |||
<div className={styles.comments}> | |||
{comments.length | |||
? comments.map((comment) => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 ?를 &&로 바꿔주시면 좋을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 여기 까먹고 빼먹었네요 감사합니다!
const pathname = router.pathname; | ||
const { pathname } = router; | ||
|
||
const [accessToken, setAccessToken] = useState<string | null>(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
null 대신 undefined를 사용하시는 것도 추천드려요! 그러면 useState<string | undefined>();
이렇게 선언 가능해집니다. 이러면 새로운 null 타입을 사용하지 않고도 처리할 수 있다는 장점이 있습니다.
혹은 string 이기 때문에 아예 초기값을 빈 문자열로 넣어주고 타입추론을 활용해서 useState('');
이렇게 선언하시는 것도 좋아보여요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오오 감사합니다 수정해보겠습니다!
} | ||
|
||
#top_banner { | ||
background-image: url("./image/상단배너이미지.png"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
파일명은 영어로 작성하시는 것을 추천드립니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 이 CSS 파일은 스프린트 1에서 그대로 가져와서 테일윈드로 변경하려고했던 파일인데 까먹고 삭제를 안했네요 😅 삭제하겠습니다!
|
||
.footer { | ||
height: 160px; | ||
background-color: #111827; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
컬러 변수 사용이 누락됐네요.
const formattedDate = date.toISOString().split("T")[0]; | ||
return formattedDate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런 경우에는 새로운 변수에 할당하지 않고 바로 return 해주시는것도 괜찮습니다.
return date.toISOString().split("T")[0];
이런식으로요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 수정하겠습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
유효성 검사해주실때, .trim 함수를 적용해주시면 좋을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 적용하겠습니다! 코드 리뷰 감사합니다!
그래서 utils 폴더 validateForm.ts에 로그인, 회원가입 폼에 각 인풋의 유효성 검사 로직은 빼놓았는데 LoginSignupForm.tsx 파일에 나머지 로직이 너무 몰려있는거 같아서 어떻게 하면 코드가 아름답게 분리될지 모르겠습니다 🤔
데이터 다루는게 아직 익숙하지 않아서 try catch로 사용하고 싶은데 어떻게 에러처리 로직을 짜야할지 팁좀 주시면 감사하겠습니다!
로컬스토리지에 저장된 토큰을 불러오는 과정에서 window객체를 그냥 쓰니까 바로 에러가 나더라구요 혹시 Next.js에서는 무조건 useEffect 안에서만 window객체에 접근할 수 있는건지 궁금합니다!
저번 코드리뷰 반영해서 .env.local에 base url 환경변수 적용했습니다!
|
요구사항
회원가입
로그인
메인
심화
주요 변경사항
스크린샷
로그인
회원가입
�로컬 스토리지
멘토에게