Conversation
There was a problem hiding this comment.
이야 전반적으로 너무 좋은 구조로 작성된 프로젝트네요!
조금 더 욕심내서 구조적으로 가져가본다면 금방 HTML CSS 구조에 대해서는 익숙하게 구현될 것 같아요.
리뷰 내용에 앞서 질문 사항에 대해 먼저 답변 드리면요,
�grid도 Flex도 레이아웃을 지정하기 위한 스타일 프로퍼티에요.
따라서 grid에 minmax가 있는것처럼, flex에는 shrink, basis가 있답니다.
이 요소들을 잘 활용해서 어느정도 구역을 가져갈 지 정해본다면 좋을 것 같아요!
코드 전반적으로 굉장히 잘 구조가 잡혀있어요.
이제 몇가지만 더 개선한다면 정말 좋은 코드가 될 것 같아 기대가 되네요!
아직 css 네이밍이 미숙한듯 한데요, 이에 대해서는 코드 단의 리뷰에 피드백 남겨두었으니
한번 참고해서 진행해보시면 좋겠습니다!
너무 고생 많으셨고, 추가 질문사항이 더 있으시다면 디스코드에 언제든 질문 남겨주세요!
| <img | ||
| class="content--img" | ||
| src="./img/landing/img_2.png" | ||
| alt="랜딩페이지 두번째 이미지" |
There was a problem hiding this comment.
alt텍스트의 경우, 사용자 친화적인 메시지가 담기는게 좋아요~
해당 이미지가 무엇을 표현하는지 글로 풀어 설명해볼까요?
| * { | ||
| box-sizing: border-box; | ||
| margin: 0; | ||
| font-family: 'pretendard'; |
There was a problem hiding this comment.
Font Family의 경우, fallback 폰트들도 함께 지정해주는게 좋아요 !
특히, 위 네트워크 요청을 통해 폰트를 가져오고 있죠?
만약 네트워크 환경이 좋지 않은곳에서 앱에 접근하거나,
네트워크 오류로 인해 해당 폰트 데이터를 불러오지 못하는 경우가 발생할수도 있잖아요.
그래서 위와 같은 이유로 폰트를 가져오지 못했을 때, 모든 운영체제들이 기본적으로 지닐법한 요소들을
fallback font fmaily로 설정해주는게 좋아요!
There was a problem hiding this comment.
serif , sanserif 같은 폰트를 말씀하시는 건가요?
| margin: 0; | ||
| font-family: 'pretendard'; | ||
| font-weight: var(--font-normal); | ||
| background-color: var(--color-gray-100); |
There was a problem hiding this comment.
inline element들의 경우, bg color가 존재하지 않는 프로퍼티인데, css reset단에서 설정하게 된다면
불필요한 CSSOM을 만들게 될 것 같아요!
또한 어떤 요소들은 기본 백그라운드 색이 달라야 할 수 있을것 같은데,
Body에 백그라운드 색을 지정하고, 이를 Inherit받아오도록 활용하는게 조금 더 좋아보입니다!
There was a problem hiding this comment.
아하, 불필요하게 CSS 를 지정했었네요...
수정하겠습니다!
| .form { | ||
| display: flex; | ||
| flex-direction: column; | ||
| align-items: center; | ||
| justify-content: center; | ||
| } |
There was a problem hiding this comment.
무슨 폼인지 조금 더 명시적이면 좋겠어요!
.login-form
.login-form .header
.login-form .header--title ....
There was a problem hiding this comment.
맞네요... 로그인 폼이라는 것을 글자만 보더라도 알 수 있도록 수정하겠습니다!
| .button { | ||
| border: none; | ||
| background-image: linear-gradient( | ||
| 90deg, | ||
| var(--color-primary), | ||
| var(--color-skyblue) | ||
| ); | ||
| color: var(--color-white); | ||
| padding: 16px 20px; | ||
| margin: 10px; | ||
| border-radius: 8px; | ||
| cursor: pointer; | ||
| } |
There was a problem hiding this comment.
기본 버튼의 경우, signin 페이지 뿐만 아니라 다른 페이지에서도 활용될 수 있겠죠?
따라서 이와 같이 공통적으로 활용하게 되는 콤포넌트성 스타일이라면
/styles/common.css 와 같은 폴더에서 관리를 해본다면 어떨까요?
| /* CSS reset */ | ||
| * { | ||
| box-sizing: border-box; | ||
| margin: 0; | ||
| font-family: 'pretendard'; | ||
| } |
There was a problem hiding this comment.
CSS 리셋고 ㅏ같이 모든 페이지에 공통적으로 활용되어야 하는 요소들은 외부로 분리해 사용하는건 어떨까요?
예를 들어, css reset 과 style variable등이 선언된 파일들, 그리고 공통 UI 스타일에 대해서는
/styles/common.css
/styles/reset.css
/styles/ui.css
와 같은 파일을 만들어 활용할 수 있다면 좋겠어요!
There was a problem hiding this comment.
와... 이런 방식은 생각을 못했었는데, 감사합니다!
| h1 { | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| font-weight: var(--font-bold); | ||
| font-size: 64px; | ||
| line-height: 80px; | ||
| font-family: 'pretendard'; | ||
| color: var(--color-black); | ||
| } |
There was a problem hiding this comment.
아무래도 전역 선언이 되다보니, 해당 페이지에만 스타일이 적용될 수 있도록 클래스이름을 활용해볼까요?
| .contents-01 { | ||
| display: flex; | ||
| flex-direction: column; | ||
| align-items: center; | ||
| background-color: var(--color-gray-100); | ||
| text-align: center; | ||
| margin-top: 93px; | ||
| } |
There was a problem hiding this comment.
메인 페이지의 섹션의 경우 동일한 스타일을 지니고 있죠.
따라서 각 섹션별로 .contents-# 형태의 스타일을 활용하기 보단
.section {
display: flex;
flex-direction: column
}
@media ... {
.section {
flex-direction: row
}
}
형태로 관리하는것도 좋아보여요
| .highlight { | ||
| color: transparent; | ||
| background-clip: text; | ||
| } | ||
| /* 변형 */ | ||
| .puplePink { | ||
| background-image: linear-gradient(90deg, #6d6afe, #ff9f9f); | ||
| } |
There was a problem hiding this comment.
너무 좋은데요!?!?
조금만 더 욕심내본다면, 이런식으로 묶어 활용하면 어떨까요?
| .highlight { | |
| color: transparent; | |
| background-clip: text; | |
| } | |
| /* 변형 */ | |
| .puplePink { | |
| background-image: linear-gradient(90deg, #6d6afe, #ff9f9f); | |
| } | |
| .text-gradient-purple-to-pink { | |
| color: transparent; | |
| background-clip: text; | |
| background-image: linear-gradient ... | |
| } |
There was a problem hiding this comment.
멘토님이 작성해주신게 더 네이밍이 좀더 명확해졌고, 같이 묶여있으니까 알아보기도 쉽네요!
| /* 좌측 글 */ | ||
| .left { | ||
| flex-direction: row; | ||
| } | ||
| /* 우측 글 */ | ||
| .right { | ||
| flex-direction: row-reverse; | ||
| } |
There was a problem hiding this comment.
기발한데요!
flex-direction을 트리거해도 좋지만, flex요소에 order를 제공하는 형식도 한번 고려해보면 좋겠습니다!
다음과 같이 활용할 수 있도록요
<section>
<text-group />
<img-group />
</section>
<section>
<text-group />
<img-group class="left"/>
</section>
.left {
order: 0
}
There was a problem hiding this comment.
아하! mdn에서 flex-direction : row-reverse; 를 하게 되면
접근성이 손상된다고 되어있네요!
order 속성을 사용해서 정렬을 다시 해보겠습니다!
요구사항
기본
심화
주요 변경사항
스크린샷
아래 그림들은 레이아웃 깨짐의 예시입니다.
지금은
display = flex를 통해서 레이아웃을 지정했기 때문에, 레이아웃의 최대최소값을 지정해주지 않아서 레이아웃이 깨진 것 같습니다.학습을 통해서
display = grid를 통해서 레이아웃을 지정하면,minmax()로 최대최소값을 지정해주면 레이아웃 깨짐 현상을 해결할 수 있을 것 같습니다.제가 생각한 것이 맞는지 궁금합니다.
멘토에게