-
Notifications
You must be signed in to change notification settings - Fork 26
[최지은]sprint6 #98
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
base: React-최지은
Are you sure you want to change the base?
[최지은]sprint6 #98
The head ref may contain hidden characters: "React-\uCD5C\uC9C0\uC740-sprint6"
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.
안녕하세요, 지은님!
너무 고생 많으셨어요. 지난번에 피드백 드린 사항들을 잘 반영해 주셨네요. 👍
질문 주신 내용 답변드릴게요.
-
제가 정리를 잘 못해서 그런지..기준이 명확하게 있지 않아서 그런지 지저분해보이는데 제가 느끼고 있는게 맞을까요..?
: 지은님께서 그렇게 느끼신다면, 정리를 할 때가 된 거예요. 이미 '기준이 명확하지 않고', '지저분해 보인다'라는 것을 스스로 느끼고 계시니까요. 이런 기분을 무시하고 기능 구현에만 집중하면, 나중에는 걷잡을 수 없이 수정하기 어려워질 수 있어요. 확장성을 고려해서 고민해보고, 여러 가설들을 세워보면서 고민을 해서 개선해 보세요. 우선은 정확한 뜻 없이 숫자/조건식 등을 그대로 넣고 있다거나, 하나의 함수에 여러 로직이 섞여있는지 등을 검사해 보세요. 🤓 -
snsData를 별도의 컴포넌트로 분리해 내부에서 관리라고 하셔서 일단 SnsList로 만들어서 따로 분리해봤는데 제가 이해를 맞게 잘 한건지 궁금합니다.
: 제가 의도한 것은 SnsList 파일 내에 snsData도 같이 두자는 거였어요. 해당 데이터는 SnsList에서만 쓰이는 데이터기도 하고, snsData 파일이 components 폴더 안에 위치하는 것도 어색하다 느꼈어요. 그래서 따로 컴포넌트를 만들고, 쓰이는 데이터를 가까이 두자는 얘기였습니다. 😊
좋습니다! 아래 피드백을 이어서 봐주세요~🚀
localStorage.setItem( | ||
"accessToken", | ||
"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6MTksInNjb3BlIjoiYWNjZXNzIiwiaWF0IjoxNzU1Nzc0MjIwLCJleHAiOjE3NTU3NzYwMjAsImlzcyI6InNwLXBhbmRhLW1hcmtldCJ9.4OaXBQYM8vVusQW7bw8-H8JnBpP1rApa3z-tXiQ0VAY" | ||
); | ||
|
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.
API Key 또는 Access Token과 같이 개인이 소지해야 할 값이 이렇게 노출이 되어 있으면 매우 위험해요. 이런 값들은 악용이 될 우려가 있어서 절대 오픈 소스로 올리시면 안됩니다.
따라서, accessToken
값은 환경 변수를 이용해 주세요.
또한, 현재 Access Token에는 만료 시간이 있어요. 따라서, 나중에는 Refresh Token 처리 로직도 필요해질 거예요. 이 부분이 잘 이해가 가지 않는다면, 아래 코드잇 강의를 참고해 주세요.
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.
인증을 한 곳에서 처리하는 점이 좋네요. 다만, 토큰이 있을 때 Bearer를 붙이기 보다는 API 요청 시 isAuth, withAuth 등의 파라미터를 통해 인증 API를 구분하는 건 어떨까요? 🤔
.itmes_title { | ||
${textStyles["text-xl-bold"]} | ||
line-height: ${pxToRem(42)}; | ||
margin-bottom: 0.5rem; | ||
} |
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.
오타가 있네요! 🙂
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.
favorit에 e가 빠졌네요. 다른 favorit도 수정이 필요해 보여요.
flex-direction: column; | ||
} | ||
|
||
.close.bnt { |
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.
여기도 오타가 있네요!
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.
images
prop이 배열로 오고있어요. img 태그의 src 속성에 그대로 배열을 넣고 있습니다! 배열의 원소가 하나만 들어오고 있고, jsx가 내부적으로 처리를 해줘서 당장은 문제가 없어보이지만, 나중에 문제가 생길 확률이 크고, src 속성에는 배열이 아닌 스트링 타입이 들어가야해요.
<input | ||
id="search" | ||
className="search_input" | ||
type="text" | ||
placeholder="검색할 상품을 입력해주세요." | ||
onChange={handleChange} | ||
onKeyDown={handleKeyDown} | ||
/> |
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.
onChange
와 value
속성은 항상 같이 붙어다닌다고 생각을 해주세요. 제어 입력과 비제어 입력에 대해서는 아래 글을 참고해 주세요. 😊
export const postFavorite = (id, data) => | ||
apiRequest(`/products/${id}/favorite`, { method: "POST" }); |
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.
사용하지 않는 매개변수가 있네요!
요구사항
Github에 PR(Pull Request)을 만들어서 미션을 제출합니다.
피그마 디자인에 맞게 페이지를 만들어 주세요.
React를 사용합니다
기본
심화
주요 변경사항
스크린샷
멘토에게