[이유진] sprint5 #134
[이유진] sprint5 #134
Conversation
[Fix] delete merged branch github action
dongqui
left a comment
dongqui
left a comment
There was a problem hiding this comment.
유진님!
구조를 잘 잡아주셔서 보기 편했습니다~! 👍
컴포넌트 역할, 도메인별로 추상화 해보는 것과 공통 컴포넌트를 좀 더 발전시켜 보시면 더욱 좋을 거 같습니다 :)
질문 주신 부분은 리뷰 참고해 주세요!
| @@ -0,0 +1,36 @@ | |||
| import React from "react"; | |||
|
|
|||
| function TextInput({ | |||
There was a problem hiding this comment.
두 가지 정도 더 발전 시켜볼 수 있을 거 같아요 :)
우선 Pwinput, Textinput, SearchInput은 UI상 거의 같습니다!
이경우 공통 컴포넌트를 좀 더 확장성 있게 만들어 재사용을 높여 볼 수 있습니다
function TextInput({suffix, prefix, ...}) {
return <div className={`input-box el-txt-input ${error ? "error" : ""}`}>
<label htmlFor={id}>{label}</label>
{prefix}
<input
id={id}
name={name}
type={type}
placeholder={placeholder}
value={value}
onChange={onChange}
onBlur={handleBlur}
/>
{suffix}
{error && <p className="error-txt">{error}</p>}
</div>이런식으로 인풋 앞뒤에 아이콘이나 버튼을 받을 수 있도록 해줄 수 있겠네요 :)
또한 컴포넌트는 되도록이면 한 가지 역할만 하는 것이 좋습니다 :)
TextInput은 공통 컴포넌트로서 UI 역할만 하면 좀 더 유연하게 사용할 수 있습니다.
지금은 blur이벤트 때 유효성 검사를 하도록 했는데, 사실 이런 부분은 onBlur만 받고 나머지 로직은 밖에서 처리하는 것이 좋습니다.
TextInput은 UI의 역할을 할뿐, blur가 발생했을 때 어떤 일이 생길지 신경쓰지 않는거죠!
근데 이미 충분히 잘 하셨습니다 🤣 참고만 해주세요!
| import "./ItemComponent.scss"; | ||
|
|
||
| function AllItems({ itemCount, onLoading, onError }) { | ||
| const [items, setItems] = useState([]); |
There was a problem hiding this comment.
items, loading 등 items와 관련된 상태, 로직들을 한 곳에 모아 훅으로 작성하면 더욱 깔끔해 질 거 같습니다 :)
const { isLoading, items } = useItems(order, page....);
| function BestItems({ itemCount, onLoading, onError }) { | ||
| const [items, setItems] = useState([]); | ||
|
|
||
| const handleGetBestItem = async () => { |
There was a problem hiding this comment.
위에서 언급드린 것 처럼 훅으로 작성하셨다면 여기서도 깔끔하게 재사용할 수 있습니다 :)
| function ItemCard({ item }) { | ||
| const [imgSrc, setImgSrc] = useState(item?.images[0] || mockImg); | ||
|
|
||
| const handleImgError = () => { |
| // } | ||
|
|
||
| function OrderSelect({ value, handleOrder }) { | ||
| const [isOpen, setIsOpen] = useState(false); |
There was a problem hiding this comment.
여유가 되신다면 query값들을 상태 값이 아니라 url 쿼리스트링으로 관리해 보시면 좋습니다~!
지금은 새로고침을 하거나 브라우저 뒤로가기 버튼 등을 누르면 모든 상태 값이 날라가게 됩니다 😢
| })); | ||
| }; | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
form과 error가 이미 상태 값이고 isValid는 두 값에 의해 결정된다면 굳이 useEffect를 쓰실 필요 없습니다 :)
const hasError = Object.values(error).some(Boolean);
const hasEmpty = Object.values(form).some((val) => val === "");
const isValid = !hasError && !hasEmpty| } from "../../utils/validator"; | ||
|
|
||
| function Signup() { | ||
| const [form, setForm] = useState({ |
There was a problem hiding this comment.
input이 늘어날 수록 관리할 상태 값들이 너무 늘어나죠 🤣
react-hook-form 같은 것들을 사용해 보셔도 좋습니다! controlled/uncontrolled component 개념도 함께 익히시면 좋아요!
| desktop: 1200, | ||
| }; | ||
|
|
||
| function useResponsiveCount(breakpoints, defaultCount) { |
There was a problem hiding this comment.
하긴 나름이긴 하지만..훅은 보통 hooks라고 폴더를 따로 만듭니다 🤣
utils은 도메인이나 프레임 워크와 상관 없이 순수 함수로 이뤄지는 경우가 많아요!
| import Loading from "../../components/Loading/Loading"; | ||
|
|
||
| function Items() { | ||
| const [bestLoading, setBestLoading] = useState(true); |
There was a problem hiding this comment.
기존 코드들을 react로 변경하면서 컴포넌트로 분리하는 부분에서 고민이 많았습니다. 비슷한 기능이 들어가는 부분을 분리해서 코드의 가독성을 높이고 싶은데, 한편으로는 재사용이 되지 않는 컴포넌트여도 분리하는게 맞는지 의문이 들었습니다. 명확한 분리 기준이 있을까요?
->
이미 충분히 잘 하셨습니다 🤣
꼭 재사용을 위해 컴포넌트화를 하는 것은 아닙니다 :) 함수로 로직을 추상화하듯이, 컴포넌트로도 jsx와 도메인 로직을 추상화 하실 수 있습니다.
지금 보면 Items 컴포넌트에 bestLoading, allLoading, allItemVisible, bestItemVisible 등의 상태 값들이 다 나와있는데, 이거를 같은 목적을 가진 상태 값과 로직을 한 컴포넌트에 모아보면 어떨까요!?bestLoading, bestItemVisible 등 bestitems에 관련 된 부분은 점부 BestItems로 넣어주고 AllItems와 관련된 부분은 전부 AllItems로 넣어주는 거죠!
function Items() {
return (
return (
<div className="layout">
<SubHeader />
<div className="contents">
<BestItems />
<AllItems />
</div>
</div>
);
)
}그럼 위처럼 정리되겠죠!
만약 BestItems관련된 부분을 찾고 싶으면 BestItems만 찾아가면 되고 반대로 AllItems 관련된 부분을 찾고 싶으면 해당 컴포넌트롤 찾아가면 되겠죠! 또 로직도 나뉘어져 있으니 수정을 해도 서로 영향 받을 일도 거의 없을테구요 :)
There was a problem hiding this comment.
물론 지금은 loading, error를 한 번에 처리하시느냐고 이렇게 된 것일 수도 있는데, 그 경우도 그리 좋은 선택지는 아닙니다.
지금은 best, all 둘 중 하나만 로딩이나 에러가 발생하면 페이지 전체가 영향을 받죠. 영향 받는 범위를 좁히는 것이 유지보수, UX에 좋은 경우가 많습니다.
물론 페이지 전체 에러, 로딩 핸들링이 필요할 때도 있지만요 :)
There was a problem hiding this comment.
정리하자면, 사실 명확한 기준은 없습니다만, 재사용성 이외에도
- 컴포넌트가 하나의 목적, 역할을 가지고 있는지?
- jsx나 상태 값이 너무 복잡해지고 있지는 않는지?
- 이 컴포넌트는 다른 컴포넌트와 무관하게 사용될 수 있는지? 등을 따져보실 수 있습니다!
4 participants
요구사항
Github에 PR(Pull Request)을 만들어서 미션을 제출합니다.
피그마 디자인에 맞게 페이지를 만들어 주세요.
React를 사용합니다
기본
중고마켓
중고마켓 반응형
베스트 상품
Desktop : 4개 보이기
Tablet : 2개 보이기
Mobile : 1개 보이기
전체 상품
Desktop : 12개 보이기
Tablet : 6개 보이기
Mobile : 4개 보이기
심화
주요 변경사항
스크린샷
멘토에게