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

KDT0_JangMunYong 투썸플레이스 홈페이지 클론 코딩 #53

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

Conversation

moonyah
Copy link

@moonyah moonyah commented Jul 28, 2023

투썸플레이스 클론코딩🍮✨

프로젝트 기간

2023년 07월 24일 ~ 2023년 07월 28일

요구 사항 체크

✔️ 과제에 대한 설명을 포함한 README.md 파일

✔️ 과제 결과와 비교할 수 있는 실제 사이트(페이지)의 주소를 명시

✔️ 실제 서비스로 배포하고 접근 가능한 링크를 추가

✔️ 시멘틱 태그를 최대한 활용

✔️ 실제 사이트의 레거시 코드 활용보단 최신의 CSS Flex 혹은 Grid 등을 활용

✔️ 부분적으로 BEM 방법론을 도입

✔️ JS가 필요한 부분 중 구현할 부분이 있다면 자유롭게 구현

주요 내용 설명

1. 전체 페이지

all

  • 총 6개의 페이지
  • 스크롤 기능 추가해 화면 전환

2. header

header

  • header를 hover하면 로고와 메뉴와 언어 설정 아이콘, 전체 메뉴 아이콘 색이 어둡게 변경됨
  • header 상단 고정

3. swiper와 toggleButton

menu_story

4. footer

footer

기술 스택

아쉬운 점

  • 스크롤 다루기

  • 13인치 노트북에서 스크롤을 할 때 페이지가 두 칸씩 넘어가는 문제를 발견
  • footer 부분이 다른 페이지와 크기가 달라서 스크롤을 다 내렸을 경우, 흰색 공간이 생김 (footer 위치를 내려서 임시로 해결)
  • flex는 많이 사용했는데 grid 활용을 안한 점
  • js에 아직 미숙해서 구현하고 싶은 기능을 못 넣은 게 아쉽다.
  • css코드가 정리가 안 된 느낌이라서 아쉽다. 공통적인 부분은 합치고 겹치는 부분은 제거해야 해야 할 필요가 있다.

느낀 점

  • 전체적으로 웹페이지의 구성과 구조를 꼼꼼히 분석해 보는 기회가 되었다.
  • 전체적으로 간단해 보이는 것도 기능 구현이 생각보다 쉽지 않다.
  • CSS부분에서 하나의 기능도 여러 가지 방식을 시도해 보고 더 편리하고 효율적인 것을 비교해 봐야겠다.

@seungjun222
Copy link

잘 봤습니다. 추후에 JS 익숙해지면 화면 우측 가운데의 pagination이나, 페이지에 따라 달라지는 네비게이션 바 색상을 구현하실 수 있을거라 생각합니다!

@lilviolie
Copy link

사이트 잘봤습니다 고생하셨어요!! 휠 이벤트로 스크롤 구현하신 부분도 잘보고갑니다👏🏻 그런데 상당히 큰 화면 기준으로 작업하신 것 같은데.. 13인치, 15인치로 모두 들어가봤는데도 헤더가 안맞아보여서ㅜㅜ 몇인치 기준으로 작업하신건지 알 수 있을까요??

@moonyah
Copy link
Author

moonyah commented Jul 31, 2023

@lilviolie 15.6인치입니다... 제가 화면 구성할 때 화면 기준을 생각하지 못해서 큰 문제가 있네요...ㅠ 개선해보겠습니다..... 감사합니다!

Copy link

@JitHoon JitHoon left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~!

