-
Notifications
You must be signed in to change notification settings - Fork 4
[Mission003] 자바스크립트로 Form 과 Modal 에 대한 UI 만들기 #18
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
base: master
Are you sure you want to change the base?
Conversation
…or email input field
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.
템플릿까지 만드시느라 고생 많으셨습니다!
동준님의 열정에 박수드립니다!! ㅎㅎ
| <span>${family.name}</span> <span>(${family.relations})</span> | ||
| </div>` | ||
|
|
||
| const myInfoViewTemplate = (formFields) => ` |
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.
template 을 다 나눠주신 것은 보기 좋은것 같아요 ㅎㅎ
이름, 이메일, 전화번호, 비밀번호를 input 태그를 사용하여 타입을 (text, email, number, password)로 해주는것이 어떨까요?ㅎ
자기소개부분은 textarea 라는 태그를 사용해도 좋을것 같아요^^
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태그를 사용 안하신 것 같아요.
| mdcInit() | ||
| } | ||
|
|
||
| const isFilledForm = () => { |
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단계에서 정보가 다 입력했을때 하단에 다음버튼을 보여주고 안보여주고의 역할같은데,
1단계에서 이름, 전화번호, 이메일, 비밀번호의 keyup할때마다 함수를 호출해주는 것 보단, blur 이벤트를 걸어서 함수를 호출해주는 것이 어떨까요?? keyup 이벤트 사용하니 키보드 입력할때마다 매번 isFilledForm 함수가 호출되는 것 같아서,, blur 이벤트 사용하여 엘리먼트에 포커스가 해제 되었을 때만 isFilledForm 을 호출해줘도 될것 같아서요 ㅎㅎ
| $myInfoEditButton.addEventListener('click', showEditDialog) | ||
| } | ||
|
|
||
| const showEditDialog = () => { |
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.
이 함수는 step1, step2, step3에서 active, inActive 처럼 같이 쓰이는 함수가 아니어서 꼭 step3 함수 안에다가 선언 안해줘도 될 것 같아요^^
| const $bullet1ClassList = document.querySelector('.bullet1').classList | ||
| const $agreementClassList = document.querySelector('.privacy-policy-agree-container').classList | ||
|
|
||
| const active = () => { |
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.
active, inActive 함수가 step1, step2, step3 안에서도 동일하게 사용 하고 있는 것 같습니다.
step, bullet 순서만 다르게 사용되는 것 같아요.
반복되는 함수를 줄이기 위해 아래와 같이 바꿔보는것도 혹시 가능할까요?
const active = (stepClassList, bulletClasList) => {
stepClassList.remove('hidden')
bulletClasList.add('bullet-active')
}
const inActive = (stepClassList, bulletClasList) => {
stepClassList.add('hidden')
bulletClasList.remove('bullet-active')
}
위처럼 사용하면 step1, step2, step3 안에서 말고 밖에서 함수 선언 후 step1, step2, step3 안에서 적당한 파라미터를 넣어서 함수를 호출해서 사용 가능 할 것 같다는 생각이 드네요 ㅎㅎ
| const togglePrivacyCheckbox = (value) => { | ||
| validator.isEmptyString(value) ? $agreementClassList.add('hidden') : $agreementClassList.remove('hidden') | ||
| } |
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.
전화번호와 이메일이 필수항목이라 결국에는 상관이 없지만 보완점을 찾기가 어려워서 use strict로 리뷰하겠습니다.ㅎㅎ
요구사항 2번을 보면 전화번호 혹은 이메일이 하나만 입력되어도 개인정보 동의 체크박스가 나타나야하는데
전화번호와 이메일 모두 입력 후 둘 중 하나만 빈칸으로 만들어도 개인정보 체크박스가 사라질 수 있어요.
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.
어렵다기보다는 조금 손이 많이 가는 미션이었던 것 같은데, 고생하셨습니다.
항상 동준님의 코드를 보면서 제가 생각하지 못한 받식을 배우는 것 같습니다.!!
| this.init() | ||
| } | ||
|
|
||
| new FormApp() |
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.
this.user 를 두지 않고,
new Form({
//...
user: new User()
})
위와 같이 하는 방법은 안좋은 방법인가요? this.user 를 활용하는 곳이 여기 말고는 안보여서 질문드립니다..!!
| family: (event) => { | ||
| const index = event.target.closest('.user-family-form-fields').dataset.index | ||
| const inputName = event.target.name | ||
| const value = event.target.value |
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.
const t = event.target 과 같이 프로퍼티 캐싱 후 사용하는 것도 괜찮을 것 같습니다..!!
| user.addFamily(family) | ||
| const lastIndex = user.family.length - 1 | ||
| const $familyForm = document.querySelector('.user-family-forms') | ||
| $familyForm.insertAdjacentHTML('beforeend', family.render(lastIndex)) |
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.
insertAdjacentHTML() 하나 배웁니다!! ㅎㅎ
| } | ||
|
|
||
| const isFilledForm = () => { | ||
| const $step1CompleteButton = document.querySelector('.step1-complete-btn') |
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.
document.querySelector('.step1-complete-btn') 는 클래스명이 step1-complete-btn 인 첫번째 요소를 가져오는 것으로 알고 있는데요, step1-complete-btn 클래스가 3개가 보입니다. 그중 두개는 상위 엘레먼트가 hidden 이라는 클래스에 의해 제어되고 있는 것 같은데요.
display: none; 상태의 요소는 document.querySelector() 로 안가져와지는건가요? (질문입니다..!!)
|
|
||
| if (this.isAgreement && user.isBasicInfoNotEmpty()) { | ||
| $step1CompleteButton.classList.remove('hidden') | ||
| $step1CompleteButton.addEventListener('click', activeStep2) |
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 값을 지우고 다시 작성하는 과정에서 이 부분이 매번 실행될 수 있을 것 같습니다.
button 의 이벤트는 처음 Form 객체서 생성되었을 때 한번만 등록해주는 방향으로 하는것은 어떨까요?
| const activeStep1 = () => { | ||
| step3().inActive() | ||
| step2().inActive() | ||
| step1().active() |
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.
step1(), step2(), step3() 부분의 활용이 객체의 메서드를 사용하는 것과 유사해보입니다.
이부분을 클래스화 하고, 또한, 위 세 함수들 중에서도 아래의 코드들이 공통적으로 보입니다.
const $step3TabClassList = document.querySelector('#step3').classList
const $bullet3ClassList = document.querySelector('.bullet3').classList
const active = () => {
$step3TabClassList.remove('hidden')
$bullet3ClassList.add('bullet-active')
}
const inActive = () => {
$step3TabClassList.add('hidden')
$bullet3ClassList.remove('bullet-active')
}
이부분은 부모 클래스로 만들어서 상속받는 형태로, 코드를 분리해보는 것은 어떨까요?
|
|
||
| const renderViewCard = () => { | ||
| document.querySelector('#my-info-view-card').innerHTML = myInfoViewTemplate(user) | ||
| const myFamilyInfoTemplate = user.family.map((member) => familyViewTemplate(member)).join('') |
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.
user.family.map(familyViewTemplate).join('') 이렇게도 가능할 것 같습니다!!
🔗 미션 링크
#8
🏴 미션 진행 내역
💬 소감
요구사항을 어느정도 해석해야하는 부분이 일정부분 필요하여 생각보다 작업을 구현하는데 많은 시간이 걸렸습니다.
특히 2가지 부분이 힘들었는데요. 다른 분들과 같이 공감대를 나눌 수 있다면 좋을 것 같습니다 :)
1. 마크업
wizard형태의 form을 만들기 위해서 어느정도 ui가 필요하기 때문에 구현해야할 부분이 많았기에, Material library의 마크업을 활용하였습니다. 사실 공식 홈페이지의 Document가 디자인 가이드를 위한 문서여서, 마크업으로 활용하기에 그다지 친절한 느낌은 아니지만, 잘 안보게 되던 디자인 가이드문서를 다시 볼 수 있어서 좋았습니다 :)
2. wizard 형태
그동안 라이브러리를 사용했어서 직접 구현하려니 어떻게 구현해야 가장 간결할까 고민해봤는데 쉽지 않았던 것 같습니다. 현재는 step이 넘어갈 때마다 이전 스텝을 inactive하고, 넘어가는 stepd을 active하는 방식으로 class들을 조절하였는데 다른 분들은 어떻게 접근하셨는지 궁금하네요 :)