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 #18

Merged

Conversation

kuum97
Copy link
Collaborator

@kuum97 kuum97 commented Feb 24, 2024

요구사항

기본

  • 피그마 디자인에 맞게 PC사이즈 랜딩 페이지,회원가입 페이지를 만들어 주세요.
  • React와 같은 UI 라이브러리를 사용하지 않고 진행합니다.
  • 랜딩 페이지의 url path는 루트(‘/’)입니다.
  • 로그인 페이지의 url path는 ‘/signin’ 입니다.
  • 회원가입 페이지의 url path는 ‘/signup’ 입니다.
  • 아래로 스크롤 해도 “Linkbrary” 로고와 “로그인” 버튼이 있는 상단 네비게이션 바(Global Navigation Bar)가 최상단에 고정되게 해주세요.
  • HTML, CSS 파일을 Netlify로 배포해 주세요. (참고: https://www.codeit.kr/learn/5309)

심화

  • palette에 있는 color값들을 css 변수로 등록하고 사용해 주세요.
  • 비밀번호 input 요소 위에 비밀번호를 확인할 수 있는 아이콘을 추가해 주세요.

주요 변경사항

스크린샷

멘토에게

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

@kuum97 kuum97 marked this pull request as draft February 24, 2024 12:57
@kuum97 kuum97 changed the title Part1 권주현 week1 [권주현] Week2 Feb 24, 2024
@kuum97 kuum97 requested a review from 1005hoon February 25, 2024 08:45
@kuum97 kuum97 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Feb 25, 2024
@kuum97 kuum97 marked this pull request as ready for review February 25, 2024 08:48
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.

기대 이상 코드푸시인데요?
너무 잘 작성해주셨고, 구조를 멘토링때 이야기했던대로 최대한 잘 나누어 정리해보고자 한 게 느껴지는 코드들이였어요!
다만 전반적으로 아직 구조 잡는게 미숙하다보니 조금 더 개선하면 훨씬 좋은 프로젝트가 나올 듯 합니다!
이에 대한 디테일한 피드백은 코드단에 남겨두었으니 참고 부탁드려요!
@hoonj170214

이야 전반적으로 너무 좋은 구조로 작성된 프로젝트네요!
조금 더 욕심내서 구조적으로 가져가본다면 금방 HTML CSS 구조에 대해서는 익숙하게 구현될 것 같아요.

리뷰 내용에 앞서 질문 사항에 대해 먼저 답변 드리면요,
�grid도 Flex도 레이아웃을 지정하기 위한 스타일 프로퍼티에요.
따라서 grid에 minmax가 있는것처럼, flex에는 shrink, basis가 있답니다.
이 요소들을 잘 활용해서 어느정도 구역을 가져갈 지 정해본다면 좋을 것 같아요!

코드 전반적으로 굉장히 잘 구조가 잡혀있어요.
이제 몇가지만 더 개선한다면 정말 좋은 코드가 될 것 같아 기대가 되네요!
아직 css 네이밍이 미숙한듯 한데요, 이에 대해서는 코드 단의 리뷰에 피드백 남겨두었으니
한번 참고해서 진행해보시면 좋겠습니다!

너무 고생 많으셨고, 추가 질문사항이 더 있으시다면 디스코드에 언제든 질문 남겨주세요!

Comment on lines +7 to +11
--gray-darker: #3e3e43;
--gray-dark: #9fa6b2;
--gray: #ccd5e3;
--gray-light: #e7effb;
--gray-lighter: #f0f6ff;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

~~er 형태의 스타일 variant도 좋지만
디자이너가 갑자기 darker와 dark사이에 색 하나 더 추가할게요~ 하면 이거 네이밍 뭐라할지 골떄리잖아요 ㅋㅋ
그래서 실무에서는 gray-### 와 같이 숫자를 적용하는 형태를 많이 활용해요
한번 그렇게 변경해보는건 어떨까요?

Comment on lines +15 to +42
header {
width: 100%;
background-color: var(--gray-lighter);
display: flex;
flex-direction: column;
align-items: center;
}

footer {
width: 100%;
display: grid;
grid-template: auto / 1fr 1fr 1fr;
padding: 40px;
margin-top: 70px;
background-color: var(--black);
}

nav {
width: 100%;
max-width: 1920px;
position: fixed;
top: 0;
height: 110px;
display: flex;
align-items: center;
justify-content: space-between;
z-index: 1;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

header와 footer, 그리고 Nav가 여러 페이지에서 다양한 콤포넌트에서 활용될 수 있을텐데요
이렇게 글로벌 선언을 해두는건 위험해보입니다.
해당 스타일이 적용 될 클래스 이름을 만들어 관리해보는건 어떨까요?

Comment on lines +56 to +60
a {
text-decoration: none;
color: var(--white);
padding: 10px 30px;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

a태그는 인라인 요소지요
따라서 a태그에 패딩값이 적용되기 위해서는 inline-block요소로 표현되도록 수정되어야 할 거에요

Comment on lines +75 to +78
.sign__button,
.home__button :hover {
cursor: pointer;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

  1. button태그를 사용한다면 기본적으로 마우스 호버 시 cursor pointer가 적용될텐데 굳이 따로 명시하신 이유가 있을까요?
  2. :hover와 같이 수도 스타일링을 하고자 한다면 클래스이름과 붙어서 활용되어야 합니다! .home-button:hover 처럼요
  3. sign_button 의 호버 스타일도 하고자 했다면, sign_button:hover 처럼 여기에도 호버가 붙어야 해요

Comment on lines +80 to +114
.header__content {
width: 1920px;
margin-top: 140px;
display: flex;
flex-direction: column;
align-items: center;
overflow: hidden;
}

.header__content h1 {
font-size: 65px;
text-align: center;
}

.header__content span {
background: linear-gradient(to right, var(--primary), #ff9f9f);
background-clip: text;
color: transparent;
}

.header__content img {
position: relative;
top: 10px;
}

.header__content a {
margin-bottom: 20px;
background: var(--primary-gradient);
border-radius: 8px;
font-size: 20px;
font-weight: 600;
display: flex;
justify-content: center;
align-items: center;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

여기서 Header가 무엇일까요?
UI 개발에서 보통 페이지 레벨의 헤더라고 칭함은 GNB 또는 최 상단에 있는 네비게이션 바를 칭합니다.
만약 hero섹션을 뜻한거라면 네이밍이 변경되어야 할 것 같아요.

Copy link
Collaborator

Choose a reason for hiding this comment

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

또한 이 구조를 잡아본다면 다음과 같이 정리를 해볼 수 있겠네요!
image

Comment on lines +234 to +241
#email :focus,
#password :focus {
border-color: var(--primary);
}

#submit {
color: var(--white);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

id가 선언될 필요는 없어보입니다!
input의 타입을 선택할 수 있는데요
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/email

요 링크를 참고해 한번 해보면 어떨까요?

Comment on lines +255 to +275
.social-login {
display: flex;
}

.social-login__button {
padding: 5px;
border-radius: 100%;
display: flex;
justify-content: center;
align-items: center;
margin-right: 5px;
box-shadow: 1px 2px 2px var(--gray);
}

.google {
background-color: var(--white);
}

.kakao {
background-color: #f5e14b;
}
Copy link
Collaborator

@1005hoon 1005hoon Feb 28, 2024

Choose a reason for hiding this comment

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

@kuum97

로그인 폼 구조도 이런식으로 활용해보면 어떨까요?

/* Block */
.login-button {
  ...
}

/* pseudo-classes */
.login-button:hover {
...
}

.login-button:disabled {
...
}

Comment on lines +42 to +47
<input
class="sign__button"
id="submit"
type="submit"
value="로그인"
/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

특별한 이유가 없다면 폼 제출 목적으로는 Input보다 버튼을 활용하는게 더 좋아보여요!

<a class="goto__sign" href="../signin/index.html">로그인 하기</a>
</div>
</header>
<main class="form__container">
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

main이 폼 컨테이너로써 동작하기보단,
form 자체가 폼 컨테이너인게 더 맞지 않을까 싶기는 합니다!
구조를 한번 생각해보면 좋을것 같아요!

Comment on lines +23 to +30
<label for="email">이메일</label>
<input
id="email"
type="email"
name="email"
placeholder="이메일을 입력해주세요."
required
/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

각 라벨과 input요소를 하나의 div그룹으로 묶어두면 이후에 관리하기가 수월할거에요!

@1005hoon 1005hoon merged commit 67773d1 into codeit-bootcamp-frontend:part1-권주현 Feb 28, 2024
@kuum97 kuum97 deleted the part1-권주현-week1 branch March 6, 2024 08:24
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