Skip to content

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

Open
YennieJ wants to merge 16 commits intomainfrom
11-yeaJin
Open

구현 과제 - 로그인/회원가입 모달 - 예진#17
YennieJ wants to merge 16 commits intomainfrom
11-yeaJin

Conversation

@YennieJ
Copy link
Copy Markdown
Collaborator

@YennieJ YennieJ commented Jul 30, 2023

로그인/회원가입 구현

배포주소 : https://yejinlogin-p3ia36n8p-merrybmc.vercel.app/index.html

코멘트 부탁드립니다🙏

  1. 소셜로그인 CSS : 더 깔끔하게 코드를 줄일 수 있는 방법이 없을까요?
    login.html +64 ~ +88
    login.css +166 ~ +222

  2. checkbox : 이미지로 만든 checkbox 성능에 괜찮을까요?
    login.html +49
    login.css +86 ~ +109

이 외에 다른 코멘트들도 모두 환영이에요🤞

@AYFG
Copy link
Copy Markdown

AYFG commented Jul 31, 2023

weniv 로그인버튼 시작에서 로그인/회원가입 모달 경로 이동 , 웹 접근성을 위한 텍스트 숨김 , css 속성들 띄워서 가독성 높이기 등 정말 꼼꼼하게 구현하셨다고 느꼈습니다 😁
2번 질문에 대하여 생각해보았는데 cursor:pointer가 있는게 좋지않을까?생각하고 네이버의 로그인은 어떤지 보고왔는데 체크박스와 로그인 상태 유지 글씨가 pointer로 변하더라고요. 예진님 덕분에 배웠습니다
공유해주셔서 감사합니다~~😊

@YennieJ YennieJ closed this Jul 31, 2023
@YennieJ YennieJ deleted the 11-yeaJin branch July 31, 2023 07:48
@YennieJ YennieJ reopened this Jul 31, 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.

클래스를 여러 개로 나누는 것과 css 파일을 여러개로 분할해서 쓰시고
각 스코프마다 코드의 양도 많지 않게 구현하신 예진님의 구현 솜씨를 보면
벌써 리액트가 눈에 아른거리네요..

엄청 잘하셨네요 정말로 바닐라JS 한 수 배우고 갑니다 😀

Comment on lines +64 to +88
<section class="social-login-container">
<h2 class="a11y-hidden">소셜서비스로 로그인</h2>
<ul class="social-login-group">
<li>
<button class="icon-google social-login-button google">
구글 계정으로 로그인
</button>
</li>
<li>
<button class="icon-facebook social-login-button facebook">
페이스북 계정으로 로그인
</button>
</li>
<li>
<button class="icon-naver social-login-button naver">
네이버 계정으로 로그인
</button>
</li>
<li>
<button class="icon-kakao social-login-button kakao">
카카오톡 계정으로 로그인
</button>
</li>
</ul>
</section>
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.

시맨틱 태그까지 고려했을 때 HTML 태그하고 CSS까지 완벽한 것 같아요 😮👍

Comment on lines +104 to +109
.login-state input[type="checkbox"] + label:before {
background-image: url("../imgs/login-checkbox.svg");
}
.login-state input[type="checkbox"]:checked + label:before {
background-image: url("../imgs/login-checked.svg");
}
Copy link
Copy Markdown
Collaborator

@merrybmc merrybmc Jul 31, 2023

Choose a reason for hiding this comment

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

이미지 2개를 사용하여 커스텀 체크박스를 구현하셨네요.
이미지 크기가 작기도 하고 성능 상으로는 지금 당장 눈에 보이는 차이가 없겠지만
이미지를 로드하고 렌더링이 늦어지는 이슈는 있습니다.

저는 체크가 안된 이미지 하나를 svg로 추출하여 class를 주고 체크가 되었을 때
fill과 stroke를 통해서 색상에 변화를 주었어요.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

멘토링 받은 내용,
체크박스는 흔히 이미지로 커스텀해서 사용하고 용량이 작아서 큰 문제는 없을것 같다고 하시네요👍

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.

아하 맞아요.
체크박스는 용량이 작아서 이미지로 커스텀해서 사용해도 성능 이슈는 거의 없을 것 같아요. 👍

@merrybmc merrybmc removed the 과제 label Aug 2, 2023
@redcontroller
Copy link
Copy Markdown

예진님 코드가 굉장하다고 해서 보러 왔어요 ~
정말 멋진 코드에 감탄하고 갑니다 ! 😀
정말 디자인 시안처럼 움직이는 로그인 모달을 만들어서 깜짝 놀랐어요 👍

@redcontroller
Copy link
Copy Markdown

