-
Notifications
You must be signed in to change notification settings - Fork 44
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
[이현승] Week2 #9
The head ref may contain hidden characters: "part1-\uC774\uD604\uC2B9-week2"
[이현승] Week2 #9
Conversation
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.
주어진 시안에 따라 적당한 태그를 사용하여 비슷하게 형태를 만드는 것까지 이번주 학습 내용을 잘 이해하신 것 같아요 👍🏽
코멘트 남겨드린 부분 꼼꼼히 확인 후 다음 과제에 꼭 반영해보세요
수고 많으셨습니다! 🥇
<head> | ||
<meta charset="UTF-8" /> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
<link href="Linkbrarystyle.css" rel="stylesheet" /> |
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.
일반적인 html, css 파일명은 주로 케밥 케이스를 많이 쓰는 것 같아요. (찾아보기: 'kebab case', 'snake case', 'camel case')
아래는 작성해주신 코드를 케밥 케이스로 변경한 예시이고 파일명이 꼭 이래야한다는 절대적인 정답은 아닙니다!
<link href="Linkbrarystyle.css" rel="stylesheet" /> | |
<link href="linkbrary-style.css" rel="stylesheet" /> |
파일명까지 포함해서 의견 드리자면,
css 파일 자체가 스타일링을 한다는 것을 의미하기 때문에 'style' 이라는 단어가 파일명에 굳이 포함되지 않아도 될 것 같아요.
작성하신 것처럼 서비스의 명칭을 사용하여 linkbrary.css
로 작성하거나 홈페이지(index.html)를 의미하는 index.css
, home.css
등으로도 작성해볼 수 있을 것 같네요 😀
href="https://fonts.googleapis.com/css2?family=Acme&display=swap" | ||
rel="stylesheet" | ||
/> | ||
<title>Linkbrary</title> |
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.
타이틀 잘 지정해주셨네요 👍🏽
<title>Linkbrary</title> | ||
</head> | ||
<body> | ||
<header class="gnb"> |
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.
gnb 는 Global Navigation Bar
의 약자로 사이트 내 주요 페이지로 이동할 수 있는 링크들의 집합을 의미하는데요,
(참고: 알아두면 좋은 웹 용어! GNB, LNB, SNB, FNB (feat. 인프런))
시안으로 봤을 때는 헤더에 네비게이션이 포함된 형태가 아니기 때문에 'gnb' 라는 클래스명과는 어울리지 않는 것 같습니다
'header' 클래스를 동일하게 사용해도 좋을 것 같아요 :)
</head> | ||
<body> | ||
<header class="gnb"> | ||
<div class="logoin"> |
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.
-
오타 주의해주세요! (login 을 의미하신거겠죠?) -> 사소한 오타 때문에 버그 생기기 쉽고 의외로 찾기 힘들어서 고생하기도 합니다
-
해당 영역 봤을 때 로고와 로그인 버튼 (헤더의 모든 영역) 을 포함하고 있어 login 이라고 네이밍 하는 게 맞을까? 하는 생각이 듭니다.
해당 영역을 포괄하는 단어를 사용하는 게 좋을 것 같아요.
아래 사용하신 container 나 wrapper 등을 잘 활용해봐도 좋을 것 같습니다
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.
logo와 login 버튼이 같이 들어가 있는 부분이라서 클래스를 logoin으로 지었습니다.
<body> | ||
<header class="gnb"> | ||
<div class="logoin"> | ||
<img src="landing/logo.svg" alt="logo" /> |
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.
alt 태그는 접근성 관점에서 중요한데요, 보다 정보를 전달할 수 있는 내용으로 작성해주시는 게 좋습니다
첫번째 멘토링 시간에 공유드렸던 자료들 시간내서 꼭 한 번 보시면 도움 되실 거에요!
alt 작성한 부분들 전체적으로 어떻게 개선하면 좋을지 고민해보세요 🙂
</head> | ||
<body> | ||
<main> | ||
<article class="logintable"> |
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.
article 태그의 용도는 무엇인가요?? 로그인 화면에 사용하기 적절할까요?? 🤔
<form> | ||
<label for="email">이메일</label> | ||
<input id="email" name="email" type="email" /> |
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.
form 태그 사용, input 과 라벨 연결, 주요 속성 추가 등 잘 적용해주셨네요 👍🏽
<form> | ||
<label for="email">이메일</label> | ||
<input id="email" name="email" type="email" /> | ||
<label for="password">비밀번호 </label> |
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.
불필요한 공백 들어가지 않게 확인해주세요! 전체적으로 몇몇 있네요
<label for="password">비밀번호 </label> | |
<label for="password">비밀번호</label> |
<div class="googlebg"> | ||
<img src="landing/icon/googlebg.svg" alt="구글 배경"> | ||
<div class="googleicon"> | ||
<img src="landing/ic/google.png" alt="구글"> | ||
</div> | ||
</div> |
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.
여기랑 카카오 이미지 부분 추출을 잘못 하신 것 같은데 배경이랑 아이콘을 묶어서 추출해야 하는 것 아닌가 싶네요???
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.
어째서인지 피그마 이미지 추출에서 따로 따로 다운로드 되는 바람에 어쩔 수 없이 저렇게 했습니다
gap: 9px; | ||
} | ||
|
||
a.tosignup { |
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.
tosignup 이라는 클래스로 해당 요소에 스타일링이 가능한데 a 태그와 같이 작성하신 이유가 있나요??
233라인도 마찬가지고요!
잘못된 건 아니지만 굳이 이렇게 작성하시지 않고 클래스로만 작성해도 될 것 같아요!!
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.
알겠습니다.
바쁘실텐데 유익한 피드백 해주셔서 감사합니다.
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게