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

[최순오]sprint 1 #18

Merged
merged 1 commit into from
Aug 13, 2024
Merged

[최순오]sprint 1 #18

merged 1 commit into from
Aug 13, 2024

Conversation

joonohjoonoh
Copy link
Collaborator

@joonohjoonoh joonohjoonoh commented Aug 11, 2024

요구사항

  • [x]PC사이즈만 고려해 주어진 디자인을 구현합니다.
  •  HTML, CSS 파일을 Netlify로 배포해 주세요. 배포주소: https://darling-phoenix-063f2b.netlify.app/
  • 랜딩 페이지의 url path는 루트(‘/’) 입니다.
  • title은 “판다마켓”으로 설정해 주세요
  • 화면의 너비가 1920px 이상이면 하늘색 배경색은 너비를 꽉 채우도록 채워지고, 내부 요소들의 위치는 고정되고, 여백만 커지도록 해주세요.
  •  화면의 너비가 1920px 보다 작아질 때, “판다마켓” 로고의 왼쪽 여백 200px“로그인" 버튼의 오른쪽 여백 200px이 유지되고,
화면의 너비가 작아질수록 두 요소간 거리가 가까워지도록 해주세요.
  • 
 클릭으로 기능이 동작해야 하는 경우, 사용자가 클릭할 수 있는 요소임을 알 수 있도록 cursor: pointer를 설정해 주세요.
  • 판다마켓” 클릭 시 루트 페이지(‘/’)로 이동시켜주세요.
  • “구경하러 가기" 클릭 시 (“/items”)페이지로 이동시켜주세요.(빈 페이지)
  •  “로그인”버튼 클릭 시 로그인 페이지(‘/login’)로 이동합니다.
  • “구경하러가기”버튼 클릭 시(’/items’)로 이동합니다.
  • 페이스북, 트위터, 유튜브, 인스타그램 아이콘은 클릭 시 각각의 홈페이지로 새로운 창이 열리면서 이동 합니다.
  •  “Privacy Policy”, “FAQ”는 클릭 시 각각 아래 페이지로 이동합니다
- Privacy 페이지(‘/privacy’), FAQ 페이지(‘/faq’)

심화

주요 변경사항

스크린샷

스크린샷 2024-08-12 오후 5 01 41

image

멘토에게

-위에 선택 사항은 잘 몰라서 코멘트로 진행 하겠습니다.

-우선 알려주셨던 피그마 개발모드로 등록은 됐는데 슬라이드 버튼이 안되더라구요.
그래서 이미지 다운 받고 피그마 미리보기 화면 보면서 피그마의 대화집인가요? 그내용에 충실하게 하도록
전체적으로 구성하였습니다.

-처음에 사진만 붙이면 되는 줄알고 사진 붙였다가 샘플이죠
특정 이미지에 버튼구성 배경색 지정해야하는걸 깨닫고 다시 작업 하였습니다.

-그리고 해더와 풋터를 구성하였고 풋터에서 privacy faq는 정해준 경로를 만들라고 했지만
html 만들어서 연결 시켰습니다. 해더의 로그인 버튼도 마찬가지 였습니다.

-그다음으로 탑 바텀 배너를 작업하였고
마지막에 가운데 3단 이미지를 작업하였습니다.

-맨토님께 상담드린데로 css 학습에 집중이 되지않아서 검색해서 긁어오고 개발도구로 css적용해 보면서
꾸역꾸역 수정하였습니다.

-그리고 클래스도 잘 활용을 못한게 공통부분은 동일하게 정용할 수있어야했는데 클래스를 마치 id처럼 써서
css에 class를 남발한것도 문제 입니다.
이 화면이 반응형으로 할경우 전부 깨질게 보여서 더 걱정입니다.

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

@joonohjoonoh joonohjoonoh requested a review from GANGYIKIM August 11, 2024 05:11
@joonohjoonoh joonohjoonoh added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 순한맛🐑 마음이 많이 여립니다.. labels Aug 11, 2024
@GANGYIKIM GANGYIKIM changed the title [최순오]sprint N [최순오]sprint 1 Aug 12, 2024
Copy link
Collaborator

@GANGYIKIM GANGYIKIM left a comment

Choose a reason for hiding this comment

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

순오님 첫번째 스프린트 미션 고생하셨습니다.
꾸역꾸역 작업하셨다고 하셨는데 우선 제출하신게 중요하다고 생각합니다! 👍
아래는 전반적인 피드백이니 다음 미션제출하실 때 참고해주세요~


  • 동일한 내용에 대해서는 코드에서 한번만 코멘트를 답니다.
  • 커밋을 더 단위별로 쪼개주시면 좋을 것 같습니다.
  • 이미지를 한번 정리하셔서 필요한것만 남기시면 좋겠습니다.
  • 이미지 명도 어떤 이미지인지 알 수 있는 이름으로 변경해주시면 좋겠습니다.
  • class를 남발하셨다는게 공통적으로 쓸 수 있는 것들도 그냥 class를 쪼개서 작성하셨다는 의미인 것 같습니다. 이번에 그렇게 느끼셨으니 다음에는 조금 더 신경써서 작성하시면 좋을 것 같습니다.
  • 피그마 계정을 학생계정으로 전환하신 후 피그마 파일을 복제해가셔야 dev 모드가 활성화됩니다.
스크린샷 2024-08-13 오전 11 45 54

@@ -0,0 +1,87 @@
<!DOCTYPE html>
<html lang="en">
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:

Suggested change
<html lang="en">
<html lang="ko">

<body>
<header class="header">
<a href="index.html">
<img src="./img/lg/Property 1=md.png" alt="로그인">
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:
이미지 명에 공백은 들어가지 않는 것이 좋습니다.
또한 어떤 이미지인지 알 수 있는 이름(logo.png)이면 좋을 것 같습니다.

Suggested change
<img src="./img/lg/Property 1=md.png" alt="로그인">
<img src="./img/logo.png" alt="판다마켓 로고">

Comment on lines +21 to +26
<button class="btn top">
<p>일상의 모든 물건을 <br>거래해보세요</p>
<a href="./links/item.html">
<img class="itemlink" src="./img/btn/state=active.png" alt="구경하러가기">
</a>
</button>
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:
button안에 a 태그가 들어가야할 이유가 있을까요?
페이지 이동을 시켜야하는 것이니 아래처럼 작성해도 될 것 같습니다.

Suggested change
<button class="btn top">
<p>일상의 모든 물건을 <br>거래해보세요</p>
<a href="./links/item.html">
<img class="itemlink" src="./img/btn/state=active.png" alt="구경하러가기">
</a>
</button>
<�div class="btn top">
<p>일상의 모든 물건을 <br>거래해보세요</p>
<a href="./links/item.html">
<img class="itemlink" src="./img/btn/state=active.png" alt="구경하러가기">
</a>
</div>

</section>
</div>
<div>
<section class="bottom_banner">
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
section이 너무 많이 사용되는 것 같습니다.
mdn의 section 페이지를 한번 읽어보시는 걸 추천드립니다.
Usage notes 부분에 스타일링을 위해서만이라면 div 태그를 추천드리고 있습니다.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/section#usage_notes

<a href="links/faq.html">FAQ</a>
</section>
<section class="foot-right">
<a href="https://www.facebook.com/"><img src="/img/icons/ic_facebook.png" alt="facebook"></a>
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
외부영역으로 이동이 되는 경우는 새창으로 열어도 좋을 것 같습니다.

Suggested change
<a href="https://www.facebook.com/"><img src="/img/icons/ic_facebook.png" alt="facebook"></a>
<a href="https://www.facebook.com/" target='_blank'><img src="/img/icons/ic_facebook.png" alt="facebook"></a>

Comment on lines +1 to +3
body {

}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
의미없는 빈 태그는 지워주세요.

Suggested change
body {
}

Comment on lines +15 to +16
margin-left: 360px;
margin-right: 360px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
아래도 동일하게 작동합니다.

Suggested change
margin-left: 360px;
margin-right: 360px;
margin: 0 360px; // 만약 중앙정렬을 하고 싶으신거라면 margin: 0 auto도 가능합니다.

background-color: transparent;
padding: 0;
border: none;
font-size: 1.5rem;
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:
rem 같은 반응형 단위를 사용하실때는 html에 font-size 속성을 선언해주시는 것이 좋습니다.
rem은 root 폰트 사이즈 기반으로 계산되는 반응형 단위입니다. 이걸 적용해주지 않으면 해당 폰트가 어떤 것 기준으로 계산되는지 알 수 없습니다

}

h5 {
color: rgb(44, 67, 214);
Copy link
Collaborator

Choose a reason for hiding this comment

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

�P2:
rgb를 사용해야 할 이유가 없다면 hex code로 컬러값을 선언해주세요~

Comment on lines +62 to +78
.main-item {
display: flex;
align-items: center;
justify-content: space-around; /* 이미지와 텍스트 같이 */
background-color: #f8f8f8; /* 각 섹션의 배경색 */
padding: 20px;
border-radius: 10px; /* 둥근 모서리 추가 */
}

.main-item.middle {
display: flex;
align-items: center;
justify-content: space-around; /* 이미지와 텍스트 같이 */
background-color: #f8f8f8; /* 각 섹션의 배경색 */
padding: 20px;
border-radius: 10px; /* 둥근 모서리 추가 */
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
완전 동일한 속성이 두번 선언될 필요가 있을까요?
개발자의 의도가 궁금해지는 코드는 가독성 측면에서 좋지 않으니 가능하면
왜 이렇게 작성하셨는지 알 수 있도록 작성해주시면 좋겠습니다.

Suggested change
.main-item {
display: flex;
align-items: center;
justify-content: space-around; /* 이미지와 텍스트 같이 */
background-color: #f8f8f8; /* 각 섹션의 배경색 */
padding: 20px;
border-radius: 10px; /* 둥근 모서리 추가 */
}
.main-item.middle {
display: flex;
align-items: center;
justify-content: space-around; /* 이미지와 텍스트 같이 */
background-color: #f8f8f8; /* 각 섹션의 배경색 */
padding: 20px;
border-radius: 10px; /* 둥근 모서리 추가 */
}
// 차라리 이렇게..?
.main-item,
.main-item.middle {
display: flex;
align-items: center;
justify-content: space-around; /* 이미지와 텍스트 같이 */
background-color: #f8f8f8; /* 각 섹션의 배경색 */
padding: 20px;
border-radius: 10px; /* 둥근 모서리 추가 */
}

@GANGYIKIM GANGYIKIM merged commit e671aa6 into codeit-bootcamp-frontend:Basic-최순오 Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 순한맛🐑 마음이 많이 여립니다..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants