[김정우] Sprint3 #167
[김정우] Sprint3 #167
Conversation
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
정우님 3번째 미션 제출 고생하셨습니다!
반응형이 쉽지 않은데 잘 작업하셨어요.
다음 4번째 미션 제출도 화이팅입니다~
- 다음 PR에는 구현하시 요구사항과 배포주소를 적어주시면 좋을 것 같습니다!
제가 네비게이션을 넣어야 하는 걸 못봐서 이번에 넣어보려고 했습니다만, 어떻게 넣어도 예쁘게 추가가 되지 않아서 추가하지 않았습니다. 픽스드를 쓰면 원래 영역이 사라져서 sticky를 넣어보기도 했지만, 원하는 결과가 나오지 않고 자꾸 안이쁘게 적용이 됩니다. 네비게이션에 미디어쿼리를 적용할 때도 제가 생각한 것처럼 되지 않고, 화면이 무너져 내렸습니다. 처음부터 설계를 잘못한 건지 궁금합니다.:
어떻게 작성하셨는지를 알아야 답변이 가능할 것 같습니다~ 말씀해주신 네비게이션 상단 고정 요구사항의 경우 코멘트 남겨드렸으니 확인해보세요~
There was a problem hiding this comment.
👍 칭찬
깃에 올라가지 않을 파일을 관리해주시는 것 좋습니다~
| padding-left: 20px; | ||
| } | ||
|
|
||
| @media (max-width: 767px) and (min-width: 375px) { |
There was a problem hiding this comment.
💊 제안
min-width가 조건에 걸려있어 375px 미만으로 화면이 작아지게 되면 PC 스타일이 적용되네요~
body에 min-width: 375px를 추가하셔서 해당 사이즈 미만으로 작아지게 되면 가로 스크롤이 생기게 해주시거나
아니라면 min-width를 조건문에서 빼주시는 것을 추천드려요~
There was a problem hiding this comment.
👍 칭찬
공통으로 사용되는 스타일들 따로 빼주신 점 좋아요!
| <label for="userName" class="userName-label">이메일</label> | ||
| <input | ||
| type="email" | ||
| name="userName" | ||
| id="userName" |
There was a problem hiding this comment.
👍 칭찬
label과 Input을 연결해주신 점 좋습니다~
| <label for="checkPassword" class="checkPassword-label" | ||
| >비밀번호 확인</label | ||
| > |
There was a problem hiding this comment.
💊 제안
현재 코드에서는 하나의 태그가 나눠져 있어 가독성이 조금 떨어질 수 있습니다.
태그 간 계층 구조를 명확하게 표현하기 위해 아래처럼 작성하시는 것을 추천드려요~
| <label for="checkPassword" class="checkPassword-label" | |
| >비밀번호 확인</label | |
| > | |
| <label for="checkPassword" class="checkPassword-label"> | |
| 비밀번호 확인 | |
| </label> |
| .contents-reverse { | ||
| display: flex; | ||
| flex-direction: column; | ||
| justify-content: center; | ||
| } |
There was a problem hiding this comment.
❗️ 수정요청
index 페이지의 해당 요소가 디자인대로 보일 수 있도록 이미지와 동일한 width를 주시는 것이 좋을 것 같아요.
.contents와 .contents-reverse에게 아래 속성을 추가해주시는 것을 추천드려요.
| .contents-reverse { | |
| display: flex; | |
| flex-direction: column; | |
| justify-content: center; | |
| } | |
| .contents, .contents-reverse { | |
| display: flex; | |
| flex-direction: column; | |
| justify-content: center; | |
| max-width: 560px; // 최대 너비는 이미지와 동일하게 설정 | |
| width: 100%; | |
| } | |
| <div class="header-container"> | ||
| <a class="login-link" href="/"> | ||
| <img class="title-logo" src="img/logo.png"> | ||
| <img class="title-logo" src="img/logo.png" /> |
There was a problem hiding this comment.
💊 제안
모바일 디자인 상에서 로고의 경우 판다 얼굴이 사라지고 "판다마켓"이라는 글자만 남게 됩니다~
확인해보시고 디자인대로 수정해보세요~
| .advice { | ||
| font-size: 14px; | ||
| font-weight: 500; | ||
| } | ||
|
|
||
| .membership { |
There was a problem hiding this comment.
💊 제안
디자인상에서는 이 두 태그 사이의 간격이 있습니다~
4px 정도 여백을 주셔서 디자인과 동일하게 구현해보세요!
| <link rel="stylesheet" href="global.css" /> | ||
| </head> | ||
| <body> | ||
| <header> |
There was a problem hiding this comment.
💊 제안
PR에 적어주신 것처럼 header를 상단에 고정시키기 위해서는 아래처럼 작성해주시면 됩니다.
제 예시코드처럼 태그 선택자를 사용하시기보다 태그에 클래스를 추가하셔서 작성하시는 것을 추천 드립니다.
header {
background-color: #fff;
position: sticky;
top: 0;
}지금 작성하신 css를 봤을 때 header 안의 자식태그인 .container의 경우 좌우 margin이 있어 디자인대로 구현되지 않을 가능성이 커 header 태그에 위와 같이 작성해주시면 됩니다~
2 participants
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게