-
Notifications
You must be signed in to change notification settings - Fork 39
[이유진] sprint2 #128
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
[이유진] sprint2 #128
The head ref may contain hidden characters: "Basic-\uC774\uC720\uC9C4-sprint2"
Conversation
addiescode-sj
left a comment
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.
수고하셨습니다!
주요 리뷰 포인트
- 반응형 이미지 리소스 최적화
- 접근성 향상
- 코드 중복 제거
| <img | ||
| class="visibility-btn-off" | ||
| src="../images/visibility_off.svg" | ||
| alt="비밀번호 숨기기" | ||
| width="24px" | ||
| height="24px" /> | ||
| <img | ||
| class="visibility-btn-on" | ||
| src="../images/visibility_on.svg" | ||
| alt="비밀번호 보기" | ||
| width="24px" | ||
| height="24px" /> |
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.
이미지의 경우 하나로 관리하기위한 팁을 드리자면
input checkbox로 만드시고, checked 상태를 활용해 알맞는 이미지가 띄워지게끔 조절해보시면 쉽게 이미지를 토글할 수 있답니다 :)
질문에 대한 답변
1,2번 질문의 경우 본문에서 코멘트로 상세히 답변드렸습니다 :) rem은 기본 폰트 사이즈에 기반한 상대단위여서 만약 특정 브레이크포인트에서 rem을 사용한 단위들이 scale 조절하듯이 일괄적으로 줄어드는걸 원한다면 사용하시면됩니다. 이때 선 두께, 모서리 둥글기등 디자인 의도를 고려했을때 변하지않기 원한다면 px로 사용하셔야겠죠? 컨테이너에 레이아웃 빛 배치를 유연하게 결정할수있도록 도와주는 flex 및 grid 속성을 적용해주면 유연하게 대비하실수있고, 주요 브레이크 포인트 이외에 중간 구간에서도 추가적인 대응이 필요하다면 그 경우에만 브레이크포인트를 추가해 스타일을 재정의해주시면될것같습니다. CLS 방지를 위해 img태그에 width, height를 명시하게되면 브라우저는 지정된 width/height 비율로 이미지의 박스(공간)를 미리 잡아줍니다. a태그의 경우 role 속성 생략이 가능하지만, 이것을 링크가 아닌 다른 역할로 쓸 때 (e.g 태그를 자바스크립트로 버튼처럼 동작하게 만들 때)가 다수 존재하기때문에 role을 명시해주시는게 접근성에 좋습니다. @youjin-hub |
배포 링크
이유진-판다마켓
(현재 index 페이지는 모바일 레이아웃만 잡혀있습니다.)
요구사항
기본 기능
심화 기능
주요 변경사항
aria-label및role속성 추가img태그에 width/height 명시rel="noopener noreferrer"추가style.css를reset.css,common.css,variables.css로 분리index.html모바일 퍼스트로 레이아웃 수정 (추후 데스크탑 반응형 추가 예정)index.htmlheader 상단에 고정스크린샷
멘토에게
비밀번호 input과 가시성 토글 버튼 구조가 현재
div로 묶여 있는데,label로 감싸는 형태가 더 적합할까요? 접근성 측면에서 개선이 필요한지 궁금합니다.모바일과 데스크탑에서 다른 로고를 보여주기 위해
img태그를 두 개를 넣고display: none으로 처리하는 방법이 괜찮은지 조언을 듣고 싶습니다. (현재 태그에는 추가되어있지 않습니다)현재 style.css에서는
px와rem단위를 혼용하고 있습니다. 미디어 쿼리에서 별도로rem크기를 조정하지 않는다면rem을 사용할 이유가 없는지, 단일화하는 것이 좋을지 의견이 궁금합니다. (반응형에rem사용이 좋다는 건 이해했지만, 피그마 시안을 보고 구현하다보니 계속 고정값을px로 주게되는 것 같습니다...)미디어쿼리를 사용할 때, 해상도 구간 중간(예: 768px 이상인데 850px쯤)에서 레이아웃이 어색해지는 경우가 있습니다. 이런 경우, 중간 해상도도 최대한 자연스럽게 흐르도록 추가 조정을 해야 할까요? 아니면 주요 브레이크포인트(모바일/태블릿/PC)만 맞추는 것을 우선시하는 게 괜찮을까요?
CLS 방지를 위해
img태그에width와height를 명시할 때, 데스크탑/태블릿/모바일 중 어떤 해상도 기준으로 크기를 설정하는 게 좋은가요? (예를 들어 데스크탑에서 크게 보이고 모바일에서는 줄어드는 이미지라면, 데스크탑 기준width/height를 넣어야 할까요? 아니면 모바일 기준으로 넣고, 데스크탑에서 CSS로 키우는 게 나을까요?)a태그는 기본적으로 링크 역할을 가지고 있어서role속성이 생략 가능한 것으로 알고 있었는데, 멘토님께서role속성 추가를 권장해주신 이유를 더 자세히 알고 싶습니다.반응형까지 고치다가 미션3이 반응형인 걸 지금 확인해서 일단 제출합니다. 질문 이외에도 부족한 부분이 있다면 조언 주시면 감사하겠습니다!