[2차 프로젝트] 상세화면 리펙토링 - 1

1. 기능 및 화면

1.1. 기능

클릭된 탭에 따라 border color가 바뀌고, 해당 탭에 알맞은 정보가 나온다.

1.2. 화면

2. 코드 및 문제점

2.1. 코드

  const [isClicked, setIsClicked] = useState(0);
  const onListClick = (e, input) => {
    setIsClicked(e.target.value);
  };

   <MovieInfoList onClick={onListClick}>
      {movieInfoList.map((item, index) => {
         return isClicked === index ? (
            <MovieInfoClicked value={index} key={index}>
              {item}
            </MovieInfoClicked>
          ) : (
            <MovieInfo value={index} key={index}>
              {item}
            </MovieInfo>
         );
      })}
   </MovieInfoList>

const movieInfoList = ['주요정보', '실관람평'];
const MovieInfoClicked = styled.li`
  display: inline-block;
  width: 50%;

  text-align: center;
  padding: 5px 0;
  cursor: pointer;
  border: 1px solid #503396;
  border-bottom: 0;
`;

const MovieInfo = styled.li`
  display: inline-block;
  width: 50%;

  border-top: 1px solid #ebebeb;
  border-right: 1px solid #ebebeb;
  border-bottom: 1px solid #503396;
  text-align: center;
  padding: 5px 0;
  cursor: pointer;
`;

2.2. 문제점

1. key값으로 map의 index를 썼다. key값을 map의 index로 쓰는 것은 문제가 있다.
2. 같은 기능을하면서 border 값만 달라지는데 . 시간이 없더라도 props를 통해서 코드를 간결화했어야 했다.
3. 탭메뉴인데 이름이 MovieInfoList 라고 적었다. 변수 이름 적을 때 좀 더 신경써야 한다. 나중에 코드볼 때 이게 뭔지 모를거다.

3. 리펙토링

1. Details 안에 선언된 onClick 함수를 DetailTabList의 인라인 이벤트로 넣었다. isClicked를 target value로 클릭해주는 것이 끝이기 때문이다.
   만약 li가 아닌 ul의 자식요소를 클릭하는 경우는 없는가? 없다. li를 스타일링할 때 width: 25%와 height는 자식요소값에 따라 유동적으로 바뀌도록 했기 때문이다.

2. 그럼 리스트 아이템이 바뀌지 않을 때는 map의 index로 key값을 대체해도 괜찮지않을까?
   리스트와 아이템이 정적이고, 계산되거나 변하지 않는다면, 그리고 아이템과 리스트가 id가 없다면 list item의 순서가 바뀌거나 필터링 되지 않는다면 index 써도 된다고 한다.
   key를 index로 쓰는 것이 문제가 되는 이유가 list의 길이가 바뀌거나 item의 값이 update될 때 리액트가 이전값과 비교해서 볼 때 헷갈릴 수 있기 때문인데 일리 있는 말인 것 같다.

3. 그래도 나는 key값을 nanoid 를 이용하여 넣었다. 텝 메뉴는 정적인 요소들이지만 메뉴와 데이터 모두의 각각 id에 nanoid()를 할당하여 사용해보았다.

4. clicked된 컴포넌트와 기본 컴포넌트를 합쳤다.

      <DetailTabList onClick={e => setIsClicked(e.target.value)}>
        {movieInfoList.map((item, index) => (
          <DetailTab
            isClicked={isClicked === item.idx}
            value={index}
            key={item.id}
            >
            {item.title}
          </DetailTab>
        ))}
      </DetailTabList>

      {movieInfodataList.map(item => (
        <DetailsInfoWrapper key={item.id} display={isClicked === item.idx}>
          {item.contents}
        </DetailsInfoWrapper>
      ))}

const DetailTab = styled.li`
  display: inline-block;
  width: 50%;
  text-align: center;
  padding: 5px 0;
  cursor: pointer;

  border-style: solid;
  border-width: 1px 1px ${props => (props.isClicked ? '0' : '1px')} 1px;
  border-top-color: ${props => (props.isClicked ? '#503396' : '#ebebeb')};
  border-right-color: ${props => (props.isClicked ? '#503396' : '#ebebeb')};
  border-bottom-color: ${props => (props.isClicked ? '#ebebeb' : '#503396')};
  border-left-color: ${props => (props.isClicked ? '#503396' : '#ebebeb')};
`;

4. 느낀점

프로젝트를 하면서 맨땅에 헤딩하며 머리 깨지는게 좋아서 무턱대고 코드를 친 것 같다. 내가 받는 느낌도 좋지만 효율성이 더 중요한 것 같다. 이제부터 바빠도 문서를 잘 읽어야겠다.
상세화면 쪽은 레이아웃과 기본 클릭이벤트 구현한 것이 전부인데 바꿀점이 참 많은 것 같다.

5. 참고문헌

1. React Docs
2. Index as a key is an anti-pattern