-
Notifications
You must be signed in to change notification settings - Fork 39
[나소연] sprint1 #24
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 #24
The head ref may contain hidden characters: "Basic-\uB098\uC18C\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.
수고하셨습니다!
다음 미션부터는 클래스네이밍을 좀더 신중히 고려해보고, 포맷팅 규칙을 일관적으로 지켜서 작성해볼까요?
주요 리뷰 포인트
- 클래스이름 작성
- 코드 중복 제거
- 불필요한 div 중첩 & 스타일 명세 제거
| <a href="index.html"> | ||
| <img src="Icon/Panda-logo-md.png" class="panda-logo-md" 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.
접근성 향상을 위해 aria-label="홈으로 이동" 과 같이 관련있는 요소에 aria-label 혹은 role을 사용해볼까요?
아래 아티클 참고해보시고 접근성을 향상시키기 위해 어떤 노력을 할수있는지 더 찾아보시면 좋은 공부 될 것같아요 :)
https://joyhong-91.tistory.com/22
https://velog.io/@a_in/WAI-ARIA-role-aria-label
| <a href="index.html"> | ||
| <img src="Icon/Panda-logo-md.png" class="panda-logo-md" alt="판다마켓 로고"> | ||
| </a> | ||
| <a href="login.html" class="login-button is-clickable button-deco">로그인</a> |
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.
button-deco가 조금 모호한 네이밍처럼 느껴지네요.
항상 클래스이름을 어떻게하면 더 잘 지을수있을지 고민해주면 좋아요.
엘리먼트의 역할에 따라 클래스이름을 잘 지을수있도록 고려해주면 유지보수할때 편해지겠죠?
| @@ -0,0 +1,95 @@ | |||
| <!DOCTYPE html> | |||
| <html lang="en"> | |||
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.
언어 설정을 "ko"로 변경하시면, 한국어 사용자에 맞춰 스크린리더가 사용될수있고 검색엔진에서도 페이지의 언어를 한국어 콘텐츠로 인식하게끔 만들어줄 수 있습니다. 웹 접근성을 생각했을때 스크린리더를 사용한다면 lang="en" 상태에서는 한국어 콘텐츠를 영어 발음으로 읽을 가능성이 있기때문에, 한국어 사용자를 대상으로한다면 "ko"로 변경하시는게 옳습니다 :)
| <html lang="en"> | |
| <html lang="ko"> |
| <li><a href="faq.html">FAQ</a></li> | ||
| </ul> | ||
| <ul class="footer-list"> | ||
| <li><a href="https://www.facebook.com" target="_blank"><img src="Icon/ic_facebook.png" |
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: 리퍼러 정보가 새 페이지로 전달되는 것을 방지
| </div> | ||
| </section> | ||
| </header> | ||
| <main class="contents"> |
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.
contents보다는 feature-section으로 변경하시고, 아래 카드 영역과 콘텐트영역 클래스이름도 feature-card, feature-content 등등으로 변경하시면 좀더 클래스명이 직관적으로 보일거예요.
너무 일반적이거나 모호한 클래스명은 가독성, 유지보수성, 확장성이 모두 떨어질 수 있습니다. 그 이유는 너무 일반적인 클래스명은 프로젝트내에서 반복적으로 사용될 가능성이 있고, 나중에 다른 개발자가 동일한 클래스명으로 스타일을 또 만들면 스타일이 충돌되거나 꼬이는 문제가 생길 수도 있기 때문입니다. 모호한 클래스명의 경우에도 유지보수를 어렵게한다는 단점이 생깁니다.
이런 문제를 최대한 방지하기위해서는 거듭 강조하지만
"너무 일반적이거나 모호한 이름을 사용하면 어떤 문제가 있는지" 이해하고,
그 문제를 해결하기위해 신경써서 작업해보시는걸 추천드립니다! :)
|
|
||
| .gnb { | ||
| display: flex; | ||
| flex-direction: row; |
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.
flex 속성을 사용하면 기본적으로 flex-direction: row; 가 적용돼요.
이렇게 기본 적용되는 디폴트 값을 잘 이해하면 불필요한 스타일 명세도 없애줄수있겠죠?
| .banner { | ||
| display: flex; | ||
| flex-direction: row; | ||
| padding: clamp(66px, 13vw, 200px) 0 0; |
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.
clamp를 잘 활용하시네요! 👍
| @@ -0,0 +1,197 @@ | |||
| body { | |||
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 (전역 스타일 처리)
| flex-direction: row; | ||
| padding: clamp(66px, 13vw, 200px) 0 0; | ||
| width: 100%; | ||
| background-color: #cfe5ff; |
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 변수로 처리해서 재사용해주면 코드 중복을 줄일수있어요 :)
예시)
/* Variables */
:root {
--primary-color: #3692ff;
--text-primary: #374151;
--text-secondary: #9ca3af;
--text-light: #e5e7eb;
--background-primary: #fcfcfc;
--background-secondary: #cfe5ff;
--background-dark: #111827;
--border-radius-sm: 8px;
--border-radius-lg: 12px;
--border-radius-xl: 40px;
--container-width: 1120px;
--content-width: 988px;
}|
|
||
| .footer-bottom { | ||
| background-color: #111827; | ||
|
|
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 변수기반으로 미디어 쿼리로 스타일 분기를 관리하시는게 좀 더 일반적인 방식입니다 :) |
요구사항
기본
심화
주요 변경사항
배포사이트
https://sprintmission-w1-nsy.netlify.app/
스크린샷
멘토에게