[이승민] Sprint2 #107
[이승민] Sprint2 #107
Conversation
ghost
requested a review
from 
ghost
added
the
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
승민님 2번째 PR 제출 고생하셨습니다.
주요 변경사항에 진행 상황을 자세히 적어주셔서 감안해서 코드리뷰 드렸어요~
새로운 기술을 학습하시고 바로 스프린트 미션을 하시는 것이 쉬운 일은 아니니 템플릿 코드를 참고하시는 것도 좋은 방법입니다~
단순 참고하시는 것을 넘어 제 피드백을 이해하고 이에 적합한 부분을 적용하시는 것이면 더 훌륭하니 너무 어려운 부분이 있으시면 그렇게 하셔도 괜찮을 것 같아요!
지금 잘하고 계신거에요 👍
-
피드백 중에 image 태그의 width 속성값을 단위없는 정수로 바꾸니까 자꾸 이미지가 콩알만해져서 원래 제 코드로 했는데 괜..찮겠죠??:
추후 반응형 디자인을 구현하는 요구사항이 나오니 그때 css 로 img width를 처리하시고 해당 속성은 지우시는 것을 추천드려요~ -
로그인. 회원가입 페이지의 html 코드는 gpt없이 구글링 + mdn + 유튜브보면서 짤 수 있었습니다 다만 스프린트 미션1페이지에 비해 난이도가 낮아서 가능했다고 생각합니다:
스스로 작업하신 것 너무 좋습니다~ 미션 1이 기반작업을 해야해서 난이도가 가장 높았다고 생각해요~ 2주차를 스스로 하셨으면 나머지도 잘 하실거에요! -
그리고 제가 파일 구조에 대해 고민을 많이 했습니다 구글링도 해보고 유튜브도 참고해봤는데 일단 저렇게 짜봤는데 소수의 파일로 합치는게 추후 웹페이지 제작할떄 편할까요??:
파일구조에 대해 고민하고 변경하신 점 좋습니다~ 다만 파일구조라는 것은 현재 프로젝트의 규모와 상황에 영향을 많이 받습니다. 어떤 구조가 특정 시점에서는 오버 엔지니어링(요구사항보다 더 대단하게 작업물을 만드는 것. 리소스가 많이드므로 불필요하다면 지양하는 것이 좋음)일 수도 있고 어떤 상황에서는 적절한 구조일 수 있습니다. 따라서 현 상황에 맞는 구조를 적용하시면 됩니다. 다만 학습이나 경험을 위해 구조를 변경해보고 싶으시다면 기준을 세워 분류해보시고, 이 프로젝트에 신입 개발자가 투입된다고 했을때를 상정해보시고 이해하기 쉬운지를 고민해보시면 좋을 것 같습니다.
| - [x] | ||
| - [] | ||
| - [] | ||
| - [0] 아래로 스크롤 해도 상단 네비게이션 바(Global Navigation Bar)가 최상단에 고정됩니다. |
There was a problem hiding this comment.
💊 제안
mdx 포멧에서 list 요소의 check 여부를 작성하실 때는 영문 o를 사용하시면 됩니다~
| - [0] 아래로 스크롤 해도 상단 네비게이션 바(Global Navigation Bar)가 최상단에 고정됩니다. | |
| - [o] 아래로 스크롤 해도 상단 네비게이션 바(Global Navigation Bar)가 최상단에 고정됩니다. |
There was a problem hiding this comment.
👍 칭찬
인라인 스타일을 지우신 것 너무 좋아요 👍
| <label for="email">이메일</label> | ||
| <input id="email" name="email" type="email" placeholder="이메일을 입력해 주세요" required /> |
There was a problem hiding this comment.
👍 칭찬
label과 Input을 연결해주신 점 좋습니다~
| <label for="password">비밀번호</label> | ||
| <div class="password-wrapper"> | ||
| <input id="password" name="password" type="password" placeholder="비밀번호를 입력해 주세요" required /> | ||
| <img src="images/login_icon/eye-invisible.svg" alt="비밀번호 감추기" class="toggle-password" /> |
There was a problem hiding this comment.
❗️ 수정요청
해당 요소는 클릭 시 비밀번호가 보이고 안 보이게 만드는 요소가 될 것이므로 button요소로 작성하시는 것을 추천드려요.
| <img src="images/login_icon/eye-invisible.svg" alt="비밀번호 감추기" class="toggle-password" /> | |
| <button type="button"><img src="images/login_icon/eye-invisible.svg" alt="비밀번호 감추기" class="toggle-password" /></button> |
| <img src="images/login_icon/eye-invisible.svg" alt="비밀번호 감추기" class="toggle-password" /> | ||
| </div> | ||
| </div> | ||
| <button type="submit" class="auth-button">로그인</button> |
There was a problem hiding this comment.
💬 여담
button 태그의 type 속성을 잘 명시해주셨어요~
참고로 type이 submit인 button의 경우 form에서 제출 이벤트가 발생하면, 해당 요소의 onclick 이벤트가 실행되게 됩니다.
이를 더 풀어서 말하자면 form 태그안의 input, button 과 같은 상호작용 요소에서 submit 이벤트, enter key 입력이 발생하면 해당 form 에서의 submit 이벤트가 실행된다는 것입니다.
유저 입장에서는 로그인이나 회원가입시 input, button에서 enter를 누르면 제출되는 것에 익숙하기 때문에 이를 신경써주시는 것이 좋습니다~
// case1: form에 onsubmit 이벤트 명시
<form onsubmit="login">
<input /> // input에서 enter 키 입력시 onsubmit 이벤트 실행
<button type="submit">login</button> // 해당 버튼 클릭 시 onsubmit 이벤트 실행
</form>
// case2: submit 버튼에 onclick 이벤트 명시
<form>
<input /> // input에서 enter 키 입력시 button의 onclick 이벤트 실행
<button type="submit" onclick="login">login</button>
</form>| @@ -0,0 +1,40 @@ | |||
| header { | |||
There was a problem hiding this comment.
💊 제안
현재 루트 페이지의 button 태그와 로그인, 회원가입 페이지의 button 태그가 서로 다른 디자인으로 존재하는 것 처럼 태그는 페이지마다 다르게 사용될 수 있습니다. 따라서 태그 선택자는 모든 페이지에서 동일한 스타일을 적용할 때만 사용하는 것이 좋습니다.
특정 페이지에서만 다르게 스타일링해야 하는 요소라면 태그 선택자보다는 class를 사용하는 것이 유지보수와 확장성 면에서 더 좋습니다. 프로젝트가 커질수록 각 페이지의 구조나 스타일이 변경될 가능성이 높으므로, 이를 미리 고려하여 class로 스타일을 지정하는 것을 추천드립니다.
There was a problem hiding this comment.
👍 칭찬
모든 페이지에 걸쳐 사용되는 속성을 global.css 로 분리해주신 것 좋습니다 👍
There was a problem hiding this comment.
👍 칭찬
중복된 코드를 줄이고자 하나의 파일로 작성하신 점 좋습니다.
| color: #1f2937; | ||
| } | ||
|
|
||
| .input-group input { |
There was a problem hiding this comment.
❗️ 수정요청
해당 input이 디자인보다 높이가 작게 작업되어 있어요~
피그마에서 해당 요소의 높이를 확인해보시고 디자인과 동일하게 변경해보세요!
|
|
||
| <div class="auth-footer"> | ||
| 판다마켓이 처음이신가요? | ||
| <a href="signup.html">회원가입</a> |
There was a problem hiding this comment.
❗️ 수정요청
해당 요소가 디자인 상에서는 파란색이 적용되어 있는데 작업물에서는 검은색이네요~
확인 후 디자인과 동일하게 수정하시는 것을 추천드려요!
1 participant
요구사항
기본
로그인 페이지
회원가입 페이지
심화
주요 변경사항
스크린샷
멘토에게