[최일우] sprint6#113
Hidden character warning
Conversation
2ruuuu
left a comment
There was a problem hiding this comment.
다음부터는 전체적으로 틀을 그려보고 시작해야할 것 같습니다 prop 꼬이는게 쉽지 않네요...
There was a problem hiding this comment.
AddItemInput 에서 모든 프롭으로 넘기는 부분입니다!
|
스프리트 미션 하시느라 수고 많으셨어요. |
|
코드에 궁금한 부분 남겨 놓겠습니다! 굿굿 ! 👍👍 한 번 확인해볼게요 ! |
|
|
||
| const axiosInstance = axios.create({ | ||
| baseURL: "https://panda-market-api.vercel.app", | ||
| baseURL: import.meta.env.VITE_BASE_URL, |
|
|
||
| return ( | ||
| <InputContainer> | ||
| <Label>{children}</Label> |
There was a problem hiding this comment.
children으로 라벨을 적용하는 것은 예상하기 어려울 것 같군요 !
children은 보통 button 사이에 넣거나 form 사이에 넣거나 Container 사이에 넣는 등 개발자 경험적으로 예상이 되는 곳에 children을 적용하는 것이 올바라보입니다 !
| value, | ||
| onChange, | ||
| }) => { | ||
| const isLong = height === 'long'; |
There was a problem hiding this comment.
객체를 활용해서 매핑해볼 수 있겠네요 !
현재는 height === 'long'으로 분기하고 있는데,
이런 경우 객체로 매핑해두면 확장성과 가독성이 더 좋아질 수 있어요.
const HEIGHT_MAP = {
short: '40px',
long: '80px',
};| const isLong = height === 'long'; | |
| const inputHeight = HEIGHT_MAP[height]; |
There was a problem hiding this comment.
다만, height보다는 variant 혹은 type으로 분기해주는게 좋겠네요 !
혹은. as를 통해서 textarea를 지정하는 방법도 있겠어요 !
현재 height === 'long'일 때 textarea로 바뀌고 있는데, 사실 이건 단순히 “높이”의 문제가 아니라 입력 타입이 바뀌는 것에 가깝습니다.
그럼 어떻게?:
const AddItemInput = ({
variant = 'input', // 'input' | 'textarea'
...
}) => {
const isTextarea = variant === 'textarea';
return (
<Input
as={isTextarea ? 'textarea' : 'input'}
...
/>
);
};as를 그대로 전달하는 방법:
const AddItemInput = ({
as = 'input',
...
}) => {
return <Input as={as} ... />;
};두 가지 방법이 있을 것 같군요. 😉
핵심은 "높이 변경"으로 오해할 여지가 있으며, 사실은 "입력 타입 자체가 변경 됨"입니다 !
| onChange, | ||
| }) => { | ||
| const isLong = height === 'long'; | ||
| const tagInput = children === '태그'; |
There was a problem hiding this comment.
컴포넌트의 역할이 텍스트 값에 의존될 수 있겠군요.
앞서 말씀드린 것처럼 children을 식별자로 사용되는 것보다는 별도의 props를 받는게 좋겠습니다 😁
| {tagInput ? ( | ||
| <TagContainer> | ||
| {tag.map((title) => ( | ||
| <Tag key={title} handleTagDelete={handleTagDelete} tag={tag}> | ||
| {title} | ||
| </Tag> | ||
| ))} | ||
| </TagContainer> | ||
| ) : null} |
There was a problem hiding this comment.
참일 경우만 고려하는거라면 &&를 사용해볼 수 있습니다 !:
| {tagInput ? ( | |
| <TagContainer> | |
| {tag.map((title) => ( | |
| <Tag key={title} handleTagDelete={handleTagDelete} tag={tag}> | |
| {title} | |
| </Tag> | |
| ))} | |
| </TagContainer> | |
| ) : null} | |
| {tagInput && ( | |
| <TagContainer> | |
| {tag.map((title) => ( | |
| <Tag key={title} handleTagDelete={handleTagDelete} tag={tag}> | |
| {title} | |
| </Tag> | |
| ))} | |
| </TagContainer> | |
| )} |
왼쪽이 true면 오른쪽 값을 그대로 반환합니다 !
그리고 왼쪽이 false면 false를 반환하며, React는 false를 렌더링하지 않으니 결과적으로 아무것도 안 그려지게 됩니다. 편리하죠? 😉
반대로
||는false를 표현하고 싶을 때 사용해볼 수 있습니다 !
일례로 지금 상황은&&대신!tagInput || ...로도 표현할 수 있단는거지요 ! (앞에!(NOT)이 들어갔습니다!)
| return <option value={value}>{children}</option>; | ||
| }; | ||
|
|
||
| DropDown.Option = Option; |
There was a problem hiding this comment.
오호. 저번에 피드백 드린 패턴을 사용해보셨군요 👍👍
| <Label htmlFor="imageRegist"> | ||
| <img src={plusIcon} alt="이미지 추가 아이콘" /> | ||
| <P>이미지 등록</P> | ||
| </Label> |
There was a problem hiding this comment.
단순 장식용인 이미지의 경우 alt를 ""로 나타낼 수 있습니다 !
| <Label htmlFor="imageRegist"> | |
| <img src={plusIcon} alt="이미지 추가 아이콘" /> | |
| <P>이미지 등록</P> | |
| </Label> | |
| <Label htmlFor="imageRegist"> | |
| <img src={plusIcon} alt="" /> | |
| <P>이미지 등록</P> | |
| </Label> |
장식 이미지는 페이지 콘텐츠에 정보를 추가하지 않습니다. 예를 들어, 이미지에서 제공하는 정보는 인접한 텍스트를 사용하여 이미 제공될 수도 있고, 웹 사이트를 시각적으로 더욱 매력적으로 만들기 위해 이미지가 포함될 수도 있습니다.
이러한 경우 스크린 리더와 같은 보조 기술에서 무시할 수 있도록 null(빈) alt텍스트를 제공해야 합니다( ). alt=""이러한 유형의 이미지에 대한 텍스트 값은 화면 판독기 출력에 청각적 혼란을 추가하거나 주제가 인접한 텍스트의 주제와 다른 경우 사용자의 주의를 산만하게 할 수 있습니다. 속성 을 생략하는 alt것도 옵션이 아닙니다. 속성이 제공되지 않으면 일부 화면 판독기가 이미지의 파일 이름을 대신 알려주기 때문입니다.
지금 같은 경우, 바로 옆에 "이미지 등록" 이라는 충분한 설명이 있는 것으로 보여서 첨부드려 봅니다. 😉
| const location = useLocation(); | ||
|
|
||
| const marketActive = | ||
| location.pathname === "/items" || location.pathname === "/additem"; |
There was a problem hiding this comment.
화이트리스트로 표현하면 관리가 용이하겠군요 !
path가 추가될 수록 해당 조건문이 늘어나게 됩니다. 즉. "특정 경로들을 필터링해줄 필요"가 있겠군요 !
이럴 때는 화이트리스트를 통해서 문제를 해결할 수 있어요 !:
const MARKET_PATHS = ["/items", "/additem"];
const marketActive = MARKET_PATHS.includes(location.pathname);| const [description, setDescription] = useState(''); | ||
| const [price, setPrice] = useState(''); | ||
|
|
||
| const isActive = file && name && description && price; |
There was a problem hiding this comment.
굿굿 조건식에 별칭을 붙여서 가독성이 더 좋아졌네요 👍👍
|
수고하셨습니다 일우님 ! 앞으로도 일우님에게는 어려운 부분도 적극적으로 피드백드려도 되겠어요 😉 이번 미션도 정말 수고 많으셨습니다 ! |
요구사항
기본
심화
주요 변경사항
스크린샷
주강사님에게
코드에 궁금한 부분 남겨 놓겠습니다!
sprint5 피드백 부분 수정했습니다!