-
Notifications
You must be signed in to change notification settings - Fork 39
[이수연] sprint1 #43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[이수연] sprint1 #43
The head ref may contain hidden characters: "Basic-\uC774\uC218\uC5F0-sprint1"
Conversation
addiescode-sj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!
전체적으로 클래스명도 보기 좋고 쉽게 쉽게 잘 짜시네요.
조금만 다듬어보면 멋진 프로젝트 나올수있을것같아요 👍
주요 리뷰 포인트
- 기본값 사용 제거
- 유지보수 고려한 개발 방식 제안
- 크로스 브라우징 이슈 체크
- 코드 중복 제거
| <body> | ||
| <header> | ||
| <nav> | ||
| <a href="/"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
접근성을 고려해 <a href="/" aria-label="홈으로 이동"> 과 같이 aria-label 혹은 role을 추가해볼까요?
| <header> | ||
| <nav> | ||
| <a href="/"> | ||
| <img class="logo" src="image/Property 1=sm.svg" alt="로고"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
귀찮더라도 img에 alt 속성 추가 & 명확한 alt 텍스트 사용을 하시는게 좋아요!
웹 접근성뿐만 아니라, 이미지 로딩 실패 시 대체 텍스트를 표시하거나 검색엔진 최적화에도 도움을 줄 수 있기 때문에 꼭 사용해주시는게 좋습니다.
레퍼런스로 애플 공식 웹사이트에 가서 alt텍스트를 어떻게 사용했는지 참고해보시면:
<img src="/kr/macbook-air/images/overview/design/design_hero_static__e56c1v71mr6u_large.jpg" onload="__lp(event)" alt="열려있는 MacBook Air 13 및 15의 모습. 한 대에는 디자인 작업을 진행 중인 화면이, 다른 한 대에는 이메일과 스프레드시트를 넘나들며 멀티태스킹을 하는 화면이 표시되어 있습니다">이런식으로 alt 속성에 이미지 설명을 위해 구체적이고 명확한 설명을 제공하는 모습을 확인해보실 수 있습니다 :)
| <a class="faq" href="features/faq.html">FAQ</a> | ||
| </div> | ||
| <div class="social-media"> | ||
| <a class="facebook" href="https://www.facebook.com/?locale=ko_KR" target="_blank"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
외부 링크의 경우, 보안을 위해 rel="noopener noreferrer"를 추가해주세요! :)
noopener: 새 탭에서 열린 페이지가 원본 페이지에 접근하는 것을 방지
noreferrer: 리퍼러 정보가 새 페이지로 전달되는 것을 방지
| body { | ||
| font-family: "Pretendard Variable", Pretendard, -apple-system, BlinkMacSystemFont, system-ui, Roboto, "Helvetica Neue", "Segoe UI", "Apple SD Gothic Neo", "Noto Sans KR", "Malgun Gothic", "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", sans-serif; | ||
| margin: 0 auto; | ||
| } | ||
|
|
||
| * { | ||
| box-sizing: border-box; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
용도에 따라 CSS를 나눠서 관리해볼까요?
- reset.css: CSS 초기화
- common.css: 전역 스타일
이렇게 용도에 따라 파일을 나눠 관리하는건 어떤 이점을 가져다줄수있는지 한번 더 생각해볼수있어요.
- 역할 분리가 되어 관리가 쉬움: 각 파일의 목적이 명확해서 수정이나 유지보수가 쉬워질거예요.
- 사용성과 확장성 증가: 여러 페이지 혹은 프로젝트에서 거의 동일하게 재사용이 가능해요.
- 스타일 우선순위가 더 예측 가능해짐: reset.css → common.css → 페이지나 컴포넌트별 스타일 순으로 로딩하면 우선순위가 명확해지고 스타일 충돌을 줄일 수 있어요.
| font-size: 40px; | ||
| font-weight: 600; | ||
| letter-spacing: 2%; | ||
| color: #374151; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
자주 사용되는 값들엔 CSS 변수를 만들어 재사용해주면 코드의 중복도 줄일수있겠죠?
예시)
:root {
--color-primary: #3692FF;
--color-text: #374151;
--color-text-light: #9CA3AF;
--color-background: #FCFCFC;
--color-background-dark: #111827;
--color-background-light: #E5E7EB;
--color-banner: #CFE5FF;
--font-family-base: 'Pretendard-Regular', sans-serif;
--font-family-heading: 'Noto Sans KR', sans-serif;
--container-width: 1120px;
--header-height: 70px;
--footer-height: 160px;
--spacing-xs: 12px;
--spacing-sm: 24px;
--spacing-md: 32px;
--spacing-lg: 60px;
--spacing-xl: 138px;
}| text-align: center; | ||
| text-decoration: none; | ||
| background-color: #3692FF; | ||
| width: auto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
width: auto;는 기본값이라 써주실 필요 없습니다~!
|
|
||
| .banner-image > img { | ||
| max-width:100%; | ||
| height:auto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요것도 기본값이예요 :)
| margin: 138px auto; | ||
| background-color: #FCFCFC; | ||
| border-radius: 12px; | ||
| width: fit-content; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://caniuse.com/mdn-css_properties_min-width_fit-content
width: fit-content는 문제가 없지만,
firefox에서 min-width로 쓰려할때 제한이 하나 있네요.
크로스 브라우징 이슈 체크해가면서 쓰시는게 좋습니다 👍
| flex-direction: column; | ||
| justify-content: center; | ||
| align-items: flex-start; | ||
| padding: 0 40px 0 40px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요런건padding: 0 40px; 로 단축해서 쓸수있어요! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mdn 링크에서 자세한 용법도 참고해보세요 :)
https://developer.mozilla.org/en-US/docs/Web/CSS/padding
질문에 대한 답변
클래스네이밍 지금 정도면 잘 짜시는 편입니다 👍
항상 디자인 시안에 따른 디자인 의도를 최대한 똑같이 구현해보세요! CSS에서 width 속성의 기본값은 auto이고, 이는 요소의 콘텐츠 크기나 부모 요소에 따라 너비가 자동으로 결정된다는 뜻입니다. |
요구사항
기본
심화
주요 변경사항
스크린샷
[https://pandamarketbysy.netlify.app/]

멘토에게