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 song hong bin #36

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

Kdt5 song hong bin #36

wants to merge 12 commits into from

Conversation

hbsongk
Copy link

@hbsongk hbsongk commented Apr 5, 2023

안녕하세요! 프론트엔드 과정 5기 송홍빈입니다.
1차 클론코딩 과제 제출합니다.

원본 사이트 경로: https://toss.im/
클론한 사이트 경로: https://hbsongk.netlify.app/

HTML, CSS를 주로 활용하였으며,
BEM과 Flex 레이아웃을 최대한 활용하여 구현하였습니다.

@hbsongk hbsongk self-assigned this Apr 5, 2023
@happyhermann
Copy link

전반적으로 첫 프로젝트임에도 잘 해주셨습니다!
BEM도 잘 지키시고 Flex 잘 활용해주셨어요

다만, h1태그는 한 페이지에 하나만 쓰일 수 있다는 것을 말씀드리고 싶습니다!
h1태그가 구글 SEO에 지대한 영향을 미치기때문에 앞으로 h1태그를 유의하고 사용하시면 좋을것 같습니다.
또한 ul,li는 최상단에 nav가 오는 것이 시멘틱합니다!
고생많으셨습니다

@hbsongk
Copy link
Author

hbsongk commented Apr 20, 2023

전반적으로 첫 프로젝트임에도 잘 해주셨습니다! BEM도 잘 지키시고 Flex 잘 활용해주셨어요

다만, h1태그는 한 페이지에 하나만 쓰일 수 있다는 것을 말씀드리고 싶습니다! h1태그가 구글 SEO에 지대한 영향을 미치기때문에 앞으로 h1태그를 유의하고 사용하시면 좋을것 같습니다. 또한 ul,li는 최상단에 nav가 오는 것이 시멘틱합니다! 고생많으셨습니다

구체적으로 comment 남겨주셔서 너무 감사드립니다 멘토님! 조언 잘 참고해서 다음 프로젝트에 적용하겠습니다. 감사합니다

@ChoEun-Sang
Copy link

애니메이션 요소들이 생각보다 많이 적은 거 같습니다~
JS로 구현해보시면 좋을 거 같습니다!

<p>내 모든 금융 내역을 한눈에 조회하고 한 곳에서 관리하세요.</p>
<p>이제껏 경험 못 했던 쉽고 편리한 금융 서비스,</p>
<p>토스와 함께라면 당신의 일상이 새로워질 거예요.</p>
</div>

Choose a reason for hiding this comment

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

홍빈님! 기존 <p> 태그 내에서 <br /> 사용하셨다가 이후 <p> 태그 연속 사용으로 수정하신 이유가 있나용?
전 수정 전 방법으로 진행했는데, 홍빈님 수정하신 방향이 더 효율적인가 해서 궁금합니다. 👀

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.

4 participants