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

Conversation

jose0229
Copy link
Collaborator

요구사항

기본

  • 기본 요구사항은 모두 충족하였습니다.

심화

  • 심화 요구사항은 모두 충족하지 못했습니다.

주요 변경사항

스크린샷

image

멘토에게

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

@jose0229 jose0229 changed the base branch from main to part1-임상훈 February 26, 2024 15:42
@jose0229 jose0229 changed the title 기능구현 [임상훈]Week2 Feb 26, 2024
@jose0229 jose0229 marked this pull request as draft February 26, 2024 15:43
@jose0229 jose0229 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Feb 26, 2024
@o-seung-yeon o-seung-yeon marked this pull request as ready for review February 27, 2024 00:31
@o-seung-yeon
Copy link
Collaborator

@jose0229 올려주신 pr 이 draft 상태라 merge 를 할 수 없어 merge 가능한 상태(pr 을 완전히 제출한 상태)로 변경했습니다.
다음 제출하실 땐 draft 가 아닌 완전한 제출로 올려주세요~

Copy link
Collaborator

@o-seung-yeon o-seung-yeon left a comment

Choose a reason for hiding this comment

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

첫번째 미션 고생하셨습니다.

다음 미션은 기한맞춰 제출해주시고,
pr description 작성하실 때 배포한 Url 포함해서 불필요한 불렛같은건 지워서 올려주세요~

<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Linkbrary</title>
<link rel = "stylesheet" href = "main_stylesheet.css">
Copy link
Collaborator

Choose a reason for hiding this comment

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

등호 기호 양 옆에 공란을 추가해주신 이유가 가독성 때문일까요?
일반적으로 공란을 두지 않는게 읽기 좋기 때문에 다른 부분들도 수정 부탁드려요~

// asis
<link rel = "stylesheet" href = "styles.css">

// todo
<link rel="stylesheet" href="styles.css">

<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link href="https://fonts.googleapis.com/css2?family=Acme&display=swap" rel="stylesheet">

<link rel="preconnect" href="https://fonts.googleapis.com">
Copy link
Collaborator

Choose a reason for hiding this comment

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

구글 폰트를 사용하시는 것 같은데 중복으로 불러오는 링크가 있네요. 확인후 정리해 주세요~

</head>

<body>
<header id = "header">
Copy link
Collaborator

Choose a reason for hiding this comment

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

등호 기호 양 옆에 공백을 주셨네요.
일반적으로 공백없는 형태를 많이 보실텐데요. 가독성 면에서도 더 보기 좋아서 html 작성하실 땐 아래처럼 작업해주시면 좋을 것 같습니다.

<header id="header"></header>

<header id = "header">

<a href = "/" target="_self">
<img src = "pictures/Linkbrary.png">
Copy link
Collaborator

Choose a reason for hiding this comment

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

img 태그를 사용하실 땐 웹 접근성과 seo 에 중요한 alt 속성을 넣어주시면 좋을 것 같습니다.
또한 이미지 사이즈도 지정해주세요.
지정하지 않게되면 이미지를 불러오기 전까진 크기를 알 수 없어 대충 영역을 잡았다가 불러온 이후 이미지 크기에 맞게 레이아웃이 변경되면서 깜빡거리는 현상이 발생할 수 있습니다.

<section id = "main_section">

<div id = "main_phrase">
<sr class = "gradation">세상의 모든 정보</sr>를 <br>
Copy link
Collaborator

Choose a reason for hiding this comment

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

sr 태그는 없는 것 같은데 의도하신게 아니라면 수정해주세요.

</header>

<section id = "main_section">

Copy link
Collaborator

Choose a reason for hiding this comment

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

위의 header 와 section 태그 사이 공백은 구조를 파악하는데 효과적이어서 좋은데요.
section 과 하위 div 태그 사이 공백은 하나의 큰 단위를 이루고 있는데 공백이 있어 가독성이 떨어지는 부분 같습니다.
이 내용 고려해서 수정해주세요~

</div>

<button id = "add_link">
링크추가하기
Copy link
Collaborator

Choose a reason for hiding this comment

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

텍스트 띄어쓰기가 잘못되었네요.

링크추가하기
</button>

<figure id = "main_pic">
Copy link
Collaborator

Choose a reason for hiding this comment

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

css 파일에서 이미지를 불러오신 것 같습니다.
단순한 배경 이미지로 불러올 때는 background-image 속성을 이용해 불러와도 괜찮은데요.
서비스의 컨텐츠로써 사용되는 이미지는 img 태그를 사용해 불러오는게 좋습니다.
웹 접근성 면에서 스크린 리더로 인식되고, seo 에 영향을 주는 부분이기 때문입니다.

font-family: pretendard;
}

body {margin: 0px;}
Copy link
Collaborator

Choose a reason for hiding this comment

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

중괄호 코드 형식을 신경써서 맞춰주세요~
불필요한 공백은 삭제하고 들여쓰기 부분 신경써서 수정해주세요.

body {
  margin: 0px;
}

#log_in {
    border: none;
    border-radius: 8px;
    width: 128px;
    height: 53px;
}

display: flex;
justify-content: space-between;
align-items: center;
background-color: rgba(240, 246, 255, 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

색상 설정값으로 여러가지 형식을 사용할 수 있는데요.
피그마를 보면 미리 정의된 색상 값들이 있습니다.
전역적으로 여러 페이지에서 동일한 값을 사용하므로 변수로 설정 후 import 해서 사용하는 것을 추천드립니다.

@o-seung-yeon o-seung-yeon merged commit 1338725 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