Skip to content
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

[정윤호] sprint3 #76

Merged

Conversation

KingNono1030
Copy link

@KingNono1030 KingNono1030 commented May 9, 2024

요구사항

  • Github에 PR(Pull Request)을 만들어서 미션을 제출합니다.
  • 피그마 디자인에 맞게 페이지를 만들어 주세요.
  • React와 같은 UI 라이브러리를 사용하지 않고 진행합니다.

기본

  • 공통

  • 브라우저에 현재 보이는 화면의 영역(viewport) 너비를 기준으로 분기되는 반응형 디자인을 적용합니다.
    (PC: 1200px 이상 / Tablet: 768px 이상 ~ 1199px 이하 / Mobile: 375px 이상 ~ 767px  이하)

  • 랜딩 페이지

  • Tablet 사이즈로 작아질 때 “판다마켓” 로고의 왼쪽에 여백 24px, “로그인” 버튼 오른쪽 여백 24px을 유지할 수 있도록 “판다마켓” 로고와 “로그인" 버튼의 간격이 가까워집니다.

  • Mobile 사이즈로 작아질 때 “판다마켓” 로고의 왼쪽에 여백 16px, “로그인” 버튼 오른쪽 여백 16px을 유지할 수 있도록 “판다마켓” 로고와 “로그인" 버튼의 간격이 가까워집니다.

  • 화면 영역이 줄어들면 “Privacy Policy”, “FAQ”, “codeit-2024”이 있는 영역과 SNS 아이콘들이 있는 영역의 간격이 줄어듭니다.

  • 로그인, 회원가입 페이지 공통

  • Tablet 사이즈에서 내부 디자인은 PC사이즈와 동일합니다.

  • Mobile 사이즈에서 좌우 여백 16px 제외하고 내부 요소들이 너비를 모두 차지합니다.

  • Mobile 사이즈에서 내부 요소들의 너비는 기기의 너비가 커지는 만큼 커지지만 400px을 넘지 않습니다.

심화

  • 페이스북, 카카오톡, 디스코드, 트위터 등 SNS에서 Linkbrary 랜딩 페이지(“/”) 공유 시 좌측 예시와 같은 미리보기를 볼 수 있도록 랜딩 페이지 메타 태그를 설정해 주세요.
  • 미리보기에서 제목은 “판다 마켓”, 설명은 “일상의 모든 물건을 거래해보세요”로 설정합니다.
  • 주소와 이미지는 자유롭게 설정하세요.

주요 변경사항

스크린샷

스크린샷 2024-05-03 20 37 16

스크린샷 2024-05-03 20 36 26

스크린샷 2024-05-03 20 36 40

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@KingNono1030 KingNono1030 deleted the Basic-정윤호 branch May 9, 2024 09:23
@KingNono1030 KingNono1030 restored the Basic-정윤호 branch May 9, 2024 09:24
@KingNono1030 KingNono1030 reopened this May 9, 2024
@KingNono1030 KingNono1030 reopened this May 9, 2024
@KingNono1030 KingNono1030 requested a review from kiJu2 May 9, 2024 15:11
@kiJu2
Copy link
Collaborator

kiJu2 commented May 11, 2024

수고 하셨습니다 ! 위클리 미션 하시느라 정말 수고 많으셨어요.
학습에 도움 되실 수 있게 꼼꼼히 리뷰 하도록 해보겠습니다.

@kiJu2
Copy link
Collaborator

kiJu2 commented May 11, 2024

커밋 메시지 정말 깔끔하군요 👍👍👍

Comment on lines +7 to +12
const isFormFilled = inputs.every((item) => item.value);
if (isFormFilled) {
button.disabled = false;
} else {
button.disabled = true;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

피드백 반영 좋습니다 ~! 😊👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

또한, 다음과 같이 작성할 수도 있어요:

Suggested change
const isFormFilled = inputs.every((item) => item.value);
if (isFormFilled) {
button.disabled = false;
} else {
button.disabled = true;
}
const isFormFilled = inputs.every((item) => item.value);
button.disabled = isFormFilled;

어차피 isFormFilledboolean이 확정이기에 바로 대입하셔도 됩니다 😊

Copy link
Author

Choose a reason for hiding this comment

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

오 생각 못했는데 훨씬 깔끔해졌어요!

@@ -3,9 +3,27 @@
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<meta
name="description"
content="당근마켓 게섯거라...! 요즘 대세는 푸바오 마켓..!"
Copy link
Collaborator

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋ 😂😂😂

@@ -31,7 +49,7 @@
<header>
<div class="header-home__wrapper bg_wrapper">
<div class="header-home">
<h2 class="header-home__title h2_title">
<h2 class="header-home__title header_text">
Copy link
Collaborator

Choose a reason for hiding this comment

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

오호. 훨씬 낫군요 !

목적이 더욱 명확해졌어요 굳굳 👍👍👍

@@ -101,3 +101,31 @@ main {
.translucent {
opacity: 0.5;
}

@media screen and (max-width: 767px) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

반응형 로직이 적은 것 보니 설계를 잘하신 것 같군요 👍👍

/* http://meyerweb.com/eric/tools/css/reset/
Copy link
Collaborator

Choose a reason for hiding this comment

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

reset.css 적용 하셨군요 ! 좋습니다 ! 👍👍

Comment on lines +163 to +165
main {
gap: 16px;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 파일은 style.css입니다. 모든 페이지의 main에 스타일을 주고싶으신게 맞으실까요?

만약 아니라면 페이지 별 css 파일에 해당 스타일을 적용해보시는 것도 고려해보시면 어떨까 싶습니다 😊

Copy link
Author

Choose a reason for hiding this comment

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

이 부분은 따로 클래스 지정자를 사용해서 스타일링해보겠습니다!

.feature__title {
margin-bottom: 16px;
}

.main-footer__wrapper {
margin-top: 32px;
height: 927px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

height를 �고정하는 것보다는 요소를 감싸도록 만드는건 어떨까요?

푸터의 컨텐츠를 감싸고 padding을 조절하여 height를 조절하는게 어떨까요? height를 고정하게 된다면 예기치 못한 오버플로우가 발생할 수 있습니다 😊

margin-top: 32px;
padding-top: 121px;
margin-top: 24px;
background-size: 497.91px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

헙 ㄷㄷㄷ.. 어쩌다가 .91px이 되었을까요..?

Copy link
Author

Choose a reason for hiding this comment

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

피그마 디자인으로 제공된 픽셀가를 그대로 가져왔는데,
한번이라도 고민 해볼걸 그랬어요

@kiJu2
Copy link
Collaborator

kiJu2 commented May 11, 2024

추가로 페이지 별 .js를 작성해서 적용해보는 것도 한 번 고려해보세요 😊
컴포넌트 별로 구분하게 되면 단위가 많아져서 나중에 파일이 너무 많아져버릴 수도 있습니다 !

@kiJu2
Copy link
Collaborator

kiJu2 commented May 11, 2024

정말 수고 많으셨습니다 윤호님 !
이번 코드는 정말 깔끔하군요 ! 칭찬 세례였습니다 !
멈출 수 없는 학습 욕구를 가지신 모습... 그리고 팀 리더로서 정말정말정말 든든했습니다. 다음에도 꼭 꼭 함께하고 싶어요.

윤호님 같은 적극적인 멤버가 있었기에 멘토링 미팅도 정말 즐겁게 임했던 것 같아요. 😊
수고 정말 많으셨고. 앞으로도 지금과 같은 모습으로 승승장구하시길 기원하겠습니다 !!(사실 걱정이 잘 안됩니다 ㅎㅎㅎ 미래가 보이는 인재군요.)

@kiJu2 kiJu2 merged commit 1c9b682 into codeit-bootcamp-frontend:Basic-정윤호 May 11, 2024
1 check passed
@KingNono1030 KingNono1030 requested a review from dev-kjy May 13, 2024 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants