-
Notifications
You must be signed in to change notification settings - Fork 23
Basic 김민성 sprint2 #83
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
Basic 김민성 sprint2 #83
The head ref may contain hidden characters: "Basic-\uAE40\uBBFC\uC131-sprint2"
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.
구현은 대체로 잘 하셨습니다. 코드도 전체적으로 시멘틱하게 잘 작성하셨네요. 크게 리뷰드릴 것이 없습니다.
이전 피드백 내용들 잘 고치셨네요 👍
리뷰
- html/index.html은 필요없는 파일이라면 삭제해주세요. index.html이 두개라서 보는 사람이 어떤 파일이 진짜로 진입점이 되는 파일인지 헷갈릴 수 있습니다.
이 외에 코드 레벨에서 리뷰를 몇가지 남겨두었으니 확인바랍니다 :)
질문
-글로벌 네비게이션바의 뒷배경을 안넣고 버튼들만 고정시켜서 했는데 강사님께서는 어떻게 생각하나요?
- 일반적으로 sticky 헤더는 배경을 넣습니다. 배경을 생략하면 컨텐츠 내용과 header의 내용이 겹쳐져서 좋지않습니다. 헤더의 배경을 투명한 유리처럼 넣는 경우는 있습니다. Glassmorphism header 라고 핀터레스트에 검색해보시면 디자인 예시가 나오는데, 참고해주세요.
-비밀번호 input요소 오른쪽에 눈모양을 어떻게 생성할지 몰라서 ai에 도움받았는데 강사님의 도움이 필요합니다!
- 현재 구현에 대해서
현재 구현은 잘 하셨습니다. 어려운 부분은 ai 활용하시는 것도 좋지만, 꼭 코드를 이해하고 넘어가시길 바랍니다.
이해한 후에 직접 다시 작성해보시는것도 좋습니다. - 기능 누락
눈모양 아이콘을 누르면 비밀번호가 보일 수 있게 해주세요.
input type을 "password" ↔ "text"로 변경하는 로직이 필요합니다. 다음 미션에서 한번 적용해보세요.
수하셨습니다.
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 폴더 이름이 Css인데, 아마도 오타이신 것 같네요. 소문자로 시작하도록 해주세요
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과 signup 모두에서 쓰이는 스타일링 코드라면 auth.css처럼 모두를 포괄할 수 있게 지어주세요.
다른 개발자가 볼때, login.css는 있는데 왜 signup.css는 없지? 라고 생각할 수 있어요.
| </header> | ||
| <main> | ||
| <!--헤더 밑 메인의 섹션1부분--> | ||
| <section class="Top-section"> |
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.
(필수) class 이름 컨벤션을 정해서 지켜주세요!
다른 class는 전부 소문자로 시작하는데, Top-section => 소문자로 시작하는것이 맞지 않을까요?
| </div> | ||
| </div> | ||
| </section> | ||
| <section class="cta-section cta-section--with-image"> |
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.
(권장) cta는 무슨 뜻의 약자인가요? 자주 쓰이지 않는 약어의 경우 다른 개발자에게 혼동을 줄 수 있어서 약어 사용은 신중히 해주세요~
| <input type="password" class="login-form__input login-form__input--password" placeholder="비밀번호를 입력하세요."> | ||
| <button type="button" class="login-form__toggle-password" aria-label="비밀번호 보기/숨기기"> | ||
| <svg width="20" height="20" viewBox="0 0 20 20" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||
| <path d="M10 3C5 3 1.73 7.11 1 10C1.73 12.89 5 17 10 17C15 17 18.27 12.89 19 10C18.27 7.11 15 3 10 3ZM10 15C7.24 15 5 12.76 5 10C5 7.24 7.24 5 10 5C12.76 5 15 7.24 15 10C15 12.76 12.76 15 10 15ZM10 7C8.34 7 7 8.34 7 10C7 11.66 8.34 13 10 13C11.66 13 13 11.66 13 10C13 8.34 11.66 7 10 7Z" fill="#9CA3AF"/> |
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.
(필수) 가능하면 아이콘 svg는 가독성때문에 따로 파일 분리해서 불러와서 사용해주세요. 특히 이 eye 아이콘 같은 경우는 회원가입 페이지에서도 쓰이는, 재사용되는 svg이기 때문에 분리하는것이 좋습니다.
요구사항
기본
심화
주강사님에게
-글로벌 네비게이션바의 뒷배경을 안넣고 버튼들만 고정시켜서 했는데 강사님께서는 어떻게 생각하나요?
-비밀번호 input요소 오른쪽에 눈모양을 어떻게 생성할지 몰라서 ai에 도움받았는데 강사님의 도움이 필요합니다!
-이전 미션1의 피드백을 수정했습니다
2.섹션 클래스네임을 번호에서 해당 기능의 이름을 넣어서 독립성을 넣었습니다
3.index.html은 루트위치로 뺏습니다
4.css파일의 위치수정으로 인해 경로 수정했습니다.
5.BEM 설계해서 클래스이름 수정했습니다.