Skip to content

Conversation

@s280493
Copy link
Collaborator

@s280493 s280493 commented Jan 9, 2020

  • Requirement 중, 3번인 <버튼을 클릭하면 폼이 제출되도록 한다>는 구현하지 않았고, <마지막에 입력된 정보를 확인한 뒤, 확인 버튼을 누르면 폼이 제출되도록 한다>로 구현했습니다.
  • 수정하고 싶은 내용이 있을 경우 수정할 수 있도록 한다. 는 구현 예정입니다.

input field의 유효성 검사도 하고 싶었는데, 시간 문제로 구현을 하지 못 했네요.
추후 구현 후 pr 날리겠습니다.

  • 최대 길이
  • 한글/영어/특문/숫자 제한
  • 전화번호, 이메일 등의 형태가 올바른가

input field는 얼마나 디테일하게 구현하느냐에 따라, 얼마든지 계속 구현할 수 있는 것 같아요.


동준님이 템플릿 주시기 전에 만들기 시작해서, UI가 조금 다릅니다.

과제 이해에 시간이 많이 걸렸구요.
처음에는 이름, 전화번호, 이메일, 비밀번호를 전부 다른 페이지에서 입력해야 하는 줄 알았습니다.

@s280493 s280493 changed the title Mission003/jisun [Mission003] 자바스크립트로 Form 과 Modal 에 대한 UI 만들기 Jan 9, 2020
@@ -0,0 +1,6 @@
import Modal from "./components/Modal.js";
import NextButton from "./components/NextButton.js";
Copy link
Collaborator

Choose a reason for hiding this comment

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

react.js 컴포넌트를 만들면서 알게 된 팁인데
NextButton 을 만들게 되면, 예를 들어 이전버튼, 제출버튼 더 추가적으로 생겨날때 PrevButton, SubmitButton 을 다 따로 만들어야 할 상황이 발생 할 수 있을 것 같아요!
저도 배운건데, 저같은 경우는 Button.js 파일을 만들고 파라미터로 버튼의 상태값(?) Button('prev') 이런식으로 상태값을 받아서 구현하고 있습니다. ㅎㅎ

};
}

const render = data => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

혹시 여기에 받는 파라미터인 data는 나중에 어디에 쓰일 예정인지 알수 있을까요??ㅎㅎ

Copy link
Collaborator

@StellaKim1230 StellaKim1230 left a comment

Choose a reason for hiding this comment

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

독감때문에 고생하셨어요 ㅠㅠ
미션4까지 만드시느라 더 고생하셨구요 ㅠ

처음보다는 읽기가 좋은 코드로 변하고 있는 것 같아요!!ㅎㅎ
코드 읽는데 재미있었습니다^^

}
</form>
</div>
<button class="btn fixed" id="btnSubmit">SUBMIT</button>
Copy link
Collaborator

Choose a reason for hiding this comment

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

button type 에 sumbit을 넣어주면 좋을 것 같아요^^

Copy link
Owner

@eastjun-dev eastjun-dev left a comment

Choose a reason for hiding this comment

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

코드가 미션1,2 때보다 더 깔끔해지고 이해하기 쉬워진 것 같아요!
그래서 딱 한가지만 더 신경써주시면 더 멋진 코드가 될 것 같은데요. 한 함수가 15줄이상 넘어간다면 혹시 역할을 분리할 것은 없는지 한번 더 고민해주시면 좋을 것 같아요 :) 저도 의식적으로 함수고 10줄이상 넘어가면 혹시 분리될 수 있지 않은지, 재사용할 수는 없는지를 고민해보고 있어요. 😃

<div class="main">
<div class="modal" id="modal"></div>
</div>
<!-- <button class="btn fixed" id="btnNext">NEXT ></button> -->
Copy link
Owner

Choose a reason for hiding this comment

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

사용하지 않을 거라면 주석을 제거해주시면 더 좋을 것 같아.요 다른 팀원이 해당 코드를 본다면 어떤 용도와 의미로 주석처리 해둔건지 고민하게 될 수 있을 것 같아요 :)

Array.from(inputs).map(elem => {
elem.addEventListener("change", onCheckNull);

const btnAdd = document.getElementById("btnAdd");
Copy link
Owner

Choose a reason for hiding this comment

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

변수명이 현재 btnAdd로 명사 + 동사 형태이고, 31line의 addFamily는 동사 + 명사인데 통일된 방법으로 변수명을 지어주시면 더 좋을 것 같아요~ 개인적으로는 addBtn이 더 잘 맞지 않을까 싶습니다.

};

export const familyNodeTemplate = (data, num) => {
return `<label class="label-family">
Copy link
Owner

Choose a reason for hiding this comment

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

template 부분의 indent를 맞춰주시면 더 좋을 것 같아요 :)

elem.addEventListener("change", onCheckNull);

const btnAdd = document.getElementById("btnAdd");
btnAdd.addEventListener("click", () => {
Copy link
Owner

Choose a reason for hiding this comment

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

넘길 파라미터가 없다면 아래와 같이 작성하실 수 있을 것 같아요 :)
("click", addFamily);

const button = document.getElementById("btnNext");
data = nextData;
inputData = data;
button.addEventListener("click", () => {
Copy link
Owner

Choose a reason for hiding this comment

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

넘길 파라미터가 없다면 ("click", onNext)로 넘겨도 괜찮을 것 같아요 :)

};
}

const onNext = () => {
Copy link
Owner

Choose a reason for hiding this comment

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

onNext라는 함수명도 약간 추상적인데, 함수에 로직이 많이 들어가 있어 정확히 무슨 기능을 하는 함수인지 구분하기가 좀 어려운 것 같아요. 역할에 따라 분리할 수 있는 함수들을 분리해보면 어떨까요?

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.

5 participants