Ahyoon

[ahyoon99]

카드삭제, 등록날짜, 담당자, 우선순위 추가

[raccoonback]

기본 구조가 복잡해서 파악하기 힘들었을텐데....;

요구사항에 맞춰서 잘 구현해준 거 같아~ 👍 👍 👍

몇 가지 리뷰남겼는데, 시간있으면 참고해서 수정해봐도 좋을 거 같아~

[raccoonback]

{}로 감싸지 않아도 될 거 같아

[raccoonback]

아래 33~35번 라인에

function(a,b){
   return a.priority<b.priority?-1:a.priority>b.priority?1:0;
 }

함수하고 중복됬는데, 별도의 함수로 분리하는게 좋을 거 같아~
예를 들어 추후에 다른 정렬 방식을 모두 변경해야 하는 경우,
함수로 정의해뒀다면 해당 함수안에서만 변경해주면 되기 때문에 빠르게 변경사항을 적용할 수 있어

function compare(a,b){
   return a.priority<b.priority?-1:a.priority>b.priority?1:0;
 }

 ...

updateCards({ 
  todo: cards.todo.sort(compare),
  done: cards.done.sort(compare)
});

[raccoonback]

불필요한 콘솔로그는 제거하는 게 좋아~

[raccoonback]

해당 코드가 56번 라인의 index = done.findIndex((value) => { return value.id === id;}); 코드하고 비슷한 형식으로 중복되는 것을 볼 수 있는데,

todo-frontend/static/src/page/Board.jsx

Lines 56 to 58 in 54e1f7e

	index = done.findIndex((value) => { //index 알아내기
	return value.id === id;
	});

remove() 함수 인자로 status 값까지 받아온다면, 아래처럼 좀더 일반화된 방식으로 중복 코드를 제거할 수 있을 거 같아~

function remove(id, status) {
       ...
       const index = cards[originStatus].findIndex((task) => task.id === id);
       ...
}

[raccoonback]

초기화 굿굿 👍

[raccoonback]

불필요한 주석 또는 콘솔 로그 삭제~

[raccoonback]

삭제 성공한 경우, status code => 204(No Content)
https://docs.google.com/document/d/18hPE1bP4o4xP9xIAbHOL5-baelotFG9_zz8uJUPPVzw/edit

[raccoonback]

리액트에서는 브라우저에서 지원하는 DOM에 직접 접근하는 방식보다 Ref 라는 방식
을 통해서 Element에 접근하는 것을 권장하고 있어.

아래 링크를 통해서도 자세한 이유를 확인할 수 있어
Ref가 document.getElementById 보다 나은 이유

(확실하진 않지만, DOM에 직접 접근하게 되면 React가 제공하는 가상 DOM의 장점을 잃게 되지 않을까..?)

Hooks API로는 useRef()를 사용할 수 있어~

const priorityEl = useRef(null);

function onDisplayPriority() {
  priorityEl.current. selectedIndex = "1";
}

return (
   ...
   <select name="priority" id="priority" onChange={onChangePriority}  ref={el}>
      ....
    </select>
   ....
);

[raccoonback]

<select> 태그의 value 속성을 priority으로 설정해주면, onDisplayPriority() 메서드 없이도 setPriority(2) 호출만으로 변경이 반영될 거 같아~

const [priority, setPriority] = useState(2);
...
const onClearClick = (event) => {
  ...
  setPriority(2);
  ....
}
...

return (
   ...
    <select name="priority" id="priority" onChange={onChangePriority}  value={priority}>
      ....
    </select>
    ....
);

[raccoonback]

인덱스로 <option> 지정할 수 있는 selectedIndex 프로퍼티를 제공하는구나
굿굿 👍 👍
https://www.w3schools.com/jsref/prop_select_selectedindex.asp

[so3500]

👍