[이태식] sprint1 #32
[이태식] sprint1 #32
Conversation
bold, semibold, medium, 4xl, 2xl, xl, 2lg 추가
a태그의 text-decoration을 none으로 지정해주었습니다. btn-login, btn-large를 추가했습니다.
1920px이상일 때 내부 요소들의 위치를 고정하고 양 옆에 여백을 주는 wrap추가, gnb완성 media를 통해 1920px보다 작을 때 양 옆 여백을 200px로 변경
home-top-content padding-bottom 수정 필요
반응형 디자인을 위해 px단위를 rem으로 변경하고 @media를 추가했습니다.
There was a problem hiding this comment.
태식님 첫번째 PR 제출 고생하셨습니다!
리뷰 드린 코멘트는 모두 필수 반영사항은 아니고 코드에 정답은 없으니
보고 마음에 드시는 방향으로 수정하시면 좋겠습니다!
다음 주차도 화이팅입니다~
-
처음에 px단위로 진행 하다가 심화 과정을 위해 rem단위로 전부 변경 하였는데 모든 길이의 단위를 rem으로 해도 되는건지 궁금합니다.:
저는 코드에 정답이 없다고 생각합니다. 구현 요구사항을 만족했다면 가능한 방법이라 생각해요. 다만 모든 단위를 rem을 쓰는게 좋은지 혹은 효율적인지는 모르겠습니다. 반응형 단위는 rem말고 다양하게 존재하니 rem을 쓰자고 정해두지 마시고 특정 상황에 적절한 단위를 사용하시는 것이 더 좋아보여요~ -
레이아웃을 만들 때 grid가 아닌 flex를 겹쳐서 여러번 사용하였는데 grid를 사용하는게 좋은지 궁금합니다.:
grid와 flex 는 특정 요소의 display 속성으로 배치를 위한 값일뿐이라고 생각합니다~ 또한 flex 내부에 flex요소를 사용하는 경우는 왕왕있습니다. 따라서 요구사항에 따라 flex와 grid 중 더 적절한 값을 사용하시면 됩니다. 현재 디자인상에서는 grid와 flex 중 어떤 것을 사용하던 큰 차이가 없을 것 같습니다. -
커밋을 할 때 약간의 수정이더라도 바로 해야하는지 궁금합니다. 같은 파일에서 a라는 작업을 하던 도중 b 의 해결 방법이 떠올라서 b를 먼저 완성하게 되면 a까지 완성해서 a,b를 동시에 커밋했는데 이래도 되는건지 궁금합니다.:
이건 해당 레포의 commit 룰에 따라 달라질 것 같습니다. 만약 작업자들끼리 최대한 나눠서 commit 하기로 정했다면 안되는 것이고, 그게 아니라면 문제가 없습니다. 지금은 태식님 혼자 작업하시는 환경이니 commit 단위를 어떻게 할 것인지 태식님께서 정해서 지키시면 되겠습니다~ 다만 학습 과정이시니 단위를 쪼개 commit 하는 연습을 하는 것이 좋습니다. -
겹치는 폰트가 많아 피그마를 보고 글자 클래스와 두께 클래스를 여러개 만들어서 클래스를 여러개씩 사용했는데 한 태그에 클래스를 여러개 사용해도 괜찮은지 궁금합니다. 만약 여러개 만들어도 괜찮다면 이번 미션에서는 이번 미션에서 사용할 폰트만 만들었는데 미래에 사용할 폰트도 미리 만들어 두는게 좋은지 궁금합니다.:
우선 위와 같은 방식을 utility 클래스 방식이라고 합니다. 관련한 라이브러리로 tailwind라는 친구가 있습니다. 이미 존재하는 방식으로 이렇게 관련 클래스들을 미리 선언하시고 태그에 여러개의 클래스를 주셔도 됩니다. 디자인에서 반복적으로 사용되는 컬러값, 폰트값들이 figma 최상단 페이지에 정의되어 있으니 이를 보고 미리 클래스를 선언해두셔도 됩니다.
다만 추후 반응형 디자인을 하실때 어떻게 하실지 같이 고민해보시는 것이 좋을 것 같습니다.
관련해서 위에서 말씀드린 tailiwind를 참고하시면 좋을 것 같아요~
There was a problem hiding this comment.
❗️ 수정요청
중복되는 이미지가 크기별로 존재하네요. 사이별로 이미지가 존재해야할 필요가 없다면 이미지 하나로 사용하시면 좋을 것 같습니다.
한 이미지가 다양한 크기로 사용될 때, 예를 들어 160x200 / 80x100 사이즈로 로고가 사용된다면, 160x200으로 추출해서 작은 사이즈의 경우 css를 통해 크기를 조절해서 사용하시면 될 것 같습니다.
There was a problem hiding this comment.
💊 제안
한 레포안에서는 동일한 룰을 따르는 것이 좋습니다.
따라서 img_home_01.png와 같은 식으로 같은 네이밍 룰을 따르시는 것을 추천드려요~
There was a problem hiding this comment.
💬 여담
figma에서 그때 그때 필요한 이미지만 추출하시는 것을 추천드려요~
아마 같이 사용하는 디자인 파일이라 export 탭에서 export 처리된 친구들을 다 추출하게 되시면
불필요한 친구들도 같이 딸려올 가능성이 큽니다!
There was a problem hiding this comment.
❗️ 수정요청
어떤 파일인지 알 수 있게 네이밍하시는 것이 좋아요.
또한 이미지명에 공백이 들어가 있는데 이는 예기치 않은 동작을 발생시킬 수 있습니다. 이러한 공백은 자동으로 공백을 의미하는 대체 문자열로 변환되기 때문입니다.
그래서 일반적으로 공백대신 _ -를 이용해 Property_1=sm, Property-1=sm 같은 식으로 공백을 표현합니다.
태식님의 다른 이미지 파일명으로 보면 _를 공백대신 사용하시면 되겠네요~
| <div class="home-bottom wrap"> | ||
| <div class="home-bottom-content"> | ||
| <p class="text-4xl bold"> | ||
| 믿을 수 있는 <br> |
There was a problem hiding this comment.
❗️ 수정요청
지금 디자인에서 괜찮을 것 같지만 공백은 의도치 않은 동작을 발생시킬 수 있습니다~
필요한 공백이 아니면 가능하면 습관적처럼 없애는 것을 추천드려요~
| 믿을 수 있는 <br> | |
| 믿을 수 있는<br> |
| <div class="sns"> | ||
| <a href="https://facebook.com" target="_blank"> | ||
| <img src="./images/ic_facebook.png" alt="페이스북"> | ||
| </a> | ||
| <a href="https://twitter.com" target="_blank"> | ||
| <img src="./images/ic_twitter.png" alt="트위터"> | ||
| </a> | ||
| <a href="https://youtube.com" target="_blank"> | ||
| <img src="./images/ic_youtube.png" alt="유튜브"> | ||
| </a> | ||
| <a href="https://instagram.com" target="_blank"> | ||
| <img src="./images/ic_instagram.png" alt="인스타그램"> | ||
| </a> | ||
| </div> |
There was a problem hiding this comment.
💊 제안
이 영역은 로고들의 모음이라고 할 수 있습니다. 관련된 요소들끼리 묶인 목록이니 의미록적인 마크업을 작성한다면 list 관련 태그로 작성하시는 것을 추천드립니다.
<ul class="sns">
<li>
<a href="https://facebook.com" target="_blank">
<img src="./images/ic_facebook.png" alt="페이스북">
</a>
</li>
</ul>
| * { | ||
| box-sizing: border-box; | ||
| font-family: Pretendard, sans-serif; | ||
| color: #374151; | ||
| font-size: 16px; | ||
| } | ||
|
|
||
| a { | ||
| text-decoration: none; | ||
| cursor: pointer; | ||
| } | ||
|
|
||
| p { | ||
| margin: 0; | ||
| } |
There was a problem hiding this comment.
💊 제안
간략한 reset css 속성들이라 따로 분리하지 않으시고 한 파일에 작성하셨나봐요~
하지만 필요시 특정 태그의 스타일을 초기화하는건 번거로운 일이고, 이를 염두에 두고 css 작성을 해야하니 가능하면 처음부터 브라우저 스타일을 초기화하시고 reset.css로 분리하시고 작업하시는 것을 추천드립니다!
| @@ -0,0 +1,235 @@ | |||
| * { | |||
| box-sizing: border-box; | |||
| font-family: Pretendard, sans-serif; | |||
There was a problem hiding this comment.
💊 제안
font-family는 상속되는 속성이므로, 모든 태그에 개별적으로 지정하기보다 html이나 body 같은 상위 태그에 한 번만 적용하는 것이 더 효율적입니다. 이렇게 하면 하위 태그들이 자연스럽게 상속받아 일관된 스타일을 유지할 수 있어요.
비슷한 속성인 font-size도 같은 방식으로 적용하는 것을 추천드려요~
| .bold { | ||
| font-weight: 700; | ||
| } | ||
|
|
||
| .semibold { | ||
| font-weight: 600; | ||
| } | ||
|
|
||
| .medium { | ||
| font-weight: 500; | ||
| } | ||
|
|
||
| .text-4xl { | ||
| font-size: 2.5rem; | ||
| line-height: 3.5rem; | ||
| letter-spacing: -0.125rem; | ||
| } | ||
|
|
||
| .text-2xl { | ||
| font-size: 1.5rem; | ||
| line-height: 2rem; | ||
| letter-spacing: -0.0625rem; | ||
| } | ||
|
|
||
| .text-xl { | ||
| font-size: 1.25rem; | ||
| line-height: 2rem; | ||
| } | ||
|
|
||
| .text-2lg { | ||
| font-size: 1.125rem; | ||
| line-height: 1.625rem; | ||
| } |
There was a problem hiding this comment.
💊 제안
이런 유틸 클래스들은 따로 파일을 빼시는 것을 추천드려요~
관심사 분리 원칙에 따라 관련있는 친구들만 모아두시는 것이 가독성 및 유지보수에 용이하답니다~
지금 태식님 구조에서 제가 추천드리는 파일 분리 구조는 아래와 같아요~
- reset.css : 브라우저 기본 스타일을 초기화하는 코드
- utility.css/common.css : 자주사용하는 스타일을 위한 클래스들이 모여있는 곳
- [page명].css : index.css, login.css 와 같이 페이지별 스타일이 모여있는 곳
2 participants
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게