-
Notifications
You must be signed in to change notification settings - Fork 39
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 #261
The head ref may contain hidden characters: "Next-\uAE40\uC608\uC900-sprint10-merge"
[김예준] Sprint10 #261
Conversation
<div | ||
key={key} | ||
className={styles.dropDownItem} | ||
onClick={() => handleOptionClick(key)} | ||
> | ||
{key} | ||
{value} |
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.
여기서 value로 바꿔줬기때문에 지금 dropdown에서 영어로 나오고 있어요!
만약 value로 여기를 바꾸면 내려오는 options도 다 바꿔줘야합니다!
@@ -9,14 +10,19 @@ import Button from '@/src/components/Button/Button' | |||
import styles from './Article.module.scss' | |||
|
|||
export default function ArticleList() { | |||
const [orderBy, setOrderBy] = useState('recent') | |||
type OrderOption = 'recent' | 'like' |
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.
여기서 dropdown으로 내려주는 친구들이있네요!
우선 보통 type은 컴포넌트 밖에서 선언을 해줘요.
그리고 oderOption에 해당하는 값들은 상수로 선언해주면 더 보기도 편하고 유지보수도 좋을 것 같아요!
const ODER_OPTION = {
recent: '최신순',
like: '인기순',
} as const
type OrderOption = keyof typeof ODER_OPTION
이렇게 key에 영어를 넣고 value에 한글을 넣으면 드롭다운에서 value로 보여줄 때 한글로 잘보일거에요!
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 url = useMemo(() => { | ||
let baseUrl = `articles?page=1&pageSize=10&orderBy=${orderBy}` | ||
if (searchTitle) { | ||
baseUrl += `&keyword=${searchTitle}` | ||
} | ||
return baseUrl | ||
}, [orderBy, searchTitle]) |
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.
useMemo를 사용하여 url변화값을 잘 메모해주셨어요!
한가지 종종사용되는 부분을 추가적으로 말씀드리자면 orderByt나 keyword같이 검색에 필요한 파라미터들은 router를 이용하여 실제 url query에 넣기도합니다.
그렇게 했을 때 좋은 점은 실제로 유저의 행동을 history에 기억할 수 있다는 점이에요!
예를들어서 무언가를 검색하고 다른페이지로 넘어갔을 때, 뒤로가기를 누르면 그 검색한 키워드를 기억 할 수 있겠죠?!
return ( | ||
<> | ||
<ArticleDetail /> | ||
|
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.
<> | ||
<CreateArticle /> | ||
</> |
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.
<> | |
<CreateArticle /> | |
</> | |
<CreateArticle /> |
불필요한 fragment인것같아요!
혹시 CreateArticle컴포넌트를 따로 빼신 이유가 있으실까요??
const fetechArticles = useFetechData<ArticleListResponse>(url) | ||
const { data: ArticleList, isLoading } = fetechArticles |
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 fetechArticles = useFetechData<ArticleListResponse>(url) | |
const { data: ArticleList, isLoading } = fetechArticles | |
const { data: articleList, isLoading } = useFetchData<ArticleListResponse>(url) |
하나로 합칠 수 있을 것 같아요!
그리고 변수이름은 camelCase로 해주는 것이 일반적입니다~ 오타도 하나 있네요!
export default function CreateArticle() { | ||
const url = `/articles` | ||
|
||
const [formValues, setFormValues] = useState<FormValues>({ |
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.
creatArticle 안에 너무 많은 코드가 들어있어서 한눈에 들어오기가 어려운것같아요!
form관려한 부분을 hook으로 빼서 분리를 해주면 관심사가 분리되어 조금 더 매끄러운 코드가 될 것 같습니다!
width={282} | ||
height={282} | ||
/> | ||
<button className={styles.xButton} onClick={handleImageDelete}> |
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.
type이 명시가 안되어있으면 기본적으로 submit이 들어가요!
그래서 예상 외의 행동을 할 수도 있을 것 같습니다!
const { id } = router.query | ||
const url = `/articles/${id}/comments` | ||
|
||
const [comment, setComment] = useState<string>('') |
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에 관련한 로직을 useAddComment 정도로 따로 분리해주면 코드읽기에도 좋고 향후 재사용성도 늘어날것같아요!
style={{ | ||
display: articleImage === '' ? 'none' : 'block', | ||
}} |
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파일을 따로 두고있으니 inline style보단 css파일에서 분리를 해준것이 더 통일성 있어보입니다!
<div className={styles.container}> | ||
<div className={styles.sectionTitle}>댓글 달기</div> | ||
<form onSubmit={handleSubmit} className={styles.commentContainer}> | ||
<textarea | ||
className={styles.commentInput} | ||
value={comment} | ||
onChange={(e) => setComment(e.target.value)} | ||
placeholder="댓글을 입력해주세요." | ||
required | ||
/> | ||
{isLoading ? ( | ||
<Spinner /> | ||
) : ( | ||
<button | ||
className={styles.submitButton} | ||
type="submit" | ||
disabled={isButtonDisabled} | ||
> | ||
등록 | ||
</button> | ||
)} | ||
{error && <div>{error}</div>} | ||
</form> | ||
</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.
파일구조가 container안에 있는데, 여기서 컴포넌트까지 다 보여주는것이 조금 어색해보입니다!
컴포넌트를 따로 빼서 작성하는게 파일구조와 일관성이 있어보여요!
<> | ||
<div className={styles.contentSection}> | ||
<div className={styles.content}>{comment.content}</div> | ||
<Image | ||
src={seemoreIcon} | ||
width={24} | ||
height={24} | ||
alt="더보기 아이콘" | ||
/> | ||
</div> | ||
|
||
<div className={styles.writerSection}> | ||
<Image | ||
src={defaultProfileIcon} | ||
alt="기본 프로필 이미지" | ||
width={32} | ||
height={32} | ||
/> | ||
<div className={styles.writer}> | ||
<div className={styles.nickname}>{comment.writer.nickname}</div> | ||
<div className={styles.createdAt}> | ||
{formatDate(comment.createdAt)} | ||
</div> | ||
</div> | ||
</div> | ||
|
||
<hr className={styles.hr} /> | ||
</> |
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.
이부분은 충분히 컴포넌트로 따로 빼서 사용이 가능할것같습니다!
요구사항
기본
상품 등록 페이지
상품 상세 페이지
심화
주요 변경사항
스크린샷
멘토에게