-
Notifications
You must be signed in to change notification settings - Fork 21
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 #128
The head ref may contain hidden characters: "Next-\uB098\uC9C0\uC6D0-sprint10"
[나지원] sprint10 #128
Conversation
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.
지원님 이번 스프린트 미션 고생하셨습니다.
if (e.target.value !== "") { | ||
setIsDisabled(false); | ||
return; | ||
} | ||
|
||
setIsDisabled(true); |
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.
P3:
아래코드도 동일동작합니다.
if (e.target.value !== "") { | |
setIsDisabled(false); | |
return; | |
} | |
setIsDisabled(true); | |
const isEmpty = !e.target.value.trim(); | |
setIsDisabled(isEmpty); |
<button | ||
type="button" | ||
className={styles.cancelButton} | ||
onClick={onCancel} | ||
> | ||
취소 | ||
</button> | ||
<Button | ||
className={styles.editButton} | ||
type="submit" | ||
disabled={isDisabled} | ||
> | ||
수정완료 | ||
</Button> |
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.
P3:
취소랑 수정완료랑 왜 다르게 작업하셨을까요? 취소 버튼은 ui에 정의하신 버튼 컴포넌트로 커버가 안될까요? 🧐
} | ||
|
||
const AuthorInfo = ({ className, nickname, image, date }: Props) => { | ||
const AuthorInfo = ({ nickname, date, className, image }: Props) => { |
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.
P3:
const AuthorInfo = ({ nickname, date, className, image }: Props) => { | |
const AuthorInfo = ({ nickname, date, className ='' , image }: Props) => { |
const allowedTypes = ["image/png", "image/jpeg"]; | ||
if (!allowedTypes.includes(nextValue.type)) return; |
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.
P3:
👍
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 HeartButton = ({ favoriteCount, className }: HeartButtonProps) => { | ||
return ( | ||
<button | ||
type="button" | ||
aria-label="좋아요 버튼" | ||
className={`${styles.heartButton} ${className}`} | ||
> |
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.
P3:
아직 필요하지 않으셔서 추가하지 않은 것 같지만 버튼이니 나중에 onClick 함수를 넘길 수 있게 추가해주세요~
const Input = ({ | ||
type = "text", | ||
name = "", | ||
className = "", | ||
label = "", | ||
placeholder = "", | ||
value = "", | ||
onChange = () => {}, | ||
onKeyUp = () => {}, | ||
children, | ||
}: InputProps) => { |
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.
P2:
많은 속성들이 초기화가 필요 없을 것 같아요. name속성도 없을 때 빈 문자열로 초기화해주지 않는것이 더 좋을 것 같고, 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.
그리고 placeholder, value, onKeyup, onChange같이 한번만 사용되는 props들은 나열하지 않고 스프레드 연산자로 받으셔서 전달하셔도 될 것 같습니다.
const Input = ({
name,
className = "",
label = "",
type = "text",
children,
...inputProps
}: InputProps) => {
return (
<Container className={`${styles.container} ${className}`}>
<Label className={styles.label} htmlFor={name}>
{label}
</Label>
<div className={styles.inputContainer}>
<input
className={styles.input}
type={type}
name={name}
id={name}
{...inputProps}
/>
{children}
</div>
</Container>
);
}
}, [refreshAccessToken]); | ||
|
||
return ( | ||
<AuthContext.Provider value={{ accessToken, login, logout }}> |
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.
P2:
멘토링때 말씀드린것처럼 context 사용시 전달되는 value는 자식들의 잦은 리렌더링을 막기위해 useMemo를 감싸 전달합니다.
관련해서 읽으실만한 글 링크드립니다~
요구사항
기본
상품 등록 페이지
상품 상세 페이지
심화
스크린샷
배포 페이지: https://najimarket.vercel.app/boards
상품 등록 페이지
상품 상세 페이지
멘토에게