Conversation
ZERO-black
left a comment
ZERO-black
left a comment
There was a problem hiding this comment.
수고하셨습니다.
아직 구조가 잡혀 있지 않아서 관련된 피드백은 생략했습니다.
- 재서님과 상의해서 디자인 패턴 적용해주세요.
- 주석이 한 줄도 없습니다. 어렵지 않은 코드라도 주석 남기는 습관 들이면 좋을거 같아요.
디자인 패턴 적용해서 다시 확인 받은 뒤 머지하는게 좋을 것 같습니다.
toy/src/App.js
Outdated
| for (let i = 0; i < topics.length; i++) { | ||
| if (topics[i].id === id) { | ||
| title = topics[i].title; | ||
| body = topics[i].body; | ||
| } | ||
| } |
There was a problem hiding this comment.
- break를 걸 수 있을거 같아요.
- 사실 이럴 때 사용할 수 있는 filter 메소드가 있습니다. 뭔가 구현하기 전에 기존 메소드를 활용할 수 있을지 MDN문서 한번 읽어보시는 것도 좋을거 같네요. 참고
- 밑에 로직에도 다 적용해주세요.
toy/src/App.js
Outdated
| for (let i = 0; i < props.topics.length; i++) { | ||
| let t = props.topics[i]; | ||
| lis.push( | ||
| <li key={t.id}> | ||
| <a | ||
| id={t.id} | ||
| href={"/read/" + t.id} | ||
| onClick={(event) => { | ||
| event.preventDefault(); | ||
| props.onChangeMode(Number(event.target.id)); | ||
| }} | ||
| > | ||
| {t.title} | ||
| </a> | ||
| </li> | ||
| ); | ||
| } |
toy/src/App.js
Outdated
| const newTopics = [...topics]; | ||
| newTopics.push(newTopic); | ||
| setTopics(newTopics); |
There was a problem hiding this comment.
- 스프레드 연산자로 바로 이어 붙일 수 있습니다. (따로 push 할 필요 없음)
- state를 업데이트할 때 기존 state를 직접 가져다가 쓰는 방식�은 적합하지 않습니다.
참고 - 리액트 공식문서
There was a problem hiding this comment.
이해했습니다.
함수를 따로 만드는 게 안전하겠네요!
| onClick={(event) => { | ||
| event.preventDefault(); | ||
| props.onChangeMode(); | ||
| }} |
There was a problem hiding this comment.
onClick 같은 이벤트에 바로 arrow 함수를 넣는 것도 좋은데,
나중에 재사용성을 위해서 함수로 따로 빼줘도 좋을 것 같아요!
보통 저는 함수 이름을 이벤트+함수가 하는 일 로 짓습니다. 협업 시, 일관성과 가독성을 위해 말씀드립니다!! ✨
예를 들어, 시나리오를 수정하는 onClick이벤트 같은 경우엔 onClickEditScenario 처럼요!!!
밑에 있는 onSubmit 이벤트 같은 것들도요...
| const title = event.target.title.value; | ||
| const body = event.target.body.value; |
There was a problem hiding this comment.
아래와 같이 key에 바로 접근하는 방식도 좋을 것 같아요!
const { title, body } = event.target;
|
디자인 패턴 관련해서는 아직 상의한 적 없지만, |
|
Conflict 수정 완료 |
4 participants
React basic CRUD and prettier