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

KDTO_LeeYeonSU 일룸(iloom) 클론코딩 #67

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

Conversation

suehub
Copy link
Member

@suehub suehub commented Jul 28, 2023

과제 수행 기간

2023.07.24~2023.07.28

구현 내용

  • 시멘틱 태그 사용
  • BEM 방법론 도입
  • 메인 페이지 구현

아쉬운 점

최대한 HTML/CSS 만 사용하는 것이 목표였는데 swiper를 사용한 것이 조금 아쉬웠다.
헤더 hover 시 나타나는 메뉴 스타일링을 완성하지 못한 점이 아쉽다.
메인 페이지를 완성시키고 반응형도 구현하고 싶었는데 스터디까지 병행하니 시간이 부족했던 것 같다.

@netlify
Copy link

netlify bot commented Jul 28, 2023

Deploy Preview for mellifluous-gnome-003496 ready!

Name Link
🔨 Latest commit 20f0892
🔍 Latest deploy log https://app.netlify.com/sites/mellifluous-gnome-003496/deploys/64c3d6e43a9baf00075d8cc9
😎 Deploy Preview https://deploy-preview-67--mellifluous-gnome-003496.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Jul 28, 2023

Deploy Preview for sue-iloom ready!

Name Link
🔨 Latest commit 0aebba4
🔍 Latest deploy log https://app.netlify.com/sites/sue-iloom/deploys/64d0f3262b5dc90008b70812
😎 Deploy Preview https://deploy-preview-67--sue-iloom.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@kyungkyuBae kyungkyuBae left a comment

Choose a reason for hiding this comment

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

선택자들을 다양하게 잘 쓰시는거같아요👍

@LeHiHo
Copy link

LeHiHo commented Aug 2, 2023

css 파일을 레이아웃 별로 나누어서 관리해서 이해하기 편했습니다. Reset css코드를 사용한것도 좋았습니다!!

@humnyenye
Copy link

이미지를 아이콘과 이미지로 나누거나 css를 레이아웃 단위로 나누는 등 구조를 파악하기 좋게 분할해 놓아 가독성을 올리신 점이 좋아요

@noSPkeepgoing
Copy link

noSPkeepgoing commented Aug 3, 2023

footer__divide 속 hr태그가 제자리를 찾지 못하는 거 같아요! 기능적으로 의미 없고 스타일만을 위한 태그라면 footer__top에 border-top, 혹은 footer_bottom에 border-bottom을 주는 방식은 어떨까요? 혹은 가상선택자를 사용해도 괜찮을 거 같아요😘

@suehub
Copy link
Member Author

suehub commented Aug 3, 2023

footer__divide 속 hr태그가 제자리를 찾지 못하는 거 같아요! 기능적으로 의미 없고 스타일만을 위한 태그라면 footer__top에 border-top, 혹은 footer_bottom에 border-bottom을 주는 방식은 어떨까요? 혹은 가상선택자를 사용해도 괜찮을 거 같아요😘

@noSPkeepgoing 좋은 의견 감사합니다~ 반영해볼게요!!

Copy link
Member

@marshallku marshallku left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

js/main.js Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
css/footer.css Outdated Show resolved Hide resolved
css/footer.css Outdated Show resolved Hide resolved
css/header.css Outdated Show resolved Hide resolved
css/main.css Outdated Show resolved Hide resolved
css/header.css Outdated
Comment on lines 107 to 134
.gnb__inner__menu {
width: 60px;
padding: 19.5px 20px;
position: absolute;
top: 50px;
margin-left: -38px;
z-index: 11;
border: 1px solid #555;
background-color: white;
display: none;
}
.gnb__inner__menu li {
float: none;
text-align: center;
font-size: 13px;
font-weight: 300;
padding: 5.5px 0px;
line-height: normal;
}
.gnb__inner__menu li a {
text-align: center;
font-size: 14px;
font-weight: 300;
line-height: normal;
display: inline-block;
width: 100%;
height: auto;
}
Copy link
Member

Choose a reason for hiding this comment

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

sass같은 전처리기를 사용하시면

.gnb__inner__menu {
  // ...
  li {
    // ...
    a {
      // ...
    }
  }
}

처럼 반복을 줄이실 수 있으니, 한 번 알아보셔도 좋을 것 같습니다!

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