redcontroller commented Aug 3, 2023

처음 페이지((index.html, index.css)도 리펙토링 해보는건 어떨까 생각이 드네요! 😊

  • 처음 페이지 페이지도 사용자 화면을 기준으로 가운데 정렬되면 더 좋을 것 같아요 (body 의 height:100vh)
  • 로그인 버튼에 키보드로 접근이 안되는데, 가상 클래스 (:focus) 추가 해주는 건 어떨까요?

로그인/회원가입 모달로 연동되는 것에서 정말 놀랐네요 🙄
저도 이전 실습 가져와서 예진님처럼 완성도를 높여보면 좋을 것 같아요! 😍

@redcontroller
Copy link
Copy Markdown

로그인/회원가입 모달에서 JS 넣은 부분 정말 부럽습니다... 대단해요! 😲🤣
JS를 좀 더 배우고 나서 예진님 브런치에 다시 와서 공부해보면 좋을 것 같다는 생각이 드는군요 🤔🤗

정말 훌륭하지만 첨언을 해보겠습니다.

  • 키보드 사용자는 Tab 키를 통해서 닫기 버튼으로 접근이 안되니 해당요소에 가상 클래스를 추가해주면 좋겠습니다. (:focus-visible)
  • 닫기 버튼과 마찬가지로 로그인 상태 유지에 키보드 접근이 안되니 위와 동일하게 적용해주시면 간단히 해결되겠군요!
  • input 태그의 placeholder 에 글꼴이 나눔고딕으로 렌더링 되는건데 혹시 의도하신건가요?

저는 회원가입, 아이디/비밀번호 찾기의 text-decoration: none; 으로 초기화 했었데,
hover 시에 밑줄이 그어지니 cursor:pointer; 만큼 직관적이네요. 👍

Copy link
Copy Markdown

@redcontroller redcontroller left a comment

Choose a reason for hiding this comment

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

checkbox 부분을 display:none; 속성을 주셨네요 🤔
코드가 정말 짧고 간결해지지만, 👏
여기에 대해서는 주의하라고 checkbox 커스터마이징 수업에서 한번 언급되었습니다.
display: none; 속성은 웹 접근성 관점 (스크린 리더 인식) 에서 취약합니다. 😥😳

대신에 overflow: hidden; 의 속성을 주는 기법을 사용해보면 어떨까요?

Comment on lines +85 to +88
/* 로그인 상태 유지 체크박스 */
.login-state input[type="checkbox"] {
display: none;
}
Copy link
Copy Markdown

@redcontroller redcontroller Aug 3, 2023

Choose a reason for hiding this comment

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

checkbox 부분을 display:none; 속성을 주셨네요 🤔
코드가 정말 짧고 간결해지지만, 👏
여기에 대해서는 주의하라고 checkbox 커스터마이징 수업에서 한번 언급되었습니다.
display: none; 속성은 웹 접근성 관점 (스크린 리더 인식) 에서 취약합니다. 😥😳

Comment 를 해드리고 싶어서 📚블로그 글을 인용해서 가져옵니다. 아래 글 참고해주세요. 🔍

그리고 수업 중에 나왔던 방식은 overflow: hidden; 으로 다음과 같은 속성을 주었습니다.
{
overflow: hidden;
width: 1px;
height: 1px;
clip-path: inset(50%);
}


CSS를 사용해 내용을 숨기려 할 때 사용하는 방법으로는,
(1) display:none
(2) visibility:hidden
(3) overflow:hidden
3가지 속성을 이용하여 영역을 숨김 처리할 수 있는데 각 특성마다 스크린 리더에서도 차이점을 보입니다.

(1) display:none 속성은센스리더(국내 스크린 리더기), Jaws, NVDA에서 모두 내용을 읽지 못합니다.
(2) visibility: hidden는 센스리더(국내 스크린 리더기)에서는 내용을 읽을 수 있었으나 Jaws, NVDA는 모두 내용을 읽지 못합니다.
(3) overflow:hidden의 경우 센스리더(국내 스크린 리더기), Jaws, NVDA 모두 내용을 읽을 수 있습니다.
국내 스크린리더만을 생각한다면 visibility: hidden 만 적용시켜도 내용을 읽는데 문제가 없으나
국외 스크린리더 사용자까지 생각을 한다면 overflow:hidden이 가장 좋은 것 같습니다.

출처 : https://whales.tistory.com/13


Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

모건님! 요거 수정했는데 제가 commit을 잘못달아놨네요 :(
감사합니다!

@YennieJ
Copy link
Copy Markdown
Collaborator Author

YennieJ commented Aug 7, 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.

4 participants