Skip to content

[리오] Amazon 1 : HTML 구조화 설계 - STEP1 #1

Merged
crongro merged 2 commits intocode-squad:feanar729from
feanar729:STEP1
Sep 18, 2018
Merged

[리오] Amazon 1 : HTML 구조화 설계 - STEP1 #1
crongro merged 2 commits intocode-squad:feanar729from
feanar729:STEP1

Conversation

@feanar729
Copy link
Copy Markdown

@feanar729 feanar729 commented Sep 14, 2018

아마존 pr 날립니다 많은 지적 부탁드립니다.
아 그리고 궁금한점이 있습니다
HTML은 git commit시 commit단위를 어떤 단위로 생각해서 commit 기록을 남기는게 좋을까요?
js가 기능 단위로 남기듯이 말입니다 답변 바랍니다

@crongro
Copy link
Copy Markdown
Contributor

crongro commented Sep 14, 2018

커밋단위는 작아도 좋아요.
html도 의미단위를 정해서 커밋하는게 좋겠죠.
전체 레이아웃을 코딩했다던가, header 영역을 완료했다, 특정content영역을 마쳤다 등으로요.

Copy link
Copy Markdown
Contributor

@crongro crongro left a comment

Choose a reason for hiding this comment

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

아주 잘 했어요.
(고생하셨군요)

몇가지 안내한 태그들도 쓰면 좋습니다.
특히 ul/li를 더 넓은 범위에서 사용하곤합니다.

클래스이름은 약간긴편이지만 문제될 정도가 아닙니다. 오히려 규칙을가지고 표현한게 좋네요.
다만 class이름이 다 필요할까? 라는 점에서는 모든게 다 필요하지 않다라고 할 수 있어요 ^^ css작업을 하면서 불필요해보이는 class이름(안쓰더라)은 제거해보세요.

그리고 그룹핑을 해줄필요가 있습니다. css작업전이지만 몇몇 그룹을 묶여야 하는 태그들은 같이 div로 묶어보세요.

main.html Outdated

<body>
<header id="prime-nav-menu">
<div id="nav-head">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nav 태그가 있어요.

main.html Outdated
<div class="nav-search-select_dropdown_menu">
<form action="" method="get">
<select class="nav-search-select_dropdown_menu">
<option value="">All Departments</option>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

실무에서는 이런데이터는 javascript나 backend의 템플릿작업이라는 과정을 통해서 생성됩니다.
html코드에 이렇게 하드코딩하지 않긴해요. (힘드셨을듯;;)

main.html Outdated
<span>All</span>
<i class="nav-icon"></i>
</div>
<div class="nav-search-select_dropdown_menu">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

컨벤션을 정한 건 좋고요.
클래스이름은 좀 긴편입니다.

main.html Outdated
<span>Departments</span>
</div>
<div class="nav-my_amazon_link">
<a href="http://">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

보통 링크url이 없으면 #을 사용해요.

main.html Outdated
</div>

<div class="card-category">
<div class="card-list1">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

card-list 라고 표현햇자나요? 네 이건 list에요.
그러니... ul/li 를 쓸 수 있겠죠?

main.html Outdated
<span class="product-type">Stream</span>
</div>
<div class="card-content">
<div class="content-click-button">1</div>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

보통 click과 같은 인터랙션은 제외하고 이름을 짓습니다.

main.html Outdated
<div class="content-click-button">4</div>
</div>
</div>
<div class="card-list5">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1,2,3,4,5 이른 이름을 클래스명에 넣으면 중간에 뭐가 들어가거나 삭제되면 sync를 맞추어야해서 어려워요.

main.html Outdated
</div>

<div class="a-carosel-viewpoint">
<ol class="a-carosel-lists">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ordered list 가 맞을까요? 순서가 필요없는 거 같은데요.

main.html Outdated
<div class="card-header">
<span class="carosel-card-category-text">ALEXA VOICE SHOPPING</span>
</div>
<div class="card-headline">The simplest way to shop. Just ask Alexa.</div>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

음 dl/dt/dd 라는게 있어요.
제목과 설명이 있는 구조인거 같으니, 그걸 쓰는걸 생각해보세요.

main.html Outdated
<span class="prime-submain-button">Prime annual</span>
</a>
</div>
<span class="subtext">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

여기 두 개의 subtext클래스를 가진 span 두개는 하나의 block으로 지정해서 이를 묶어두는게 위치를 조정하기도 좋아요.
유사한 녀석들을 자꾸 그룹핑하게 될텐데요. HTML구조를 구현하면서 그런부분은 같이 묶어주세요

@feanar729
Copy link
Copy Markdown
Author

HTML commit 단위가 인식하고 commit 나눠 넣기가 어렵네요;;
refactoring 진행했습니다 리뷰 부탁드립니다.

Copy link
Copy Markdown
Contributor

@crongro crongro left a comment

Choose a reason for hiding this comment

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

잘 수정하셨네요.

앞으로 이 규칙으로 클래스이름도 지어가며 진행해보고요.

데이터를 너무 추가하려고 노력하지 않았으면 좋겠어요. 너무 수고가 많으신듯해서요.

html tag를 앞으로 익히면서 좀더 어울리는것이 있다면 바꿔보죠.

잘 했습니다.

@crongro crongro merged commit a9b6c48 into code-squad:feanar729 Sep 18, 2018
@feanar729 feanar729 changed the title [리오] HTML 구조화 설계 - STEP1 [리오] HTML 구조화 설계 - STEP1 Oct 1, 2018
@feanar729 feanar729 changed the title [리오] HTML 구조화 설계 - STEP1 [리오] Amazon 1 : HTML 구조화 설계 - STEP1 Oct 1, 2018
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