Skip to content

구현 과제 - 로그인/회원가입 모달 - 하진#34

Open
Hajin-Bang wants to merge 3 commits intomainfrom
11-haJin
Open

구현 과제 - 로그인/회원가입 모달 - 하진#34
Hajin-Bang wants to merge 3 commits intomainfrom
11-haJin

Conversation

@Hajin-Bang
Copy link
Copy Markdown

로그인/회원가입 모달 과제 구현하였습니다!

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을 축하드립니다. 🎉🎉

제가 피드백드리는 내용이 별로 없는 이유는 피드백 드릴 내용이 없기 때문입니다.. 😭😀👍👍

Comment on lines +79 to +84
.back {
display: inline-block;
width: 16px;
height: 16px;
margin: 2px 24px auto auto;
}
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.

웹접근성을 고려해서 사용자가 X 모양 표시를 클릭할 수 있는 버튼이란걸 알 수 있게 커서를 포인터로 변경해주면 좋을 것 같아요. 😀

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.

그러네요!! 의견 감사합니다. 변경해볼게요!

index.html Outdated
Comment on lines +30 to +45
<div class="login-form">
<input type="text" placeholder="아이디">
<span>아이디를 입력해 주세요.</span>

<input type="text" placeholder="비밀번호">
<span>아이디 혹은 비밀번호가 일치하지 않습니다.</span>
</div>

<div class="keep-login">
<input type="checkbox" id="check">
<label for="check" class="check-label">
<span>로그인 상태 유지</span>
</label>
</div>

<button class="login-submit" type="submit">로그인</button>
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.

웹접근성과 나중에 js 구현을 고려하였을 때 input 태그와 연관되는 button은 form 태그로 묶어주시는게 좋아요.

한 번 참고해보시는 것도 좋을 것 같습니다. :)
https://developer.mozilla.org/ko/docs/Web/HTML/Element/form

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태그를 사용하는 것이 좋군요 ..
반영해보겠습니다☺️ 감사합니다 병민님!

@Hajin-Bang Hajin-Bang closed this Aug 7, 2023
@Hajin-Bang Hajin-Bang reopened this Aug 7, 2023
Copy link
Copy Markdown
Collaborator

@YennieJ YennieJ left a comment

Choose a reason for hiding this comment

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

하진님 고생하셨습니다👍
제가 본 체크박스중에 가장 깔끔한 코드였던거 같아여👍
그 외에 코드도 가독성이 좋았어요!

css/style.css Outdated
Comment on lines +165 to +167
input[type='checkbox'] {
display: none;
}
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.

접근성을 위해서 .a11y-hidden로 숨겨주는것이 좋을거 같아여!

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.

오 그렇게 해볼게요!!
감사합니다 예진님! ☺️

index.html Outdated
Comment on lines +59 to +62
<button class="google" type="submit">구글 계정으로 로그인</button>
<button class="facebook" type="submit">페이스북 계정으로 로그인</button>
<button class="naver" type="submit">네이버 계정으로 로그인</button>
<button class="kakao" type="submit">카카오톡 계정으로 로그인</button>
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.

button의 타입이 submit 보단 button일꺼같고, a태그도 고려해보시는 것도 좋을꺼같습니다:)👍

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.

네! 우선 타입을 button으로 다시 작성해야겠어요..😮 감사합니다!

index.html Outdated
Comment on lines 30 to 46
<form class="login-form">
<input type="text" placeholder="아이디">
<span>아이디를 입력해 주세요.</span>

<input type="text" placeholder="비밀번호">
<span>아이디 혹은 비밀번호가 일치하지 않습니다.</span>
</div>
</form>

<div class="keep-login">
<input type="checkbox" id="check">
<form class="keep-login">
<input type="checkbox" id="check" class="a11y-hidden">
<label for="check" class="check-label">
<span>로그인 상태 유지</span>
</label>
</div>
</form>

<button class="login-submit" type="submit">로그인</button>

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.

하진님 +30~+46까지 form으로 묶어야 input 의 모든 정보가 submit으로 정보가 전달됩니다!

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.

앗 모든 정보가 같이 묶여야 하는군요!
기존 div 태그는 놔두고 form으로 한꺼번에 묶어볼게요☺️ 알려주셔서 감사합니다!

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