Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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단계 - 웹 기반 로또 게임] - 버건디(전태헌) 미션 제출합니다. #296
[2단계 - 웹 기반 로또 게임] - 버건디(전태헌) 미션 제출합니다. #296
Changes from 91 commits
fdfcd07
c253530
69229a1
f6a82ea
30663f4
95befe2
809ad41
bc8db2a
397fa59
78789df
40534e9
725017c
c472560
afa9e77
c6887ec
508421f
9e7379c
1eb10a2
fd87bfc
70c32e1
57e8da5
b231ab2
3e6406d
a6b5882
664aa91
69137d9
418ce55
15cf4ce
2e569a2
10f90da
1cc0870
6d49a41
b16c0fb
449235f
cef738a
0a25e49
9d864e6
ac843b7
496c66b
faefdb8
33ef117
45bb854
1277912
96e58fa
7b08843
85b106e
c85d82a
a91b64a
86164ee
dc51b8a
a42dc1b
de31467
dd817cf
e0f5e40
5736c0f
0ddc5d4
7320cc8
41a9e0d
8c5635c
e787be9
a082919
f8f09bb
d877f0c
a43db49
859a7c8
9406ad6
8e7563c
71d5908
ed2f44c
68771df
12c69ef
6ea399d
2053d88
2c50958
23d0398
722e096
50aae06
38b29a9
3e14516
13353c6
ffdd462
c9b2342
0f557fb
e45034c
d248b5c
db823ea
01a8094
cf389cf
cc62e16
733f435
189aea0
726f3ed
6b35665
45b5d15
a841560
74f70e6
428dc81
d9ab863
0e5ef18
1b076c3
dc13d2d
95a4b3b
b59570a
07475e7
bba910f
b87b28e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
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.
id와 class를 작성하는 기준이 있으실까요?
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.
class 는 단순히 styling을 위한 명,
id는 js에서 직접 하나의 값을 불러와서 이벤트를 처리해주어야할때 라는 기준으로 작성했습니다!
하지만 당첨 번호 관련된 인풋들 같은 경우에, 동일한 도메인이지만 하나의 인풋만 처리해주는것이 아니다보니 class로 가져와서 여러 인풋을 한번에 처리해주려고 했습니다.
정리하자면 id => js내에서 동적으로 이벤트를 핸들링 하기 위해서 고유한 값을 부여
class => js 내에서 여러개의 동일 속성을 핸들링 하기 위해서 부여할수도 있지만, 스타일링을 먼저 고려해서 부여
라고 말씀드려볼 수 있을것 같습니다.
혹여나 제가 잘못생각하고 있는 부분이 있다면 짚어주신다면 감사하겠습니다! 🙇
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.
잘못 생각하신 건 아니고 버건디의 기준이 궁금해서 여쭤봤습니다 :) 다만 저라면, js와 css로 구분 두기보다는 후반에 설명 주신 것처럼 '유일한 요소'와 '다중 요소'로 기준을 두어 css, js 내에서 각 선택자를 유연하게 사용하는 편이 DX 상 적합한 방식이지 않을까 싶어요!
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.
이번에 1차 pr을 한후에 제가 시멘틱 태그와 기본적인 css에 관해서 너무 무지했었다는걸 깨닫게 되었습니다..!
이번에 pr을 준비하면서는 최대한 시멘틱 태그를 지키고, css 컨벤션을 지켜보려고 해보았습니다..!
하지만 아직 BEM 방식에 익숙치 않아, 네이밍이 매끄럽게 된건지는 확실치 않은것 같습니다..!
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.
말씀 감사합니다! 한번 다듬어 나가보겠습니다!
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.
(질문)
label
의for
는 어떤 목적으로 사용하는 걸까요?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.
input의 name 값과 label의 for 값을 매칭시켜주어서 label을 클릭하면 input에 포커싱이 가도록 사용자의 편의성을 고양시켜주는 것으로 알고 있는데요..!
지금 살펴보니 label for 속성 값과 input의 name 값이 다르네요..
짚어주셔서 감사합니다..!
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.
이렇게 동일한 요소가 반복되는 경우 js를 이용해서 동적으로 처리할 수 있는 방법은 없을까요~?
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.
제가 현재 알고 있는 바로는, html에서는 필요한 부분을 일단 다 작성을 해주고 js 에서 기능적인 부분 (에를 들어서 값을 따로따로 받아야할때)에 관해서는 querySelectorAll 이나 각각 input에 다른 id 값들을 주어서 처리하는 방식인데요..!
혹시 카일이 말씀하시는 부분은 기능적인 부분이 아니라, 아예 저 input 태그를 js를 통해 반복적으로 렌더링해주는걸 말씀하시는 걸까요 ??
만약 그렇다면
createElement
메서드를 통해서 아예 js 코드가 실행될때 바로 input 태그를 생성해줄수 있을거라고 예상하는데,근데 만약 이렇다면 직접적으로 DOM에 접근을 하게 되는거라서 비용이 많이 들지 않을까? 라는 생각도 듭니다..!
만약 100개가 넘을정도로 많은 동일 요소가 필요하다면 js 내에서 동적으로 처리해주는것이 맞겠으나, 현재는 필요한 동일 요소가 6개이기 때문에 직접 적어주어도 괜찮지 않을까 ?라는 생각도 동시에 들긴 드는데 카일의 의견도 한번 말씀 주시면 감사하겠습니다!
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.
타당한 의견이라고 생각합니다. 다만 아래와 같은 부분을 같이 고려해보시면 좋을 것 같아요.
따라서 어떤 방식을 사용하는 데 있어 트레이드 오프를 고려하는 습관은 지향하는 게 좋지만, 어느 한쪽에 치우쳐 이미 제공되고 있는 방법론을 아예 배제하는 자세는 경계해야 하는 부분이라고 생각해요. 저라면 DOM을 조작하는 방식은 적극적으로 사용하되, 성능을 고려한 코드 패턴을 찾아보시면서 (이벤트 위임, reflow 최소화 등) 본인만의 타협점을 찾아보면 어떨까 싶네요!
+ 성급한 성능 최적화를 지양해야 하는 이유에 대해서도 같이 찾아보시면 좋을 것 같습니다~
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.
여기 안에 있는 매직 넘버들도 상수로 같이 관리할 수 있는 방법이 없을까요?
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.
이 또한 현재 상수 객체로 분리해서 js 파일 내에서
createElement
를 통해 직접 만들어주는 방법이 생각납니다..!카일의 리뷰를 보다보니, 제가 js를 통해 직접 DOM에 내용물을 추가하는 것을 비용적으로 많이 드는 것이라고 생각하여 무의식적으로 계속 지양했던것 같다는 생각이 듭니다.
예를 들어서 아예 텍스트가 없다가, 어떠한 기능이 실행 된후에 텍스트가 생겨야만 하는 상황이라면 (직접 DOM에 내용물을 삽입해야하는경우) js를 사용 할수밖에 없지! 라고 생각했었는데요.
지금 현재 이미 하드코딩으로 레이아웃을 잡아줄수 있는 부분들이 있다면, 굳이 js로 렌더링을 안해주려고 했던것 같습니다.
갯수가 수십, 수백개여서 제가 직접 수동으로 일일히 넣어주기 힘든 경우가 아니라고 판단해서 직접 html 내에서 작성해주었던것 같아요..!
이런 관점에 대해서 카일은 어떻게 생각하실지 여쭈어 보고 싶습니다!
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.
script
태그에type="module"
을 선언하면 어떤 이점이 있을까요?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.
이번에 자바스크립트의
module
과 관련하여 pr을 올린후 정리해보았는데요..!요약하자면,
module
의 선언이 없다면 각각의 스크립트는 분리되어 작성 되더라도 같은 스코프를 공유하기때문에 변수 오염이나 에러가 발생할 가능성이 크다.하지만
module
의 선언을 통해서 각각의 독립된 스코프를 보장해줄수 있다. 라고 말씀드려볼수 있을것 같습니다!글 링크
한번 읽어봐주시고 혹여나 잘못된 부분이 있으시다면 지적해주시면 새겨 듣겠습니다..!
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.
오.. 모르는 지식에 대해 블로그로 바로 정리하시다니 바람직한 습관을 갖고 계시는 군요 ㅎㅎ 현재 정리하신 부분에 추가로 말씀 드리고 싶은 부분은
type="module"
을 선언한다는 것은defer
속성을 내포하고 있기 때문에 굳이body
하단에 스크립트 태그를 명시하지 않아도 된다는 것이었습니다!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.
.js
suffix를 붙여주게 된 이유가 있을까요? 이거 때문에 file diff가 더 생긴 거 같아서 여쭤봅니다~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.
현재 웹팩에서
resolve
속성에.js
가 포함이 되어있어서 step1 때는 확장자에 js파일을 붙이지 않더라도 에러가 발생하지 않았었는데요.이번 step2에서는 왜인지 모르게 js 확장자가 없으면 에러가 발생해서, 임의로 전부 다 붙여주게 되었습니다..🥲
- step2.config.js
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.
넵 이해했습니다~ 현재 PR에서 다룰 영역은 아니라 이에 대한 자세한 피드백은 스킵하겠지만, 추후 webpack같은 번들러를 학습하실 때 이 문제에 대한 원인이나 해결방법도 고민해보시면 좋겠네요!
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.
말씀 감사합니다!