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

[이종욱] sprint1 #43

Conversation

Bersk3r
Copy link
Collaborator

@Bersk3r Bersk3r commented Mar 15, 2024

요구사항

기본

  • “로그인”버튼 클릭 시 로그인 페이지(‘/signin’)로 이동합니다
  • “구경하러가기”버튼 클릭 시(’/items’)로 이동합니다.
  • 랜딩 페이지의 url path는 루트(‘/’) 입니다.
  • title은 “판다마켓”으로 설정해 주세요.
  • 화면의 너비가 1920px 이상이면 하늘색 배경색은 너비를 꽉 채우도록 채워지고, 내부 요소들의 위치는 고정되고, 여백만 커지도록 해주세요.
  • 화면의 너비가 1920px 보다 작아질 때, “판다마켓” 로고의 왼쪽 여백 200px“로그인" 버튼의 오른쪽 여백 200px이 유지되고, 화면의 너비가 작아질수록 두 요소간 거리가 가까워지도록 해주세요.
  • 클릭으로 기능이 동작해야 하는 경우, 사용자가 클릭할 수 있는 요소임을 알 수 있도록 cursor: pointer를 설정해 주세요.
  • “판다마켓” 클릭 시 루트 페이지(‘/’)로 이동시켜주세요.
  • “구경하러 가기" 클릭 시 (“/items”)페이지로 이동시켜주세요.(빈 페이지)
  • “Privacy Policy”, “FAQ”는 클릭 시 각각 아래 페이지로 이동합니다

    - Privacy 페이지(‘/privacy’) 

    - FAQ 페이지(‘/faq’)
    -[x] 페이스북, 트위터, 유튜브, 인스타그램 아이콘은 클릭 시 각각의 홈페이지로 새로운 창이 열리면서 이동 합니다.

심화

  • 사용자의 브라우저가 크고 작아짐에 따라 페이지의 요소간 간격, 요소의 크기, font-size 등 모든 크기와 관련된 값이
    크고 작아지도록 설정해 보세요.(설정값은 자유입니다)

주요 변경사항

  • 최초 제출이므로 아직까지 큰 변경 사항은 없습니다.

스크린샷

image
sprintwook.netlify.app

멘토에게

  • 멘토님 해당 소스 코드 내에서 header와 footer 부분에서 html 밖에 공백이 생기고 있습니다.
    • 원인 사항으로는 특정 요소가 html보다 큰 경우 발생되는 문제라는 점이라는 답변을 찾았으나
      현재 원인 중에서 발생될만한 요소가 어떤 것인지를 찾지 못했습니다.
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@Bersk3r Bersk3r added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Mar 15, 2024
@Bersk3r Bersk3r requested a review from addiescode-sj March 15, 2024 12:00
Copy link
Collaborator

@addiescode-sj addiescode-sj left a comment

Choose a reason for hiding this comment

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

좋은 시도들이 많이 보인 PR이었네요 👍
BEM 네이밍 규칙, css-vars 사용법정도 추가적으로 이번 기회에 알아가시면 좋을것같고,
폴더 구조 관련해서도 참고될만한 코멘트 드려봤습니다!
코멘트 드린 부분 참고해서, 다음주 미션때 리팩터링하시고 미션 진행하시면 좋을것같아요.

바로 머지해드리겠습니다, 수고하셨어요! ㅎㅎ 👏

<link href="/assets/fonts/index.css" rel="stylesheet" type="text/css">
</head>
<body>
<header>
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.

폴더 구조화 시도해보셨네요! 좋습니다 👍
시도 정말 좋은데, 폴더를 만드는 기준도 구조화되면 더 괜찮을것같아요.

예를 들자면, pages폴더를 따로 만든다고했을때 -> 이 안에는 각 페이지에 해당하는 폴더들이 있고, 해당 페이지 폴더 하위에 각 페이지에 연관된 html css js파일이 모여있게되겠죠? 참고해보시면 좋을 듯 합니다! :)

<a class="login" href="/signin">로그인</a>
</header>
<main>
<section class="panda intro">
Copy link
Collaborator

Choose a reason for hiding this comment

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

html+css를 사용할 시 클래스이름의 경우 어떤 네이밍규칙을 적용해야한다는 의무사항은 없지만,
보통 BEM 규칙을 많이 따르는 편입니다 :)
이 경우 block요소니까 공백보다는 대시(-)을 사용해 구분해주는게 바람직해보여요.
아래 아티클 참고해보세요!

BEM 방법론 (네이밍 규칙)

</div>
<img src="/assets/images/panda2.svg" />
</section>
<section class="panda-section left-start">
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기에 쓰인 클래스이름도 BEM 규칙 따라서 적용해보시면 조금 달라지겠죠? ㅎㅎㅎ

<img src="/assets/images/panda6.svg" />
</section>
</main>
<div class="footer">
Copy link
Collaborator

Choose a reason for hiding this comment

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

footer라는 시맨틱 태그도 있답니다~! :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

굳굳 👍 이 파일이 styles 폴더 아래에 있는건 좋은 선택같아요. 이유는, 보통 앱 디렉토리 루트에 있는 styles 폴더 하위에 있는 파일들은 프로그램안에서 여기저기서 쓰이는 공용 css파일, 혹은 스타일링에 관련된 utils 파일을 모아두는편이기때문입니다.
따라서, style.css는 이런 파일이 아니고 index.html을 위한 css파일이기때문에 이름 변경 + 이동을 고려해보면 좋겠죠? :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

위에서 드린 코멘트를 이유로,
저라면 index.html과 동일한 위계를 가지게끔 파일을 루트로 이동하고 이름도 index.css로 바꿔줬을것같습니다! :)

justify-content: space-between;
padding: 11px 200px;
flex: 0 1 auto;
/*gap: calc();*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

PR 올리시기전에 쓰이지않는 주석은 제거해주세요~

@@ -0,0 +1,197 @@
* {
Copy link
Collaborator

Choose a reason for hiding this comment

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

요런 코드의 경우에는 멘토링때 말씀드렸던것처럼 common.css에 따로 빼두는게 좋을것같아요!
common.css에는 이런 box-sizing속성뿐만 아니라 base font 스타일, css-vars를 사용한 컬러코드같은것을 적용해둘수있답니다! css-vars의 경우 이 아티클 참고해보세요 :)

https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties

@addiescode-sj
Copy link
Collaborator

addiescode-sj commented Mar 18, 2024

멘토의 답변

멘토님 해당 소스 코드 내에서 header와 footer 부분에서 html 밖에 공백이 생기고 있습니다. 원인 사항으로는 특정 요소가 html보다 큰 경우 발생되는 문제라는 점이라는 답변을 찾았으나 현재 원인 중에서 발생될만한 요소가 어떤 것인지를 찾지 못했습니다.

우선, padding-inline: 404px 195px 을 해지하시면 html 바깥으로 컨텐츠가 삐져나오는 현상이 사라지는것을 보실수있습니다!
이건 writing-mode 속성도 고려해야해서 복잡하게 스타일링하시기보다는 단순히 padding값, margin값, flex box를 이용하시는게 제일 깔끔할것같아요. 리팩터링 포인트 몇개 짚어드리겠습니다 :)

  • grid 쓰신걸 빼고 flexbox를 사용해보세요! grid는 flexbox보다 아이템 크기에 대한 유연성이 떨어집니다. 물론 grid를 쓰셔도되긴하지만 코드가 더 복잡해질수있어 이 경우에는 flexbox를 사용해 레이아웃 짜시는게 더 적절해보여요.
  • 전체 container width를 최대 1200px로 적용해주시고, 그 안에서 세부 레이아웃을 나눠보세요. flex 속성들 + 필요한 경우 margin, padding값을 적용해주는것으로도 충분히 해결하실수있습니다.

@Bersk3r 이 코멘트도 리팩터링하실때 꼭 참고해주세요~! :)

@addiescode-sj addiescode-sj merged commit f10627e into codeit-bootcamp-frontend:part1-이종욱 Mar 18, 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