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 Lim Seungyi 과제제출 #69

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

Conversation

doitidey
Copy link

@doitidey doitidey commented Apr 6, 2023

사이트 레이아웃 클론 과제

실제 사이트

🔗 https://twitter.com

과제 결과

🔗 https://remarkable-palmier-bf8451.netlify.app/

리뷰

사이트 선정 이유

  1. SNS 사이트에 관심이 있었고, 이 기회에 레이아웃을 분석해보고 싶다는 마음
  2. 독특한 구조의 트위터 레이아웃이 눈길을 끌었다.

제작기간

3월 28일 ~ 4월 6일

언어

  • HTML
  • CSS

설명

  • 배운 시멘틱 태그들을 활용해봤습니다. header, section, footer 등....
  • Flex를 최대한 활용해보려고 했습니다.
    • 하지만 활용 지식이 부족해서 많은 부분에 어려움이 있었습니다! 더 공부해보겠습니다.
  • BEM 방법론을 도입해보려고 했으나 class name을 좋지 않은 방식으로 사용 한 것 같습니다.
    • 앞으로는 HTML 구조를 처음부터 잘 짜고 나눠놓고 class name을 부여하려고 합니다.
  • footer 에만 링크를 걸어놓았습니다.
  • 반응형, JS를 적용하지 않았습니다.

Copy link

@chanuuuuu chanuuuuu left a comment

Choose a reason for hiding this comment

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

BEM적용이나 시맨틱 태그의 적용까지 엄청 잘하셔서 코멘트할 부분을 찾느라 힘들었습니다..👍🏻

이러한 의견도 있겠구나 하는 생각으로 필요한 부분에 대해서만 리팩토링 진행하시면 됩니다.

고생하셨습니다.

<!-- 구글 머터리얼 아이콘 -->
<link rel="stylesheet" href="https://fonts.googleapis.com/icon?family=Material+Icons" />
<!-- 구글 폰트 -->
<link rel="preconnect" href="https://fonts.googleapis.com">

Choose a reason for hiding this comment

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

외부링크에 대해서 rel="preconnect" 속성을 사용하였네요. 해당 속성은 어떠한 용도로 사용하셨는지 궁금합니다.🤔

Copy link
Author

Choose a reason for hiding this comment

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

구글 폰트에서 제공하는 link 태그를 그대로 복사해 사용했습니다!...
다만 멘토님께서 질문해주신 덕에 rel="preconnect"에 대해 조금 알아보니 외부 링크를 미리 연결해두어 리소스 로드 시간을 줄일 수 있다는 장점이 있지만 cpu 사용량을 늘릴 수도 있는 속성인 것 같습니다. 앞으로는 잘 알아본 뒤에, 의도를 가지고 사용해보도록 하겠습니다!💪🏻

Choose a reason for hiding this comment

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

네네 나중에 단순히 HTML을 구현하는 것에서 더 나아가 성능까지 고려해야하는 경우, 문서내 리소스의 다운로드를 위해 preconnect를 사용하는 것도 중요합니다. 아직까지는 고려하지 않아도 되지만 한번쯤 알고가는 것도 도움이 될 것 같습니다:)

index.html Show resolved Hide resolved
<!-- 섹션 -->
<div class="inner">

<div class="main-section">

Choose a reason for hiding this comment

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

section이라는 단어를 제거해보는 것은 어떨까요? 물론 해당 태그가 어떠한 영역을 담당하고 있다는 의미를 가질 수 있으나, BEM에서 이미 클래스명이 block(영역)의 의미를 담고 있기 때문에 줄일 수 있다고 생각합니다. (물론 main이라는 클래스만을 사용하는 것은 너무 추상적이라고 생각합니다. herofilter와 같이 의미하는 바를 더 구체적으로 명시할 수 있는 단어를 선택하는 것이 좋다고 생각합니다.)

승이님의 생각은 어떠신가요?🤔

</div>
<span class="material-icons">more_horiz</span>
</div>
<div class="tweet__inside">

Choose a reason for hiding this comment

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

위의 tweet_inside가 존재하는데 두 클래스명의 차이는 어떤 것일까요?

Copy link
Author

Choose a reason for hiding this comment

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

tweet_inside는 각 tweet 안에 담긴 프로필사진을 제외한, 오른쪽 정렬된 내용들을 묶어주고
tweet__insidetweet_inside안의 이름, 아이디를 제외한 내부요소들을 묶어주기 위해 사용했습니다...만!
좋지 않은 이름이라고 생각하여 변경을 고민해보도록 하겠습니다

index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
</div>
</div>
</div>
<div class="tweets">

Choose a reason for hiding this comment

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

각 트윗을 하나의 <section> 또는 <article>을 사용할 수 있을 것 같아요. 두 시맨틱 태그는 어떠한 의미를 가지는지 한번 확인해보시고 변경해보는 것은 어떨까요?

index.html Show resolved Hide resolved
css/main.css Show resolved Hide resolved
</div>
<ul>
<li>
<a href="javascript:void(0)">

Choose a reason for hiding this comment

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

a 태그에서 javascript를 실행하기 위해 href를 사용할 수도 있지만 onclick 이벤트 핸들러를 추가하여 구현할 수도 있습니다. 이러한 두가지 방식 중에 href를 사용한 이유는 무엇일까요? 또는 두 방식의 차이는 어떤 것이 있을까요?

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.

2 participants