[김제성] sprint 9#75
Hidden character warning
Conversation
rl0425
left a comment
rl0425
left a comment
There was a problem hiding this comment.
고생하셨습니다.
다만 코드를 작성하실때, 한 컴포넌트에 너무 많은 코드와 중첩된 요소를 사용하게 되면 가독성이 떨어지게 됩니다. 이는 다른 개발자가 볼 때 뿐 아니라, 본인이 유지 보수하기에도 힘들 있기 때문에 이 점 참고하셔서, 작성하실 때 어떻게 더 알아보기 쉽게 코드를 작성할 수 있을까를 생각하시면 더 좋은 개발자가 되실 것 같습니다.
지금 문제가 있어서 말씀드리는게 아니라, 추후 더 복잡한 코드 개발 시에 팁이라고 생각해주시면 될 것 같습니다.
현재 공통 컴포넌트를 만드시는건 너무 잘하고 계시고, 리뷰에 남겨드린 children 요소 삽입이나 기본값 설정만 주의하시면 될 것 같습니다. 또 next의 Link, Image 사용도 자연스럽게 잘 사용하고 계십니다.
| export default function BestPosts({ className = "", posts }) { | ||
| const router = useRouter(); | ||
|
|
||
| const handlePostClick = (postId) => { |
There was a problem hiding this comment.
링크를 임포트해서 사용하셔서 이 펑션은 사용하시지 않는 것 같아, 라우터 코드와 함께 지우셔야 할 것 같습니다.
| <div className={`${styles.bestPosts} ${className}`}> | ||
| <h2 className={styles.title}>베스트 게시글</h2> | ||
| <div className={styles.postGrid}> | ||
| {posts?.map((post) => ( |
There was a problem hiding this comment.
posts를 ?로 체킹하시는건 좋지만, 막상 포스트가 없을 경우에 띄울 내용이 없습니다. 때문에
posts && <현재 컴포넌트>
!posts && <비었을 경우 컴포넌트>위 코드처럼 &&을 사용하시거나, 삼항 연산자를 사용하셔서 비었을 경우도 만들어 주시면 좋을 것 같습니다.
There was a problem hiding this comment.
밑에 post.views, post.date도 마찬가지로, 해당 데이터가 없을 경우의 예외처리를 같이 해주셔야 합니다.
ex)
{post.title || "제목 없음"}| @@ -0,0 +1,7 @@ | |||
| import styles from "./Button.module.css"; | |||
|
|
|||
| export default function Button({ className = "", as, ...props }) { | |||
There was a problem hiding this comment.
공통 컴포넌트를 만드시는거라면 Button 같은 경우에는 열린 태그이기 때문에, 안쪽 요소를 받을 수 있어야 합니다. 보통은 children default props를 사용해서 내부 데이터를 표현합니다.
function Button({ className = "", as, children, ...props }) {
...
return <button className={`${buttonStyles.button} ${className}`} {...props}>{children}</button>;| const classNames = `${styles.input} ${ | ||
| isOpen ? styles.opened : "" | ||
| } ${className}`; | ||
| const selectedOption = options.find((option) => option.value === value); |
There was a problem hiding this comment.
옵션에서 벨류를 못 찾을 경우의 예외처리가 필요합니다.
| import { useEffect, useState, useRef } from "react"; | ||
| import styles from "./Dropdown.module.css"; | ||
|
|
||
| export default function Dropdown({ |
There was a problem hiding this comment.
공통 드랍다운 컴포넌트면, 접근성을 챙기기 위해서 ARIA 속성과 키보드 네비게이션도 추후에 지원하시는걸 추천드립니다.
| {selectedOption.label} | ||
| <span className={styles.arrow}>▴</span> | ||
| <div className={styles.options}> | ||
| {options.map((option) => { |
There was a problem hiding this comment.
jsx 안에서 조건문이나 변수 사용을 하시기 보다는, 따로 컴포넌트를 파시고 컴포넌트를 호출하시는 방법을 더 추천드립니다.
| @@ -0,0 +1,30 @@ | |||
| const { createContext, useContext, useState, useEffect } = require("react"); | |||
There was a problem hiding this comment.
혹시 require을 쓰신 다른 이유가 있으실까요? 없으시면 ES6 문법인 import를 사용하시는게 통일성이 있을 것 같습니다.
| @@ -0,0 +1,30 @@ | |||
| const { createContext, useContext, useState, useEffect } = require("react"); | |||
|
|
|||
| export const ThemeContext = createContext(); | |||
There was a problem hiding this comment.
Context를 사용하실 땐 초기값을 설정해주시는게 좋습니다!
export const ThemeContext = createContext({
theme: "dark",
setTheme: () => {},
});2 participants
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게