Conversation
| @@ -0,0 +1,6 @@ | |||
| import Modal from "./components/Modal.js"; | |||
| import NextButton from "./components/NextButton.js"; | |||
There was a problem hiding this comment.
react.js 컴포넌트를 만들면서 알게 된 팁인데
NextButton 을 만들게 되면, 예를 들어 이전버튼, 제출버튼 더 추가적으로 생겨날때 PrevButton, SubmitButton 을 다 따로 만들어야 할 상황이 발생 할 수 있을 것 같아요!
저도 배운건데, 저같은 경우는 Button.js 파일을 만들고 파라미터로 버튼의 상태값(?) Button('prev') 이런식으로 상태값을 받아서 구현하고 있습니다. ㅎㅎ
| }; | ||
| } | ||
|
|
||
| const render = data => { |
There was a problem hiding this comment.
혹시 여기에 받는 파라미터인 data는 나중에 어디에 쓰일 예정인지 알수 있을까요??ㅎㅎ
StellaKim1230
left a comment
There was a problem hiding this comment.
독감때문에 고생하셨어요 ㅠㅠ
미션4까지 만드시느라 더 고생하셨구요 ㅠ
처음보다는 읽기가 좋은 코드로 변하고 있는 것 같아요!!ㅎㅎ
코드 읽는데 재미있었습니다^^
| } | ||
| </form> | ||
| </div> | ||
| <button class="btn fixed" id="btnSubmit">SUBMIT</button> |
There was a problem hiding this comment.
button type 에 sumbit을 넣어주면 좋을 것 같아요^^
eastjun-dev
left a comment
There was a problem hiding this comment.
코드가 미션1,2 때보다 더 깔끔해지고 이해하기 쉬워진 것 같아요!
그래서 딱 한가지만 더 신경써주시면 더 멋진 코드가 될 것 같은데요. 한 함수가 15줄이상 넘어간다면 혹시 역할을 분리할 것은 없는지 한번 더 고민해주시면 좋을 것 같아요 :) 저도 의식적으로 함수고 10줄이상 넘어가면 혹시 분리될 수 있지 않은지, 재사용할 수는 없는지를 고민해보고 있어요. 😃
| <div class="main"> | ||
| <div class="modal" id="modal"></div> | ||
| </div> | ||
| <!-- <button class="btn fixed" id="btnNext">NEXT ></button> --> |
There was a problem hiding this comment.
사용하지 않을 거라면 주석을 제거해주시면 더 좋을 것 같아.요 다른 팀원이 해당 코드를 본다면 어떤 용도와 의미로 주석처리 해둔건지 고민하게 될 수 있을 것 같아요 :)
| Array.from(inputs).map(elem => { | ||
| elem.addEventListener("change", onCheckNull); | ||
|
|
||
| const btnAdd = document.getElementById("btnAdd"); |
There was a problem hiding this comment.
변수명이 현재 btnAdd로 명사 + 동사 형태이고, 31line의 addFamily는 동사 + 명사인데 통일된 방법으로 변수명을 지어주시면 더 좋을 것 같아요~ 개인적으로는 addBtn이 더 잘 맞지 않을까 싶습니다.
| }; | ||
|
|
||
| export const familyNodeTemplate = (data, num) => { | ||
| return `<label class="label-family"> |
There was a problem hiding this comment.
template 부분의 indent를 맞춰주시면 더 좋을 것 같아요 :)
| elem.addEventListener("change", onCheckNull); | ||
|
|
||
| const btnAdd = document.getElementById("btnAdd"); | ||
| btnAdd.addEventListener("click", () => { |
There was a problem hiding this comment.
넘길 파라미터가 없다면 아래와 같이 작성하실 수 있을 것 같아요 :)
("click", addFamily);
| const button = document.getElementById("btnNext"); | ||
| data = nextData; | ||
| inputData = data; | ||
| button.addEventListener("click", () => { |
There was a problem hiding this comment.
넘길 파라미터가 없다면 ("click", onNext)로 넘겨도 괜찮을 것 같아요 :)
| }; | ||
| } | ||
|
|
||
| const onNext = () => { |
There was a problem hiding this comment.
onNext라는 함수명도 약간 추상적인데, 함수에 로직이 많이 들어가 있어 정확히 무슨 기능을 하는 함수인지 구분하기가 좀 어려운 것 같아요. 역할에 따라 분리할 수 있는 함수들을 분리해보면 어떨까요?
input field의 유효성 검사도 하고 싶었는데, 시간 문제로 구현을 하지 못 했네요.
추후 구현 후 pr 날리겠습니다.
input field는 얼마나 디테일하게 구현하느냐에 따라, 얼마든지 계속 구현할 수 있는 것 같아요.
동준님이 템플릿 주시기 전에 만들기 시작해서, UI가 조금 다릅니다.
과제 이해에 시간이 많이 걸렸구요.
처음에는 이름, 전화번호, 이메일, 비밀번호를 전부 다른 페이지에서 입력해야 하는 줄 알았습니다.