[문혜란] Sprint3 #103
[문혜란] Sprint3 #103
Conversation
fix: 눈 아이콘을 이미지로 변경 refactor: form 태그 추가 및 HTML 구조 개선 chore: 불필요한 CSS 정리 및 최적화
style: 중복된 css 하나의 클래스로 통일
There was a problem hiding this comment.
혜란님 3번째 스프린트 미션 제출 고생하셨습니다!
반응형 작업을 아주 잘하셔서 매끄럽게 동작하네요~
PR 도 깔끔하게 올려주셔서 코드리뷰 드리기 편했습니다.
다음번 미션도 화이팅입니다!
-
피그마 요구사항에 의하면, Mobile 사이즈에서 판다마켓 로고가 변경되어야 합니다. CSS에서 background 속성을 사용하여 이미지를 교체하는 방법이 가장 먼저 떠오르는데, 이 방법이 적절한지 혹은 더 나은 방식이 있을지 궁금합니다.:
가능한 방법은 여러가지가 있을 수 있습니다. 말씀하신 css를 통해서 처리하는 방법이 있고 이도 세부적으로는 다양하게 구현이 가능합니다. 혹은 picture라는 태그를 이용해 화면 사이즈에 따라 이미지를 제어할 수 있습니다. 각 방식은 세부적으로 어떻게 구현하느냐에 따라 장단이 달라질 수 있습니다. 다만 지금의 로고의 경우에는 각 방식의 장단이 크게 차이가 나지 않을 것 같아서 원하시는 방식으로 구현하셔도 될 것 같습니다! -
다음 PR 올리실때는 base branch를 혜란님 브런치로 해서 올려주세요~
| :root { | ||
| --gray-900: #111827; | ||
| --gray-800: #1F2937; | ||
| --gray-700: #374151; | ||
| --gray-600: #4B5563; | ||
| --gray-500: #6B7280; | ||
| --gray-400: #9CA3AF; | ||
| --gray-200: #E5E7EB; | ||
| --gray-100: #F3F4F6; | ||
| --gray-50: #F9FAFB; | ||
|
|
||
| --blue-300: #1251AA; | ||
| --blue-200: #1967D6; | ||
| --blue-100: #3692FF; | ||
| --blue-light: #CFE5FF; | ||
|
|
||
| --gray: #FCFCFC; | ||
| --white: #FFFFFF; | ||
| } | ||
|
|
There was a problem hiding this comment.
❗️ 수정요청
특정 코드를 분류하는 기준을 추측할 수 있는 가장 큰 단서가 이름이라고 생각합니다.
reset 이라는 파일명을 통해 브라우저 스타일을 초기화하는 코드의 모음이라고 예상할 수 있습니다~
다만 css 변수 선언은 이러한 기준과 잘 맞지 않는다고 생각합니다!
만약 변수만 파일을 분리하시는 것이 싫으셨던 것이라면 해당 변수는 모든 페이지에서 걸쳐서 사용되는 코드이므로 common.css, base.css와 같은 식으로 공통 코드들을 모아두는 파일을 생성하시고 그 안에 두시면 좋겠습니다!
| } | ||
|
|
||
| .banner-title h2 { | ||
| color: var(--gray-700); |
There was a problem hiding this comment.
👍 칭찬
색상변수 사용해주신 것 좋아요 👍
| <meta property="og:title" content="판다마켓" /> | ||
| <meta property="og:url" content="https://hizpanda-market.netlify.app/" /> | ||
| <meta property="og:type" content="website" /> | ||
| <meta | ||
| property="og:description" | ||
| content="일상의 모든 물건을 거래해 보세요" | ||
| /> | ||
| <meta property="og:image" content="/assets/auth_logo.png" /> |
There was a problem hiding this comment.
❗️ 수정요청
이러한 메타태그가 제대로 되어 있는지 확인할 수 있는 방법이 여러가지 있습니다~
확인해보시고 요구사항에 따라 다른 SNS도 커버할 수 있게 meta 태그를 수정해보세요~
| <a href="/"> | ||
| <img src="assets/auth_logo.png" alt="판다마켓"/> | ||
| </a> |
There was a problem hiding this comment.
❓ 질문
왜 h1 태그를 빼신걸까요? 일반적으로 h1 태그는 로고에 많이 사용되서 적절한 배치라고 생각햇어서 아쉽네요~
| <label for="inputPwd">비밀번호</label> | ||
| <div class="inputPwd-wrap"> | ||
| <input type="password" class="input-font" id="inputPwd" placeholder="비밀번호를 입력하세요"> | ||
| <img src="assets/visibility_off.png" alt="비밀번호 숨김" class="toggle-password"/> |
There was a problem hiding this comment.
❗️ 수정요청
추후 구현하시겠지만 해당 요소는 비밀번호가 보이고 안 보이게 만드는 상호작용 요소이므로 button요소로 작성하시는 것을 추천드려요.
| <img src="assets/visibility_off.png" alt="비밀번호 숨김" class="toggle-password"/> | |
| <button type="button"><img src="assets/visibility_off.png" alt="비밀번호 숨김" class="toggle-password"/></button> |
| gap: 60px; | ||
| } | ||
|
|
||
| footer { |
There was a problem hiding this comment.
💊 제안
태그 선택자는 해당 태그가 모든 페이지에 걸쳐 적용되어야 하는 스타일링 외에는 사용을 추천드리지 않습니다~
위의 경우도 프로젝트가 확장되다보면 변경될 수 있으니 가능한 class를 사용하시는 것을 추천드릴께요~
예를 들어, 기존에는 모든 페이지에서 footer가 동일한 디자인이었지만 추후 추가된 페이지에서는 다른 디자인일 수 있습니다. 그래서 대부분의 경우 classname을 통해 개별로 css 해주시는 것을 추천드려요.
물론 지금 저희가 작업하는 환경의 경우 위와 같은 일은 없겠지만 클래스를 이용해 스타일링하시는 습관을 들이시는 것이 좋아 코멘트 남깁니다!
| background-color: var(--white); | ||
| } | ||
|
|
||
| .content-wrap img { |
There was a problem hiding this comment.
❗️ 수정요청
반응형 작업을 하시면서 이미지에 width, height값이 모두 존재하니 이미지가 기존 비율을 유지하지 못하고 왜곡되어서 보이게 됩니다.
width 속성만 남겨두시고 height 속성을 제거해주시면 기존 비율을 유지하면서 이미지가 렌더링됩니다~
| .content-wrap img { | |
| .content-wrap img { | |
| height: auto; // auto로 하셔도 비율을 유지하며 height를 차지합니다. |
2 participants
🌐 배포 URL : 판다마켓URL
요구사항
기본
체크리스트 [기본]
공통
1200px이상768px이상 ~1199px이하375px이상 ~767px이하375px미만 사이즈의 디자인은 고려하지 않는다.랜딩 페이지
24px, "로그인" 버튼 오른쪽 여백24px을 유지할 수 있도록 "판다마켓" 로고와 "로그인" 버튼의 간격이 가까워진다.16px, "로그인 버튼 오른쪽 여백16px을 유지할 수 있도록 "판다마켓" 로고와 "로그인" 버튼의 간격이 가까워진다.로그인, 회원가입 페이지 공통
16px제외하고 내부 요소들이 너비를 모두 차지한다.400px을 넘지 않는다.체크리스트 [심화]
스크린샷
랜딩 페이지 Mobile 사이즈
랜딩 페이지 Tablet 사이즈
멘토에게