[FE] refactor: Checkbox, CheckboxItem 컴포넌트 역할 분리

[ImxYJL]

• resolves [FE] Checkbox 컴포넌트의 책임을 분리한다. #997

---

🚀 어떤 기능을 구현했나요 ?

• 기본 체크박스 형식 외에 읽기 전용 기능, 접근성 지원이 추가되며 두 컴포넌트가 담고 있는 내용이 점점 많아졌습니다.
• 또 공통 컴포넌트를 사용하는 개발자들에게 기본적으로 필요한 속성을 안내해주고 싶었습니다.
• 코드 가독성 및 유지보수를 위해 기본 체크박스를 만들고, 그 체크박스를 상속받는 특화 체크박스들을 만들었습니다.

🔥 어떻게 해결했나요 ?

상속 구조

• -Base 컴포넌트는 공통 속성(id, label, isChecked 등등)을 기본으로 요구하고, 스타일링을 담당하는 기본 컴포넌트입니다.
• onChange 핸들러를 통해 사용자와 상호작용 가능한 컴포넌트, readonly인 컴포넌트는 일단 Base를 상속받습니다.
• 그리고 각자 필요한 속성을 추가해서 사용합니다.

용도별 인터페이스 분리

• 기존에는 style용 인터페이스와 컴포넌트 인터페이스만 있었는데, Checkbox 컴포넌트 인터페이스가 길어짐에 따라 역할을 명확히 하기 위해 접근성 전용 인터페이스를 만들었습니다.

컴포넌트 분리로 필요한 속성만 전달

컴포넌트 분리의 연장선입니다.

• 기존 코드
  • 읽기 전용 체크박스라면 항상 $isReadonly, isDisabled가 true임에도 매번 값을 전달해야 합니다.

<CheckboxItem
  key={`${question.questionId}_${index}`}
  id={`${question.questionId}_${index}`}
  name={`${question.questionId}_${index}`}
  isChecked={isSelectedChoice(question.questionId, option.optionId)}
  label={option.content}
  // 항상 true인 값들도 매번 전달해야함
  isDisabled={true}
  $isReadonly={true}
/>

• 현재 코드
  • 컴포넌트 이름을 통해 컴포넌트의 동작을 직관적으로 확인할 수 있습니다.
  • readonly 동작을 위한 기본 속성을 사용하는 쪽에서 전달하지 않아도 됩니다.
    즉 동적으로 꼭 전달해야 하는 값만 보낼 수 있습니다.

<ReadonlyCheckboxItem
  key={`${question.questionId}_${index}`}
  id={`${question.questionId}_${index}`}
  name={`${question.questionId}_${index}`}
  label={option.content}
  isChecked={isSelectedChoice(question.questionId, option.optionId)}
/>

기본 인터페이스를 제공하되, rest prop 사용 가능

• 이건 기존 방식과 똑같습니다!
• rest를 사용하고 있지만, 기본적으로 필요한 속성들은 인터페이스로 명시하는 방식입니다.
• 따라서 개발할 때 컴포넌트에 마우스를 갖다 대면 필요한 속성들을 확인할 수 있습니다.

• 일반 CheckboxItem
  
• ReadonlyCheckboxItem
  

• 예전에는 컴포넌트 분리가 되어 있지 않아서, 모든 속성을 안내했습니다.
  

📝 어떤 부분에 집중해서 리뷰해야 할까요?

• 같은 리팩토링이지만, 앞 PR은 리뷰 반영이었고 이번 PR은 그냥 해 보고 싶어서 한 것입니다.
• 리뷰 제일 나중에 해 주셔도 됩니다. close를 각오하고 올립니다 ㅎㅎㅎ

📚 참고 자료, 할 말

이제 복원 로직 리팩토링하러 갑니다 🙂

[cloudflare-workers-and-pages]

Deploying 2024-review-me-release with    Cloudflare Pages

Latest commit:	c382dbf
Status:	✅  Deploy successful!
Preview URL:	https://e272ebb4.2024-review-me-release.pages.dev
Branch Preview URL:	https://fe-refactor-997-checkbox-ref.2024-review-me-release.pages.dev

View logs

[BadaHertz52]

CheckboxItemProps가 이미 CheckboxItemBaseProps를 확장하고 있는데, interface FinalCheckboxItemBaseProps extends CheckboxItemBaseProps, CheckboxItemProps{}; 라고 타입을 지정해야하는 이유가 있나요?

[BadaHertz52]

props에 handleKeyDown은 Checkbox에게 전달되고, 컴포넌트안에서 handleKeyDown을 또 선언하고 있어서 헷갈리네요 🤔

[BadaHertz52]

CheckboxItemBase가 스타일링을 담당하는 기본 컴포넌트라고 설명이 되어 있지만, 체크 박스나 라벨 선택 시 선택/해제 기능을 위해서 onKeyDown이라는 이벤트를 가지는 건가요?

[BadaHertz52]

기존에 구현 사항에 충돌 때문에 클래스명을 checkbox-item이라고 한건가요?

CheckboxItem 내에서 CheckboxItemBase가 사용되고 CheckboxItemBase의 클래스가 checkbox-item인 구조가 헷갈렸어요.

[BadaHertz52]

올리 컴포넌트 역할이 많아져서, 책임을 분리하고 싶었군요.

의도와 시도는 좋으나, 제가 느꼈을 때 체크박스 ui 구조, 스타일이 바뀌었을 때 변경해야하는 부분이 많아서 유지 보수 측면에서 좋을까, 체크 박스 컴포넌트를 구현한 올리가 아닌 다른 개발자가 사용했을 때 이해하기 쉬울까라는 점에서 생각을 해봐야할 것 같아요.

스타일을 담당하는 컴포넌트라는 책임이 컴포넌트명에 더 들어났으면 좋겠어요.
비슷한 컴포넌트명이 많고, 체크 박스라는 단순한 컴포넌트에 비해 계층 구조가 있는 편이라 헷갈렸어요. (CheckboxItem, CheckboxItemBase, CheckBox, CheckBoxBase....)

열심히 구현했는데 아쉬운 피드백을 남기게 되어서 미안하네요 🥹
그래도 더 좋은 코드를 위해 시도하는 올리의 모습 칭찬합니다. 👍