[유용민] sprint1 #14
[유용민] sprint1 #14
Conversation
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
용민님 첫번째 PR 수고하셨습니다~
리뷰 드린 코멘트는 모두 필수 반영사항은 아니고 코드에 정답은 없으니
보고 마음에 드시는 방향으로 수정하시면 좋겠습니다!
다음 주차도 화이팅입니다~
-
figma에 있는 px값으로 css디자인을 진행하였더니, 화면이 줄어듦에 따라서 글씨 배열이 원하는 대로 나오지 않아서 em이나 rem값을 써야할지 생각했습니다.:
코멘트에서도 언급했지만 figma에 있는 속성값들은 디자인 파일을 보고 생성된 값일뿐 그것이 디자인 구현을 위한 최적화 값은 아닙니다. 따라서 해당값과 속성을 그대로 사용하면 왜 그러한 값이 필요한지 이해하기 어렵고 따라서 유지보수에 좋지 않습니다. 가능하면 figma에서는 디자인만 보시고 스스로 구현하시는 것을 추천드려요! 또한 추후 미션을 진행함에 따라 반응형 디자인이 제공되므로 우선은 요구사항만 구현하시고 나중에 반응형 디자인 구현하실때, 디자인에 따라 수정하시는 것이 좋을 것 같습니다! -
"화면의 너비가 1920px 이상이면 하늘색 배경색은 너비를 꽉 채우도록 채워지고, 내부 요소들의 위치는 고정되고, 여백만 커지도록 합니다." 조건과 "화면의 너비가 1920px 보다 작아질 때, "판다마켓" 로고의 왼쪽 여백 200px"로그인" 버튼의 오른쪽 여백 200px이 유지되고, 화면의 너비가 작아질수록 두 요소간 거리가 가까워지도록 설정합니다." 라는 조건을 나름대로 표현해봤는데, 제대로 잘 작동하는 건지 아직잘 모르겠습니다.: 저도 용민님과 동일하게 이해했습니다. 잘구현하셨어요~
There was a problem hiding this comment.
❗️ 수정요청
해당 이미지 파일의 크기를 보면 588× 444로 되어 있습니다.
저희는 반응형 사이트를 만들고 있고, 이런 이미지들은 모바일에서도 사용할 것이기 때문에 추출하실때, 1배수가 아닌 2배수로 추출해주시는 것이 화질면에서 좋습니다.
아래처럼 figma export 탭에서 2x 로 설정하고 추출해주세요~
There was a problem hiding this comment.
💊 제안
한 레포에서는 같은 룰을 공유하는 것이 좋습니다. 따라서 이미지명도 같은 규칙을 기반으로 명명하는 것이 좋아요.
다른 이미지들처럼 소문자로 시작하는 파일명인 img_home_01.png 로 변경하시는 것을 추천드려요~
| @@ -0,0 +1,115 @@ | |||
| <!DOCTYPE html> | |||
| <html lang="en"> | |||
There was a problem hiding this comment.
❗️ 수정요청
html lang 속성은 스크린리더, 검색 엔진등에게 해당 사이트가 어떤 언어 기반으로 작성되었는지 설명하는 속성입니다.
en은 english라는 의미이니 ko로 변경해주세요!
| <html lang="en"> | |
| <html lang="ko"> |
| <header> | ||
| <nav> | ||
| <a href="/"><img src="img/logo.png" /></a> | ||
| <a href="/login"> <button>로그인</button></a> |
There was a problem hiding this comment.
❗️ 수정요청
로그인 버튼을 눌렀을 때 './login.html'로 이동시켜주는 역할을 하니, 앵커 태그만 남겨두셔도 될 것 같습니다~
버튼 요소처럼 보여도 꼭 버튼 태그로 작성하실 필요는 없습니다!
해당 태그의 역할에 따라 태그를 작성하시는 것이 더 시맨틱하게 작성하는 방법입니다.
또한 지금 단계에서 페이지 이동을 구현하시기 위해서는 확장자를 포함시켜주셔야 합니다.
관련 이슈를 첨부해드리니 한번 확인해보시면 좋을 것 같습니다.
| <a href="/login"> <button>로그인</button></a> | |
| <a href="/login.html">로그인</a> |
There was a problem hiding this comment.
💬 여담
구현을 완료하신 후에는 작업하신 사항들이 잘 구현되엇는지, a tag, button tag들을 눌러보시면서 확인하시면 좋습니다~
|
|
||
| body { | ||
| font-family: "Pretendard-Regular"; | ||
| width: 100vw; |
There was a problem hiding this comment.
❗️ 수정요청
배포사이트에 들어가보시면 가로 스크롤이 생긴걸 확인할 수 있습니다. 이는 100vw 때문이에요~
해당 속성을 제거하셔도 body 태그는 따로 display 속성을 주지 않으셔도 block 요소처럼 동작하므로 해당 속성은 지우셔도 됩니다.
footer 태그에 있는 width: 100vw도 같이 지우시면 가로 스크롤이 없는 것을 확인하실 수 있습니다~
https://dev.to/tepythai/why-100vw-causes-horizontal-scrollbar-4nlm
| font-family: "Pretendard-Regular"; | ||
| src: url("https://fastly.jsdelivr.net/gh/Project-Noonnu/[email protected]/Pretendard-Regular.woff") | ||
| format("woff"); |
There was a problem hiding this comment.
💊 제안
용민님 폰트 잘 적용하셨습니다.
다만 최적화를 위해 가변 글꼴이나 다이나믹 폰트를 사용하시는 것이 좋습니다.
지금 import 하신 폰트 명을 보면 Pretendard-Regular라고 되어 있습니다.
즉 regular, 400에 해당하는 굵기의 폰트파일이라는 의미입니다.
이 폰트 파일을 기반으로 bold, semi bold와 같이 font-weight를 조정하셔도 되지만
그것이 Pretendard-Bold, Pretendard-SemiBold와 동일한 결과값이라고 보장할 수는 없습니다.
따라서 Pretendard Variable과 같이 가변 굵기용 폰트를 쓰시는 것을 추천드립니다.
이는 최적화를 위한 제안이므로 시간이 되실때 적용해보시면 좋을 것 같습니다~
| button { | ||
| border: none; | ||
| } | ||
|
|
||
| a { | ||
| text-decoration: none; | ||
| color: inherit; | ||
| } |
There was a problem hiding this comment.
💊 제안
브라우저 기본 스타일을 초기화하는 코드이므로 reset.css에 들어가는게 더 좋아보여요~
| width: 1120px; | ||
| color: #ffffff; | ||
| font-size: 16px; | ||
| line-height: 19.09px; |
There was a problem hiding this comment.
💊 제안
해당 line-height 값을 아마 figma에서 보고 추가하셨을 것 같아요.
다만 아래 속성값은 사실 의미가 없을 가능성이 큽니다.
피그마는 디자인을 보고 css 값들을 나열하고, 해당 값이 실제로 구현하는데 필수적인지는 알 수 없습니다.
그래서 피그마 속성값을 그대로 쓰시기보다 참고만해주세요~
그리고 line-height 값이 필요하시더라도 정수로 사용하시는 것이 좋습니다~
| line-height: 19.09px; | |
| line-height: 19px; |
There was a problem hiding this comment.
👍 칭찬
용민님이 생각하시기에 목적에 따라 파일 분리해서 작성하신 점 좋습니다~
2 participants
요구사항
UI 디자인 기초 토픽을 수강해 보세요.
피그마 디자인에 맞게 페이지를 만들어 주세요.
React와 같은 UI 라이브러리를 사용하지 않고 진행합니다.
HTML, CSS 파일을 Netlify로 배포해 주세요. (https://fe-sprint1-yongmin.netlify.app/)
PC사이즈만 고려해 주어진 디자인으로 구현합니다.
기본
심화
주요 변경사항
스크린샷
멘토에게
figma에 있는 px값으로 css디자인을 진행하였더니, 화면이 줄어듦에 따라서 글씨 배열이 원하는 대로 나오지 않아서 em이나 rem값을 써야할지 생각했습니다.
"화면의 너비가 1920px 이상이면 하늘색 배경색은 너비를 꽉 채우도록 채워지고, 내부 요소들의 위치는 고정되고, 여백만 커지도록 합니다." 조건과 "화면의 너비가 1920px 보다 작아질 때, "판다마켓" 로고의 왼쪽 여백 200px"로그인" 버튼의 오른쪽 여백 200px이 유지되고, 화면의 너비가 작아질수록 두 요소간 거리가 가까워지도록 설정합니다." 라는 조건을 나름대로 표현해봤는데, 제대로 잘 작동하는 건지 아직잘 모르겠습니다.
셀프 코드 리뷰를 통해 질문 이어가겠습니다.