Skip to content

구현 과제 - 로그인/회원가입 모달 - 준홍#15

Open
JJamVa wants to merge 1 commit intomainfrom
11-junHong
Open

구현 과제 - 로그인/회원가입 모달 - 준홍#15
JJamVa wants to merge 1 commit intomainfrom
11-junHong

Conversation

@JJamVa
Copy link
Copy Markdown

@JJamVa JJamVa commented Jul 28, 2023

로그인/회원가입 레이아웃을 구현하였습니다.

To do

  • 체크박스 기능 구현
  • 로그인/비밀번호 input border 수정

@JJamVa JJamVa changed the title [#11] 로그인/회원가입 모달 레이아웃 구현 구현과제 - 로그인/회원가입 모달 레이아웃 구현 Jul 28, 2023
@JJamVa JJamVa changed the title 구현과제 - 로그인/회원가입 모달 레이아웃 구현 구현과제 - 로그인/회원가입 모달 - 준홍 Jul 28, 2023
@merrybmc merrybmc changed the title 구현과제 - 로그인/회원가입 모달 - 준홍 구현 과제 - 로그인/회원가입 모달 - 준홍 Jul 28, 2023
Copy link
Copy Markdown
Collaborator

@merrybmc merrybmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

준홍님 첫 pr을 축하드려요 🎉🎉🎉
image
image
로그인 버튼과 SNS 로그인 버튼들, 우측 상단에 X 표시를 클라이언트가 누를 수 있는걸 알 수 있게
커서 모양을 바꿔주면 좋을 것 같아요.

Comment on lines +20 to +31
<div class="input-content">
<input id="id" type="text" placeholder="아이디" />
<label for="id">아이디를 입력해 주세요.</label>
<input id="pw" type="password" placeholder="비밀번호" />
<label for="pw">아이디 혹은 비밀번호과 일치하지 않습니다.</label>
</div>

<div class="state">
<input type="checkbox" id="cb" class="txt-hide login-state">
</input>
<label class="box" for="cb">로그인 상태 유지</label>
</div>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

밑에 login-form 클래스에 form 태그로 묶어주는 것 보단 input과 연관된 태그들에 form 태그로 묶어주는게 좋을 것 같아요.

https://developer.mozilla.org/ko/docs/Web/HTML/Element/form
참고하시면 좋을 것 같아요!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 input부터 button submit까지 하나의 form으로 묶어주는게 좋을거 같습니다!

@merrybmc
Copy link
Copy Markdown
Collaborator

전체적으로 특히 HTML 태그는 가독성 좋게 태그를 잘 묶어주고 구현하셨네요!
엄청 보기 편했어요. 👍👍👍

@merrybmc merrybmc requested review from AYFG and merrybmc and removed request for AYFG and merrybmc July 28, 2023 16:07
@YennieJ
Copy link
Copy Markdown
Collaborator

YennieJ commented Jul 29, 2023

전체적으로 코드가 깔끔하고 간결해서 가독성이 좋았습니다!! 고생하셨습니다:)

<meta charset="UTF-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>login Page</title>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

title까지👍

<span class="string-line">또는</span>
</div>
<section class="icon-btn">
<form class="login-form">
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

form태그는 사용자가 양식(input)을 전달할 때 쓰는 태그로 알고있습니다!
말씀하신대로 ul,li 사용이 더 좋을꺼 같아요!
변경하신다면 클래스명도 함께 변경하면 더 좋을꺼 같습니다:)

Comment on lines +238 to +239
padding-top: 10px;
padding-bottom: 10px;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

padding top,bottom의 영역이 로그인 버튼처럼 좀 더 넓어져야 할꺼 같아요!
참고로,
padding: top right bottom left; => padding: top&bottom right&left;
padding: 10px 50px 10px 50px; => padding: 10px 50px;
로 간결하게 줄일 수 있습니다!
준홍님의 경우 padding: 10px 0; 으로 변경하면 코드가 줄 수 있을꺼 같아요!(지금은 고작 한줄이지만 모이면 만줄이 될지도 모르자나여^0^)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

form태그 피드백, css refactory 작업 완료했습니다. 감사합니다.

@merrybmc merrybmc removed the 과제 label Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants