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

[우제윤] Week2 #13

Conversation

wjy308
Copy link
Collaborator

@wjy308 wjy308 commented Feb 24, 2024

요구사항

기본

  • PC사이즈만 고려해 주어진 디자인을 구현했나요?

  • HTML, CSS 파일을 Netlify로 배포했나요? (참고: https://creative-pony-d46807.netlify.app/)

  • 랜딩 페이지의 url path는 루트(‘/’)로 설정했나요?

  • title은 Linkbrary로 설정했나요?

  • 화면의 너비가 1920px 이상이면 하늘색 배경색은 너비를 꽉 채우도록 채워지고, 내부 요소들의 위치는 고정되고, 여백만 커지도록 했나요?

  • 화면의 너비가 1920px 보다 작아질 때, “Linkbrary” 로고의 왼쪽 여백 200px “로그인" 버튼의 오른쪽 여백 200px이 유지되고, 화면의 너비가 작아질수록 두 요소간 거리가 가까워지도록 했나요?

  • Linkbrary 아이콘은 클릭 시 루트 페이지(‘/’)로 이동하나요?

  • 클릭으로 기능이 동작해야 하는 경우, 사용자가 클릭할 수 있는 요소임을 알 수 있도록 cursor: pointer를 설정했나요?

  • “로그인”은 클릭 시 로그인 페이지(‘/signin’)로 이동하나요?

  • “링크 추가하기”는 클릭 시 회원가입(‘/signup’)로 이동하나요?

  • 화면의 너비가 1920px 이상이면 내부에 있는 요소간 동일한 간격을 유지하며 가운데 정렬해야 합니다.

  • 화면의 너비가 1920px 보다 작아질 때, 최하단에 있는 “codeit-2023”의 왼쪽 여백 104px과 SNS 아이콘들의 오른쪽 여백 104px을 유지하면서 가운데 있는 “Privacy Policy”, “FAQ” 요소와 각각 동일한 간격을 유지하며 가까워져야 합니다.

  • “Privacy Policy”, “FAQ”는 클릭 시 각각 Privacy 페이지(‘/privacy’), FAQ 페이지(‘/faq’)로 이동하나요?

  • 페이스북, 트위터, 유튜브, 인스타그램 아이콘은 클릭 시 각각의 홈페이지로 새로운 창이 열리면서 이동 하나요?

  • 아래로 스크롤 해도 “Linkbrary” 로고와 “로그인” 버튼이 있는 상단 네비게이션 바(Global Navigation Bar)영역이 최상단에 고정되게 했나요?

  • Linkbrary 로고 클릭시 루트 페이지(“/”)로 이동하나요?

  • 로그인 페이지, 회원가입 페이지 모두 로고 위 상단 여백이 동일한가요?

  • input 요소에 focus in 일 때, 테두리 색상이 파랑색인가요?

  • input 요소에 focus out 일 때, 테두리 색상이 회색인가요?

  • SNS 아이콘들은 클릭시 각각 “https://www.google.com/”, “https://www.kakaocorp.com/page/” 으로 이동하나요?

  • “회원 가입하기”는 클릭시 “/signup” 페이지로 이동하나요?

  • “로그인 하기”는 클릭시 “/signin” 페이지로 이동하나요?

심화

  • []사용자의 브라우저 font-size가 크고 작아짐에 따라 페이지의 요소간 간격, 요소의 크기, font-size 등 모든 크기와 관련된 값이 크고 작아지도록 설정했나요?

  • palette에 있는 color값들을 css 변수로 등록하고 사용했나요?

  • [] 비밀번호 input 요소 위에 비밀번호를 확인할 수 있는 눈 모양 아이콘을 추가했나요?

주요 변경사항

스크린샷

image

멘토에게

  • 아직 미흡하지만 잘 부탁 드립니다!

@wjy308 wjy308 marked this pull request as draft February 24, 2024 04:59
@wjy308 wjy308 marked this pull request as ready for review February 24, 2024 05:00
@wjy308 wjy308 changed the title 2주차 기능구현 [우제윤] Week2 Feb 24, 2024
@wjy308 wjy308 self-assigned this Feb 24, 2024
@wjy308 wjy308 requested a review from 1005hoon February 24, 2024 18:05
@wjy308 wjy308 added the 순한맛🐑 마음이 많이 여립니다.. label Feb 24, 2024
Copy link
Collaborator

@1005hoon 1005hoon left a comment

Choose a reason for hiding this comment

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

@wjy308

전반적으로 마크업을 잘 쪼개서 가져가고 있는 프로젝트에요!
아직 구조 정리를 많이 안해봤다보니 어쩔수 없이 네이밍이 조금 어려운 부분은 있는데,
이 부분들 고려해가면서 작업 개선시켜나가보면 금방 좋은 프로젝트가 나올 것 같습니다!

코드레벨에서 리뷰 남긴 요소들과
내일 진행할 멘토링 세션 참고하셔서 한번 리팩토링 작업 해볼까요?

고생 많으셨습니다!

Comment on lines +8 to +14
--cst_red-color:#FF5B56;
--cst_black-color:#111322;
--cst_darkGray-color:#3E3E43;
--cst_darkGray2-color:#9FA6B2;
--cst_midGray-color:#CCD5E3;
--cst_lightGray-color:#E7EFFB;
--cst_lightGray2-color:#F0F6FF;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@wjy308

cst를 빼보면 어떨까요?
그리구 혹시 네이밍 지정된게 있을까요?
있다면 지정된 네이밍을 활용하구 없다면 gray-100, gray-200과 같이 primary나 white, black등 지정색을 제외하고는 color-### 형태로 관리하는게 좋을듯해요

Comment on lines +22 to +36
header {
background-color: var(--cst_lightGray2-color);
}

a {
text-decoration: none;
}

nav {
position: sticky;
background-color: var(--cst_lightGray2-color);
top: 0px;
height: 50px;
width: 100%;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@wjy308

이런식으로 html element에 스타일을 지정하게 되면 이 요소를 활용하는 모든 콤포넌트에 스타일이 적용되게 된답니다!
클래스 네임을 주어 해당 클래스 이름을 지닌 요소만 스타일이 적용되도록 해볼까요?

Comment on lines +63 to +72
.login {
margin-right: 200px;
font-size: 16px;
font-weight: 600;
padding: 16px 20px;
height: 40px;
margin-top: 5px;
margin-bottom: 5px;
min-width: 85px;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@wjy308

만약 특정 block element에 대한 스타일이라면
.btn-login 이라는 이름이 조금더 좋아보여요

Comment on lines +88 to +93
.gra-PurToPink {
background-image: linear-gradient(to right, #6D6AFE, #FF9F9F);
background-clip: text;
-webkit-background-clip: text;
color: transparent;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@wjy308

좋습니다!
조금 더 명시적으로 이게 텍스트 색 그라디안트임을 표현해볼까요?
.text-gradient-purple-to-pink 처럼요!

Comment on lines +133 to +135
grid-template-columns: 1fr minmax(300px,998px) 1fr;
/* grid-template-columns: 1fr 998px 1fr;*/
grid-template-rows: repeat(4, 450px);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@wjy308

그리드 개수를 이렇게 지정해보리면 조금 위험할 수 있을것 같아요.
만약 section요소가 4개 이상 된다면 더이상 이 스타일은 valid하지 않잖아요.

그래서 레이아웃이 배치되는 형태는 기본적으로 최대 어느정도까지 넓어질건지를 기록하는 .container 와
각 섹션별로 레이아웃을 관장하는 요소들의 조합을 활용하는게 좋을것 같아요

Comment on lines +1 to +29
* {
box-sizing: border-box;
}

body {
/* 폰트 색상 weight 변수 */
--primary-color:#6d6afd;
--logo-weight:700;
margin: 0;
font-family: "Montserrat Alternates", 'Noto Sans KR', sans-serif;
background-color: #F0F6FF;
}

a {
text-decoration: none;
}

input{
width: 100%;
height: 60px;
border-radius: 8px;
padding: 18px 15px;
}

a.btn-home {
color: var(--primary-color);
font-weight: var(--logo-weight);
font-size: 45px;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@wjy308

이 요소들처럼 프로젝트 전반적으로 다 활용되는 스타일이라면

/styles
  /common.css
  /reset.css
  /ui.css

처럼 만들어 활용하는건 어떨까요

Comment on lines +31 to +37
.container {
margin: 238px 520px;
display: flex;
justify-content: center;
align-items: center;
flex-direction: column;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@wjy308

어디서 활용되는 컨테이너인지 조금 더 명확하면 좋겠어요!

Comment on lines +76 to +79
<div>
저장한 링크를<br>
<span class="gra-BluToBlu">공유</span>해 보세요.<br>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

@wjy308

단순 div를 텍스트 래퍼로 사용하는건 좋지 않은 활용방법인것 같아요
p 또는 h처럼 해당 텍스트가 일반 텍스트일지, 아니면 헤드라인일지를 명시하는 요소를 사용해볼까요?

<div class="mainTxt-description">
중요한 정보들을 검색으로 쉽게 찾아보세요.<br>
</div>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

@wjy308

전반적인 구조가 크게 나쁘지 않지만, 조금 더 시맨틱한 스타일링과 마크업을 시도해보면 좋겠어요!
아래 이미지를 참고해서 한번 개편해볼까요?
image

Comment on lines +16 to +41
<div class="container">
<div class="login-top">
<a class="btn-home" href="./index.html">Linkbrary</a><br><br>
회원이 아니신가요? <a class="signUp" href="./signUp.html">회원 가입하기</a>
</div>
<div class="login-mid">
<form>
<label name="email">이메일<br>
<input type="email" name="email" id="input-email"><br>
</label>
<label name="password"> 비밀번호<br>
<input type="password" name="password" id="input-password">
</label>
<button class="btn login" style="cursor: pointer;">로그인</button>
</form>
</div>
<div class="login-btm">
<div class="snsLogin-container">
소셜 로그인
<div class="btn-snsLogin">
<a href="#"><img class="google-login" src="./img/icn_goolge.png"></a>
<a href="#"><img class="kakao-login" src="./img/icn_kakao.png"></a>
</div>
</div>
</div>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

@wjy308

구조 자체는 좋습니다!
조금만 더 시맨틱한 구조로 레이아웃을 짜면 더 좋을것 같아요!
아래 구조를 참고해서 작업해볼까요?
image

@1005hoon 1005hoon merged commit cac7ed9 into codeit-bootcamp-frontend:part1-우제윤 Feb 28, 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