-
Notifications
You must be signed in to change notification settings - Fork 168
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
[2단계 - 행운의 로또 미션] 크리스(이송원) 미션 제출합니다. #54
Changes from 24 commits
1b29e74
bcfb7df
0441001
138152f
8f7a174
b3015b1
0ff747f
d6981cf
d5f43af
d04dc82
965b02f
9e3d93d
9022e46
506af7e
045ead8
8a63b01
69fecee
f654044
60c7dc4
48f83bd
c288e07
51c8a8f
56145e8
0439909
48f39d0
2029389
972cfd1
591d57b
c8e8697
fc61f4f
c8861f8
dfa427d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,7 @@ <h1 class="text-center">🎱 행운의 로또</h1> | |
</button> | ||
</div> | ||
</form> | ||
<section id="purchase-result" class="mt-9 hide"> | ||
<section id="purchase-result" class="mt-9 removed"> | ||
<div class="d-flex"> | ||
<label id="purchase-item-count" class="flex-auto my-0"></label> | ||
<div class="flex-auto d-flex justify-end pr-1"> | ||
|
@@ -41,62 +41,76 @@ <h1 class="text-center">🎱 행운의 로또</h1> | |
</label> | ||
</div> | ||
</div> | ||
<div id="purchase-item-list" class="d-flex flex-wrap hide hide-lotto-numbers"></div> | ||
<div | ||
id="purchase-item-list" | ||
class="d-flex flex-wrap removed lotto-numbers-removed" | ||
></div> | ||
</section> | ||
<form class="mt-9" style="display: none"> | ||
<form id="winning-number-input-form" class="mt-9 removed"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. id 네이밍이 굉장히 기네요! 하위에도 element 의 id가 길어지는 걸 볼 수 있구요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BEM 을 적용하면 오히려 id 나 클래스가 길어지는 것으로 알고 있었는데 확실히 더 깔끔해진 것 같습니다! 그런데 조금이라도 더 짧게 하기 위해 check-number-form__input 이 아니라 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 넵 좋은 것 같습니다! |
||
<label class="flex-auto d-inline-block mb-3" | ||
>지난 주 당첨번호 6개와 보너스 넘버 1개를 입력해주세요.</label | ||
> | ||
<div class="d-flex"> | ||
<div id="correct-number-wrapper" class="d-flex"> | ||
<div> | ||
<h4 class="mt-0 mb-3 text-center">당첨 번호</h4> | ||
<div> | ||
<input | ||
type="number" | ||
data-custom-input | ||
class="winning-number mx-1 text-center" | ||
/> | ||
<input | ||
type="number" | ||
data-custom-input | ||
class="winning-number mx-1 text-center" | ||
/> | ||
<input | ||
type="number" | ||
data-custom-input | ||
class="winning-number mx-1 text-center" | ||
/> | ||
<input | ||
type="number" | ||
data-custom-input | ||
class="winning-number mx-1 text-center" | ||
/> | ||
<input | ||
type="number" | ||
data-custom-input | ||
class="winning-number mx-1 text-center" | ||
/> | ||
<input | ||
type="number" | ||
data-custom-input | ||
class="winning-number mx-1 text-center" | ||
/> | ||
</div> | ||
</div> | ||
<div class="bonus-number-container flex-grow"> | ||
<h4 class="mt-0 mb-3 text-center">보너스 번호</h4> | ||
<div class="d-flex justify-center"> | ||
<input type="number" class="bonus-number text-center" /> | ||
<input | ||
type="number" | ||
data-custom-input | ||
class="bonus-number text-center" | ||
/> | ||
</div> | ||
</div> | ||
</div> | ||
<button | ||
type="button" | ||
class="open-result-modal-button mt-5 btn btn-cyan w-100" | ||
id="result-modal-open-button" | ||
class="mt-5 btn btn-cyan w-100" | ||
> | ||
결과 확인하기 | ||
</button> | ||
</form> | ||
</div> | ||
</div> | ||
|
||
<div class="modal"> | ||
<div id="modal"> | ||
<div class="modal-inner p-10"> | ||
<div class="modal-close"> | ||
<div id="modal-close"> | ||
<svg viewbox="0 0 40 40"> | ||
<path class="close-x" d="M 10,10 L 30,30 M 30,10 L 10,30" /> | ||
</svg> | ||
|
@@ -112,38 +126,16 @@ <h2 class="text-center">🏆 당첨 통계 🏆</h2> | |
<th class="p-3">당첨 갯수</th> | ||
</tr> | ||
</thead> | ||
<tbody> | ||
<tr class="text-center"> | ||
<td class="p-3">3개</td> | ||
<td class="p-3">5,000</td> | ||
<td class="p-3">n개</td> | ||
</tr> | ||
<tr class="text-center"> | ||
<td class="p-3">4개</td> | ||
<td class="p-3">50,000</td> | ||
<td class="p-3">n개</td> | ||
</tr> | ||
<tr class="text-center"> | ||
<td class="p-3">5개</td> | ||
<td class="p-3">1,500,000</td> | ||
<td class="p-3">n개</td> | ||
</tr> | ||
<tr class="text-center"> | ||
<td class="p-3">5개 + 보너스볼</td> | ||
<td class="p-3">30,000,000</td> | ||
<td class="p-3">n개</td> | ||
</tr> | ||
<tr class="text-center"> | ||
<td class="p-3">6개</td> | ||
<td class="p-3">2,000,000,000</td> | ||
<td class="p-3">n개</td> | ||
</tr> | ||
</tbody> | ||
<tbody id="result-tbody"></tbody> | ||
</table> | ||
</div> | ||
<p class="text-center font-bold">당신의 총 수익률은 %입니다.</p> | ||
<p class="text-center font-bold"> | ||
당신의 총 수익률은 <span id="profit-rate"></span>%입니다. | ||
</p> | ||
<div class="d-flex justify-center mt-5"> | ||
<button type="button" class="btn btn-cyan">다시 시작하기</button> | ||
<button id="restart-button" type="button" class="btn btn-cyan"> | ||
다시 시작하기 | ||
</button> | ||
</div> | ||
</div> | ||
</div> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,10 +31,14 @@ body { | |
padding-bottom: 4px; | ||
} | ||
|
||
#purchase-item-list.hide-lotto-numbers .lotto-numbers { | ||
#purchase-item-list.lotto-numbers-removed .lotto-numbers { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. display를 none하거나 display를 block으로 보이도록 설정하는 것을 따로 visible, invisible등의 css 로직으로 때내서 사용하는게 좀 더 좋지 않을까요? 그 때 그 때 마다 -removed 와 같은 긴 이름을 붙이는 것 보다 나아보여서요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 별도로, 위의 winning-number나 bonus-number의 width, height가 같아보이는데요! 이 친구들을 묶을 수 있는 하나의 클래스 단위를 만들어서 함께 쓰면 좋을 것 같습니다 :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아래의 display.css에서 쓰고 있네요! 그걸 써보는 건 어떨까요? 요 로직은 필요없어보입니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. winning-number나 bonus-number 에 적용할 스타일링은 input-box 라는 이름의 클래스에 넣어서 관리하도록 만들었습니다! 개인적인 생각으로는 저 lotto-numbers-removed 라는 클래스는 로또 아이템들이 안보이게 만드는게 아니라 로또 아이템 안의 로또 번호들만 안보이게 하는 동작에 사용되기 때문에, 정확히 어떤 동작이 일어나는지를 명시하기 위해서 어느 정도 길더라도 저렇게 명시하는 것이 필요하다고 생각했습니다. 다만 너무 길다는 생각이 들어서 lotto-numbers-removed => numbers-removed 로 변경하였습니다. 지적해주신 것처럼 #purchase-item-list 이 요소에 저런 조건식을 붙이지 않고 .lotto-numbers 요소들에 removed, added 라는 클래스를 넣고 붙이는 식으로 동작하는 방법이 더 간단하다는 말씀의 의도는 알겠습니다. 다만 그렇게 하면 모든 .lotto-numbers 요소들의 클래스를 일괄적으로 조작해주는 동작을 해야하는데, 그러면 .lotto-numbers 요소의 개수만큼 리플로우가 일어나지 않을까 하는 우려가 있습니다. 기존에 #purchase-item-list 요소에 클래스 하나를 넣고 붙이는 동작은 한번의 리플로우만 발생시키는 방식이라고 생각해서 렌더링 성능을 생각해보면 이쪽이 더 유효한 방법이 아닐까 하는 의견을 조심스럽게 내봅니다. 혹시 그 정도의 성능 차이는 크게 의미 없다고 생각하시는지, 제가 놓치고 있거나 제대로 이해하지 못한 부분이 있다면 지적해주시면 감사드리겠습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 제가 생각하기에는, 성능이 무조껀 좋은 코드가 능사는 아닌 것 같아요. 해당 프로젝트에 맞게 함수를 만들고 설정하시면 될 것 같은데요~ 실시간 형태로 repaint, reflow가 몇십번 일어나지 않는 걸로 보이는데, 적용할 이유가 없어보입니다! 그리고 display가 reflow 의 문제가 있어보인다면, visibility나 opacity등으로 reflow의 해결도 할 수 있을 것 같습니다. |
||
display: none; | ||
} | ||
|
||
#purchase-item-list.lotto-numbers-added .lotto-numbers { | ||
display: ''; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. display: ''는 무얼 나타내는걸까요? display: block; 이 좀 더 명확하지 않을까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 빈 문자열로 만들면 가장 초기의 display 값으로 돌아간다는 말을 듣고 적용했던 것인데, 확실히 'block'이나 'initial'이라는 값으로 나타내는게 더 명시적이겠군요! 수정했습니다! |
||
} | ||
|
||
.lotto-item { | ||
display: flex; | ||
align-items: center; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
.hide { | ||
.block-added { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 분리한 것은 아주 좋습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 감사합니다! 그런데 생각해보니 block-added 는 뭔가 이해가 잘 안될 수도 있는 이름 같기도 해서 block-added 를 d-block 으로 변경했습니다! |
||
display: none; | ||
} | ||
|
||
.removed { | ||
display: none; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
.visible { | ||
visibility: visible; | ||
} | ||
|
||
.invisible { | ||
visibility: hidden; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
.modal { | ||
#modal { | ||
opacity: 0; | ||
visibility: hidden; | ||
display: flex; | ||
|
@@ -12,7 +12,7 @@ | |
z-index: 2; | ||
} | ||
|
||
.modal.open { | ||
#modal.open { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 좋네요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 감사합니다! |
||
opacity: 1; | ||
visibility: visible; | ||
} | ||
|
@@ -27,7 +27,7 @@ | |
position: relative; | ||
} | ||
|
||
.modal-close { | ||
#modal-close { | ||
margin: 20px; | ||
width: 20px; | ||
position: absolute; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
일렬적인 패턴이 반복되고 있네요!
이러한 반복 행위는 함수로 빼도 괜찮지 않을까요~?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그렇네요...! 반영했습니다! 앞으로도 3개 이상 테스트에서 반복되는 2줄 이상의 코드는 함수로 만들어보겠습니다!