-
Notifications
You must be signed in to change notification settings - Fork 39
[명지우] Sprint3 #94
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
[명지우] Sprint3 #94
The head ref may contain hidden characters: "Basic-\uBA85\uC9C0\uC6B0-sprint3"
Conversation
GANGYIKIM
left a comment
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.
지우님 3번째 PR 제출 고생하셨습니다~
디자인 감각이 있으셔서 그런가 디자인과 약간 다르지만 반응형 처리를 잘 하신점이 인상깊었습니다!
지원하지 않는 화면 크기를 정하시고 그에 대한 안내를 하는 컴포넌트를 추가하신 점도 좋아요!
다음 미션도 화이팅입니다!
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.
👍 칭찬
잘 어울리는 preview 이미지 좋습니다 👍
| @@ -0,0 +1,28 @@ | |||
| @media screen and (max-width: 374px) and (min-width: 0px) { | |||
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.
💊 제안
이런 경우 min-width를 제거하시는 것이 더 좋을 것 같아요~
| @media screen and (max-width: 374px) and (min-width: 0px) { | |
| @media screen and (max-width: 374px) { |
| @@ -0,0 +1,28 @@ | |||
| @media screen and (max-width: 374px) and (min-width: 0px) { | |||
| body .unsupported-screen { | |||
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.
💊 제안
지금의 경우 태그 선택자와 클래스 선택자를 같이 작성해야할 필요가 있을까요?
아래처럼 클래스 선택자만 작성하셔도 충분할 것 같아요~
불필요하게 css 우선순위를 높이시면 나중에 css를 덮어씌우시거나 관리하기 어려워집니다~
.unsupported-screen {
display: none;
}
@media screen and (max-width: 374px) {
.unsupported-screen {
display: flex;
}
}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.
👍 칭찬
태그 선택자에서 클래스 선택자로 변경하신 것 좋습니다 👍
| footer > div { | ||
| padding: 0 4.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.
❗️ 수정요청
좌우 padding 값이 디자인보다 크게 들어간 것 같아요~
확인 후 수정해보세요!
| placeholder="비밀번호를 입력해주세요" | ||
| class="input-account password" | ||
| /> | ||
| <button class="btn-visibility-on"></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.
❗️ 수정요청
해당 버튼에 type이 명시되어 있지않아, default type인 submit 버튼이 됩니다~
그래서 버튼을 클릭하면 form이 제출되네요! type을 아래처럼 명시해주세요!
| <button class="btn-visibility-on"></button> | |
| <button class="btn-visibility-on" type="button"></button> |
판다마켓 3
🌐 배포 주소 : https://myungjiwoo-pandamarket.netlify.app/
기본 요구사항
체크 리스트 (기본)
공통
랜딩 페이지
로그인, 회원가입 페이지 공통
체크 리스트 (심화)
지난 코드 리뷰
스크린샷
멘토에게