css/main.css Outdated
/* HEADER */
header {
/* background: transparent;
height: 100px;
Copy link

Choose a reason for hiding this comment

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

header 태그안 a, ul, button 태그들이 화면상에서 겹쳐보이는 레이아웃 문제가 있네요!

className이 .main-menu인 ul 태그의 자식들을 겹치지 않고 가로로 잘 나열하기위해 사용하신 display: flex 속성을 header 태그에도 추가하면 레이아웃 문제가 해결될 것 같습니다!

Copy link

@minsoo-web minsoo-web left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다!
전체적으로 왜 사용되는지 모를 코드들과, 불필요하게 중복되는 코드들, 태그 선택자들을 개선해보면 좋을 것 같습니다.

README.md Outdated
✔️ 과제 결과와 비교할 수 있는 실제 사이트(페이지)의 주소를 명시<br>
✔️ 실제 서비스로 배포하고 접근 가능한 링크를 추가<br>
✔️ 시멘틱 태그를 최대한 활용<br>
✔️ 실제 사이트의 레거시 코드 활용보단 최신의 CSS Flex 혹은 Grid 등을 활용<br />

Choose a reason for hiding this comment

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

<br/> 태그를 쓰지 않아도 마지막 줄에 띄어쓰기를 두 번 해주면 줄 바꿈 처리가 됩니다!

Copy link
Author

Choose a reason for hiding this comment

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

넵 감사합니다. 수정하였습니다! f62e52f

README.md Outdated
Comment on lines 29 to 30
#### **3. swiper와 toggleButton**
![menu_story](https://github.com/moonyah/moonyah.github.io/assets/51106050/86369778-b063-455d-8d7f-90c658f357d9)

Choose a reason for hiding this comment

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

prettier를 활용해서 md 문서를 포메팅해보세요!

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다. md 문서 포메팅하였습니다! b089b36

css/main.css Outdated
@@ -0,0 +1,978 @@

/* COMMON */

Choose a reason for hiding this comment

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

주석으로 역할을 구분 짓는 것도 방법이지만, 파일로 분리하는 것이 좀 더 좋은 방법입니다!

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다. CSS 파일 분리 하였습니다! 29cc50c

css/main.css Outdated
display: inline-block;
width: 20px;
height: 20px;
margin: 0 0 0 16px;

Choose a reason for hiding this comment

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

이렇게 적을 때는, margin-left 가 훨씬 더 가독성있습니다~!

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다! 8fa5415

css/main.css Outdated
/* main-quick-container */
.main-quick-container {
position: fixed;
display: -moz-flex;

Choose a reason for hiding this comment

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

moz가 어떤 역할인지 아시나요? 왜 필요한지도 같이 공부해보시면 좋을 것 같습니다

Copy link
Author

Choose a reason for hiding this comment

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

display: -moz-flex; 는 오래된 버전의 모질라 파이어폭스 브라우저에서 사용되던 CSS 속성으로 현재 사용이 권장되지 않고 display: flex; 속성이 현재의 대부분의 브라우저에서 지원되고 있고 크로스 브라우저 호환성과 현대 웹 표준을 준수를 위해 display: flex;를 사용하는 것이 바람직 하다는 것을 알게 되었습니다. 수정하겠습니다!

Choose a reason for hiding this comment

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

역할에 맞게 폴더 구성하신 것 좋습니다!!

Comment on lines +315 to +321
<style>
.icon-foot-sns-twitter {
width: 32px;
height: 32px;
background: url(../images/icon/ico_foot_sns_twitter.svg) 50% 50% no-repeat;
}
</style>

Choose a reason for hiding this comment

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

얘만 여기에 따로 적으신 이유가 있나요?

Choose a reason for hiding this comment

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

이 함수 이름만 봤을 때 무슨 역할을 하는 애인지 전혀 알 수 없습니다. 어떤 역할을 하는지, 함수 이름에서도, 파일 이름에서도 감이 올 수 있도록 하는 습관을 들여보시는 게 좋을 것 같습니다.

e.preventDefault();
}, { passive: false });

var $html = $("html");

Choose a reason for hiding this comment

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

jquery는 한 번 익숙해지면 헤어나오기가 쉽지않습니다.
지금은 js 기본기에 익숙해지시는 게 훨씬 좋은 공부 습관이라고 생각됩니다.

setPage(page);
});

// // 기본 이벤트 제거

Choose a reason for hiding this comment

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

안 쓰이는 코드는 주석이 아니라 삭제하는 걸 습관화 하세요!

@LikeFireAndSky
Copy link

처음에 클론 코딩한 홈페이지랑 참고한 홈페이지랑 구분하는데 오래 걸렸네요...😲 정말 잘 만드신 것 같아요. 특히 scroll 구현하는 부분에서 정말 기능을 잘 구현하신 것 같아서 많이 배우고 가는 것 같습니다. HTML에서도 중간 중간 comment를 추가해서 어떠한 기능을 가지고 있는 지 파악하기 정말 쉬운 것 같습니다!! 저도 리펙토링 시간에 이러한 디테일을 추가해보고 싶네요!!

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.

6 participants