-
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단계 - 웹 기반 로또 게임] 해리(최현웅) 미션 제출합니다. #314
Conversation
- 로또 게임 컨트롤러 호출 후 이벤트 바인딩
- 로또 구입 금액 제출 - 당첨 번호, 보너스 번호 입력 제출 - 당첨 결과 모달 열고 닫기 - 로또 게임 다시 시작 버튼 클릭
- 로또 구입 금액을 입력하면 입력한 만큼 로또를 생성 후 DOM에 추가
- 당첨 번호, 보너스 번호 입력 input 뷰를 생성 후 DOM에 추가 - 당첨 번호의 첫 번째 입력에 focus 되도록 구현
- [x] 유효하지 않은 입력의 경우 에러 메시지를 input 태그 아래에 표시한다 | ||
|
||
![alt text](image-1.png) |
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 태그 아래에 표시하는 것은 기능 요구 사항을 적을 때는 고려했으나 구현은 하지 못했습니다. 체크 표시를 해버리고 제출했네요, 리뷰 요청을 하고나면 수정을 하면 안돼서 참고 부탁드립니다..! 😅
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을 다루는 것도 익숙하신 것 같고 전반적으로 모듈이 자기 일만 잘 하고 있는 것 같아 좋았습니다.
PR 내용을 보니 이번 스텝에서 좋은 고민을 많이 하신 듯 해요.
동일한 책임을 가지는 DOM을 클래스로 묶고 해당 클래스 내부에서만 참조할 수 있도록 하면 전역변수에 대한 우려와 중복해서 DOM을 찾는 우려도 해결할 수 있을 것 같아 이 방법으로 구현하게 되었습니다.
UI의 특정 영역을 컴포넌트화 하는 방식을 터득하셨군요~ 훌륭합니다.
브콜이 생각했을 때 추가적으로 UX를 위해서 고민해볼 수 있는 것들은 어떤 것들이 있을까요~?
UX에 대한 고민도 참 좋네요👍
질문 주신 김에 몇가지 던져볼게요.
- 스크롤 영역에 대해
content가 scroll 할 만큼 많지않은데 스크롤바 영역이 항상 보여지고 있습니다. 스크롤바는 필요할 때만 보여주는 게 좋을 듯해요. (스크롤을 해야한다는 메시지도 마찬가지 입니다.)
- 당첨 번호 input
input에 min/max attribute가 있어 submit 당시에는 validation을 해주긴 하지만 입력 당시부터 validation을 해줄 수 있어도 좋을 듯 해요.
- 레이아웃
viewport의 width가 작아지면 전반적으로 레이아웃이 무너집니다. 전체 contaienr에 min-width라도 적용되어 있으면 좀 나을 듯 해요.
이외에 소소한 코멘트 남겨두었으니 확인 부탁드립니다.
- scrollHeight, currentHeight을 비교하는 방법으로 수정
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.
안녕하세요 해리~
마지막까지 고생 많으셨습니다.
이번 미션은 여기서 마무리하도록 할게요!
다음도 화이팅 입니다.
@@ -21,17 +21,17 @@ export default class LottoController { | |||
#renderPurchasedLotto() { | |||
LottoPurchaseForm.resetPurchaseForm(); | |||
const lottoNumbers = this.#lottos.map((lotto) => lotto.getNumbers()); | |||
LottoPurchaseForm.renderPurchasedLottos(lottoNumbers, lottoNumbers.length); | |||
LottoPurchaseForm.renderPurchasedLottos(lottoNumbers); |
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.
👍
src/step2/dom/event.js
Outdated
lottoPurchaseForm.addEventListener("submit", (event) => { | ||
event.preventDefault(); | ||
purchaseCallback(lottoPurchaseInput.value); | ||
|
||
const formData = new FormData(lottoPurchaseForm); | ||
const lottoPurchaseAmount = formData.get("lotto-purchase-input"); | ||
purchaseCallback(lottoPurchaseAmount); |
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.
지금도 좋습니다 훌륭하네요 👍
제가 의도했던 바만 전달하고 넘어가겠습니다!
저는 event객체를 활용할 것을 기대했어요~
event.target (or currentTarget) 에서 아래와 같은 방식으로 input에 접근할 수 있거든요!
// html
<form>
<input name="foo" />
</form>
// js
lottoPurchaseForm.addEventListener("submit", (event) => {
const amount = event.taget.foo
// or
const amount2 = event.target.elements.foo
})
FormData를 사용하시더라도 특정 Form 요소를 넣어주는 것보다 event.target을 넣어주면 어떨까 싶어요.
큰 차이는 없겠지만 명시적으로 실제 발생한 그 event에 대한 핸들링을 하게 하기 위한 목적입니다.
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.
아하, 그렇군요..!
명시적으로 실제 발생한 그event에 대한 핸들링을 하게 하기 위한 목적입니다.
확실히 이벤트에 대한 핸들링이라는 것을 더 명시적으로 표현할 수 있어서 좋은 것 같아요!
좋은 인사이트 주셔서 감사해요~
안녕하세요, 브콜! 2단계도 잘 부탁 드립니다 :)
배포 링크
웹 기반 로또 게임
실행 방법
구현 방법
1. DOM 조작
step1이 위와 같은 구조였다면, step2는
위와 같은 구조를 가진다고 생각했습니다. 따라서, 로또 게임을 진행하기 위한 동작을 컨트롤러에서 구현하고 해당 동작을 트리거 하는 이벤트를 등록해주기로 했습니다. 즉,
컨트롤러가 위와 같은 동작을 구현하면, 이 동작을 등록하기 위한 함수를 아래와 같이 구현하고 이 함수를 실행함으로써 이벤트를 등록할 수 있도록 했습니다.
2. 중복되는 스타일 재사용하기
css를 사용하면서, 중복되는 스타일을 계속해서 작성해주는 것에 대한 불편함이 생겼습니다. 예를 들어,
위 버튼 3개의 스타일은
width
를 제외하면 모두 같았습니다. 이 스타일을 모두 중복해서 작성하는 것에 대한 불편함을 느껴공통된 스타일 속성들은 묶고 공통되지 않은 것들은 다른 곳에서 정의해서 사용할 수 있도록 했습니다. 버튼 뿐만 아니라
위 스타일 처럼 여기저기서 많이 사용될 수 있는 스타일들도 따로 빼서 필요한 곳에서 추가해서 사용할 수 있도록 구현했습니다.(이렇게 구현하다보니
tailwind-CSS
를 쓰는듯한 느낌도 받았습니다..😊 )고민내용
1. DOM을 안전하게 다루기 위해서는?
이번 미션을 진행할 때, DOM을 조작하면서 기존에는
참조할 DOM을 전역에 두고 해당 DOM 객체가 필요한 곳에서 참조해서 어떤 동작을 구현하도록 했습니다. 하지만 이 방법은 DOM 객체가 전역변수가 되어버리기 때문에 의도하지 않은 사이드 이펙트에 대한 우려가 있어서, 함수 스코프 내부로 옮겨보기로 했습니다. 예를 들어,
로또 구입 금액을 입력하고 당첨 번호와 보너스 번호를 입력받기 위한 폼을 렌더링 하거나 예외가 발생했을 경우 해당 폼을 리셋하는 동작이 필요하다면 해당 동작을 하는 함수 내부에서만 DOM 객체를 참조하고 동작을 할 수 있도록 했습니다. 전역변수로 두고 참조하는 것 보다 안전하겠지만, 저는 또 다른 불편함이 생겼습니다. 중복에 대한 불편함이였는데 어떤 DOM 객체에 필요한 동작에 비례해서 해당 DOM을 참조하는 함수가 생길 것이고, 중복해서 한 DOM을 계속해서 찾는 동작이 생기는 상황에 대한 우려 때문이었습니다. 이를 해결해 볼 방법을 생각하다가, 동일한 책임을 가지는 DOM을 클래스로 묶고 해당 클래스 내부에서만 참조할 수 있도록 하면 전역변수에 대한 우려와 중복해서 DOM을 찾는 우려도 해결할 수 있을 것 같아 이 방법으로 구현하게 되었습니다.
2. UX
사용자의 입장에서 개발하면 UX를 향상시킬 수 있을 것 같아서 다음과 같은 내용을 고민하고 적용해보았습니다.
autofocus
overflow-y : scroll
, 스크롤할 수 있다는 메시지 추가alert
를 사용하는 것 보다, 입력 폼 아래에 빨간색 메시지를 보여주기5번의 경우, 위와 같은 내용들을 고민하면서 시간이 생각보다 많이 지체되었고 리뷰를 요청한 후 리팩토링하는 시간까지 생각해보았을 때 이번 미션은
alert
를 사용하기로 했습니다. 브콜이 생각했을 때 추가적으로 UX를 위해서 고민해볼 수 있는 것들은 어떤 것들이 있을까요~?