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

Kdt5 lee chang hwi #39

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

leechanghwi
Copy link

@leechanghwi leechanghwi commented Apr 5, 2023

YouTube 웹사이트를 클론 코딩하였습니다.
클론코딩 홈페이지 : https://changhwiyoutube.netlify.app/
원본홈페이지 : https://www.youtube.com/

필수 요구사항

[ O ] 과제에 대한 설명을 포함한 README.md 파일을 제공하세요!
[ O ]과제 결과와 비교할 수 있는 실제 사이트(페이지)의 주소를 명시하세요!
[ O ]과정에서 사용한 프로젝트 폴더/파일이 모두 포함돼야 합니다, 일부 파일만 제출하지 마세요!
[ O ]실제 서비스로 배포하고 접근 가능한 링크를 추가해야 합니다.

선택 요구사항

[ O ]header, section 등 시멘틱 태그를 최대한 활용해보세요.
[ O ]실제 사이트의 레거시 코드 활용보단 최신의 CSS Flex 혹은 Grid 등을 활용해보세요.
[ O ]부분적으로 BEM 방법론을 도입해보세요.
[ O ]JS가 필요한 부분은 되도록 생략하되 이유를 명시해보세요.(CSS로 대체 가능한지 피드백이 있을 수 있겠죠?!)
[ X ]JS가 필요한 부분 중 구현할 부분이 있다면 자유롭게 구현해보세요.(JS 과제가 아니니까 가볍게 구현하시길 추천해요)
[ X ]SCSS 등의 CSS 전처리도구를 도입해보세요.
[ X ]SCSS 컴파일에 Webpack이나 Parcel 같은 번들러를 활용해보세요.


<hr class="seperate" />
</div>

Choose a reason for hiding this comment

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

헤더에 3줄 버튼(햄버거)을 클릭했을 때 미니 사이드 버튼들이 나타났으면 재밌을 것 같아요. mini-side-button요소를 만들고 visibility: hidden을 사용해서 숨긴 다음 js로 visibility 속성만 껐다 킬 수 있게 기능을 추가하면 될 것 같습니다.

Copy link
Member

@iamidlek iamidlek left a comment

Choose a reason for hiding this comment

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

고생하셨습니다.
grid 사용 등 요구 사항에 맞추기 위해 노력하신 것 같아 좋은 것 같습니다.
다만 시멘틱 태그 활용에 체크 해주셨는데 부족한 것 같습니다.
또한 js가 필요한 부분을 생략한 이유에 대한 명시 또한 체크해주셨지만
명시된 내용이 없는 것 같습니다.

중복되는 부분은 코멘트 하지 않았습니다.

HTML

gird 사용으로 페이지의 layout 구조를 생각해 보신 만큼
시메틱한 태그 사용도 조금 더 고려해주시면 문제 없을 것 같습니다.

CSS

변수 사용과 grid 사용 좋은 것 같습니다.
선택자가 3개 이상 넘어가면서 불편함을 느끼셨으리라 생각이 됩니다.
scss를 활용해 보시면 좋을 것 같습니다.
BEM 방법론을 조금 더 정확하게 사용해 보시면 좋을 것 같습니다.

<div class="container">
<!-- Header -->

<div class="header">
Copy link
Member

Choose a reason for hiding this comment

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

header 태그를 사용하시면 좋을 것 같아요.

index.html Outdated

<!-- side button -->

<div class="sidebutton">
Copy link
Member

Choose a reason for hiding this comment

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

네이게이션 부분이라면 시멘틱 하게 작성해 보시면 좋을 것 같아요.

index.html Outdated
<section class="video__section">
<article class="content__main">
<div class="content">
<img src="images/first.jpg" alt="" />
Copy link
Member

Choose a reason for hiding this comment

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

alt 속성은 값을 주시는 것이 좋을 것 같아요.

border-top: 1px solid var(--grey-light-color);
border-left: 1px solid var(--grey-light-color);
border-bottom: 1px solid var(--grey-light-color);
border-right: none;
Copy link
Member

Choose a reason for hiding this comment

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

단축 속성을 사용하여 더욱 간결하게 작성하는 방법을 찾아 보시는 것도 좋을 것 같아요.

main.css Outdated
}
}

@media (max-width: 768px) {
Copy link
Member

Choose a reason for hiding this comment

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

같은 미디어 쿼리라면 안에 한번에 작성해 보시면 좋을 것 같습니다.

README.md Outdated
- [ ] JS가 필요한 부분 중 구현할 부분이 있다면 자유롭게 구현해보세요.(JS 과제가 아니니까 가볍게 구현하시길 추천해요)
- [ ] SCSS 등의 CSS 전처리도구를 도입해보세요.
- [ ] SCSS 컴파일에 Webpack이나 Parcel 같은 번들러를 활용해보세요.
- [o ] `<header>`, `<section>` 등 시멘틱 태그를 최대한 활용해보세요.
Copy link
Member

Choose a reason for hiding this comment

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

마크다운 문법을 찾아보시면 좋을 것 같아요.
[ x ]로 하시면 체크 표시가 됩니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants