[차경훈] sprint6 #157
[차경훈] sprint6 #157
Conversation
dongqui
left a comment
dongqui
left a comment
There was a problem hiding this comment.
경훈님 이번 미션도 고생 많으셨습니다~!! 👍
요구 사항을 충실하게 잘 맞춰 주셨네요! 💯 스타일이 좀 더 유연하게 작성되면 더욱 좋을 거 같습니다 :)
| function App() { | ||
| function AppContent() { | ||
| const location = useLocation(); | ||
| const hideFooterRoutes = ["/additem", "/signin", "/signup"]; |
There was a problem hiding this comment.
| style, | ||
| onKeyDown, | ||
| onBlur, | ||
| className, |
There was a problem hiding this comment.
단순히 기존의 html 속성을 전달하는 거라면 줄여서 써도 괜찮습니다~!
functoin Input({ ...rest }) {
return <StyledInput {...rest} />
}| const [error, setError] = useState(""); | ||
|
|
||
| // 모든 필수 입력값이 채워졌는지 | ||
| const isFormValid = useMemo(() => { |
There was a problem hiding this comment.
웬만한 경우, useMemo는 안 쓰시는 게 나을 수 있어요 :)
https://velog.io/@lky5697/stop-using-usememo-now
또 최근 리엑트에서는 useMemo, useCallback을 컴파일 단계에서 처리할 예정이라고 합니다!
| "react": "^19.1.0", | ||
| "react-dom": "^19.1.0" | ||
| "react-dom": "^19.1.0", | ||
| "styled-components": "^6.1.18" |
There was a problem hiding this comment.
무슨 일인지, react-router-dom이 커밋에서 빠져있습니다.. 😿
| </section> | ||
| </div> | ||
| <ItemsPageContainer> | ||
| <BestItemsSection items={visibleBestItems} loading={loadingBest} /> |
There was a problem hiding this comment.
jsx는 깔끔하게 잘 정리해 주셨습니다 :) 가능하다면 상태 값, fetch 로직 등도 관심사가 같은 것들끼리 모아두면 좋겠죠!
지금은 한 파일에 상태, 로직들이 뒤섞여 있고 파일을 넘나들며 봐야하는 단점이 있습니다~!
| pathname: location.pathname, | ||
| search: newParams.toString() ? `?${newParams.toString()}` : "", | ||
| }, | ||
| { replace: true } |
There was a problem hiding this comment.
경우에 따라 다를 수는 있지만.. replace는 브라우저 히스토리가 안 남으므로 부적절할 거 같습니다.
예를들면,
지금은 랜딩 -> 상품 -> 검색 이후 뒤로가기를 누르면 랜딩페이지가 나오게 되죠..!
| margin-top: 8px; | ||
| `; | ||
|
|
||
| const FormSection = styled.div` |
There was a problem hiding this comment.
간격이 일정하다면 부모에서 flex, gap 사용을 고려해도 좋을 거 같네요~!
| * @param {function} renderField - custom 타입일 때 사용할 렌더 함수 | ||
| * @param {object} props - 추가 속성 | ||
| */ | ||
| function FormField({ |
There was a problem hiding this comment.
크게 문제가 있지는 않지만.. 조금은 아쉬운 컴포넌트 같아요! (의견)
일단 type이 html 속성과 뒤섞여 쓰이고 있는 것이 아쉽습니다.
또한 type별로 나눴을때 해당 컴포넌트가 점점 비대해지는 문제가 있을 거 같아요.
차라리 label, error 등을 각각 컴포넌트에 포함 시켜주거나 아니면 FormField를 wrapper로 쓰면서 간단하게 각 필드를 children으로 받아오는 게 조금 더 직관적일 거 같습니다.
| min-width: 221px !important; | ||
| min-height: 221px !important; | ||
| max-width: 221px !important; | ||
| max-height: 221px !important; |
There was a problem hiding this comment.
음.. 어떤 일이 있었던 걸까요! 스타일이 너무 무섭게 들어가고 있습니다 😨
width, min, max가 모두 같은 값에 important..!!
important는 유지보수가 힘든 경향이 있어 조심해서 쓰시는 것이 좋습니다 :)
또한 width, min, max를 모두 같은 값으로 주는 것은 의미 없는 코드 같아요!
There was a problem hiding this comment.
또 정확하게 크기를 주는 것보다 컨테이너 크기에 맞춰 유연하게 대응하는 것이 좀 더 나은 UI/UX가 될 수 있습니다 :) 실무에서는 조금 애매한 경우는 디자이너분과 상의해 보는 것도 좋겠죠!
고정된 크기
유연한 크기
| return ( | ||
| <SearchFormContainer onSubmit={onSubmit}> | ||
| <SearchInputWrapper> | ||
| <SearchIcon |
There was a problem hiding this comment.
이렇게 하셔도 상관 없지만, 작성해두신 Input 컴포넌트를 좀 더 확장성 있게 정의하는 방법도 있습니다 :)
function Input({ prefix }) {
return
<Wrapper>
{prefix}
<StyledInput/>
<Wrapper/>
}
<Input prefix={<SearchIcon/>} />
2 participants
요구사항
기본
상품 등록
심화
상품 등록
주요 변경사항
멘토에게