[FE] Checkbox component 변경 #960

Todari · 2025-01-19T19:10:12Z

issue

close [FE] Checkbox component 변경 #959

구현 목적

Icon component 변경 이후, checkbox가 시각적으로 제대로 작동하지 않았습니다. 이를 올바르게 작동하도록 수정합니다.
checkbox 상태 변경 시 기본적인 animation이 없습니다.
checkbox에 ref를 넘겨줄 수 없으며, 비제어 컴포넌트로 사용할 수 없습니다.
접근성 관련 태그 및 기능이 존재하지 않습니다.

구현 사항

Style을 변경하여 checkbox가 시각적으로 옳게 작동하도록 변경하였습니다.
체크 상태 변경 시 animation이 적용되도록 변경하였습니다.

2025-01-20.4.05.09.mov

ref를 input에 전달할 수 있게 하였고, 외부에서 checked를 넘겨주지 않더라도, 비제어 component로 사용할 수 있도록 변경하였습니다.
접근성 태그를 추가하고, 키보드 접근성을 위한 기능을 추가하였습니다.

2025-01-20.4.09.14.mov

변경 전

const Checkbox = ({labelText, isChecked = false, onChange}: Props) => {
  const {theme} = useTheme();
  return (
    <label css={checkboxStyle}>
      <div css={inputGroupStyle({theme, isChecked})}>
        {isChecked ? <IconCheck size={20} color="onPrimary" className="check-icon" /> : null}
        <input type="checkbox" checked={isChecked} onChange={onChange} className="checkbox-input" />
      </div>
      {labelText && <Text size="bodyBold">{labelText}</Text>}
    </label>
  );
};

변경 후

const Checkbox = forwardRef<HTMLInputElement, CheckboxProps>(
  ({children, checked: controlledChecked, onChange, defaultChecked = false, ...props}, ref) => {
    const {theme} = useTheme();
    const [internalChecked, setInternalChecked] = useState(defaultChecked);

    const isControlled = controlledChecked !== undefined;
    const checked = isControlled ? controlledChecked : internalChecked;

    const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
      if (!isControlled) {
        setInternalChecked(e.target.checked);
      }
      onChange?.(e);
    };

    const handleKeyDown = (e: React.KeyboardEvent) => {
      if (e.key === ' ' || e.key === 'Enter') {
        e.preventDefault();
        const input = e.currentTarget.querySelector('input');
        if (input) {
          input.click();
        }
      }
    };

    return (
      <label css={checkboxStyle} role="checkbox" aria-checked={checked} tabIndex={0} onKeyDown={handleKeyDown}>
        <div css={boxStyle({theme, checked})} aria-hidden="true">
          {checked && <IconCheck size={20} color="onPrimary" />}
          <input
            ref={ref}
            type="checkbox"
            checked={checked}
            onChange={handleChange}
            css={invisibleInputStyle}
            aria-label="체크박스"
            {...props}
          />
        </div>
        {children}
      </label>
    );
  },
);

중점적으로 리뷰받고 싶은 부분(선택)

어떤 부분을 중점으로 리뷰했으면 좋겠는지 작성해주세요.

논의하고 싶은 부분(선택)

논의하고 싶은 부분이 있다면 작성해주세요.

🫡 참고사항

github-actions · 2025-01-19T19:12:59Z

🚀 storybook: https://66a9dea5db27afc2b8f91ba7-busqodelfj.chromatic.com/

github-actions · 2025-01-19T20:06:43Z

🚀 storybook: https://66a9dea5db27afc2b8f91ba7-xozcgspvbr.chromatic.com/

github-actions · 2025-01-20T02:09:53Z

🚀 storybook: https://66a9dea5db27afc2b8f91ba7-mypryoyccb.chromatic.com/

jinhokim98

고생했습니다~ 덕분에 체크박스에 애니메이션도 생기고 너무 좋은 것 같아요!
궁금한 점이 있어 몇 가지 코멘트 남겼습니다

jinhokim98 · 2025-01-20T03:16:36Z

client/src/components/Design/components/Checkbox/Checkbox.style.ts

@@ -3,7 +3,7 @@ import {css} from '@emotion/react';
 import {WithTheme} from '@components/Design/type/withTheme';

 interface CheckboxStyleProps {
-  isChecked: boolean;
+  checked: boolean;


isChecked보다 checked가 더 자연스럽다고 생각해서 is 키워드를 제거하신 걸까요!?

애초에 기본적으로 input 태그의 prop으로 있는 이름이 checked 에요
기존에는 input props를 extends하지 않고, isChecked, onChange, labelText 세개의 prop만 받았는데, 이렇게 구현하면 나중에 접근성 태그나 disabled 등 input(Checkbox)를 커스터마이징 하는데 어렵다고 생각해서 input props를 extends 하면서 자연스럽게 isChecked를 input의 기본 prop인 checked로 바꿨습니다

그렇군요. 기본 태그를 활용해서 checked로 변경하는 것 동의합니다.

jinhokim98 · 2025-01-20T03:17:02Z

client/src/components/Design/components/Checkbox/Checkbox.style.ts

+    transition: 'all 0.2s',
+    transitionTimingFunction: 'cubic-bezier(0.7, 0, 0.3, 1)',
+    '&:focus-visible': {
+      outline: `2px solid ${theme.colors.primary}`,
+      outlineOffset: '2px',


와우... 애니메이션의 신 토다리

jinhokim98 · 2025-01-20T03:23:01Z

client/src/components/Design/components/Checkbox/Checkbox.tsx

+    const {theme} = useTheme();
+    const [internalChecked, setInternalChecked] = useState(defaultChecked);
+
+    const isControlled = controlledChecked !== undefined;


비제어 컴포넌트를 고려하신 이유가 궁금해요!

제가 생각하는 비제어 컴포넌트의 장점은 state의 prop drilling 없이 form submit 시 name으로 값을 가져올 수 있다는 것이에요. 혹시 이점을 고려해서 만든 것인지 아니면 토다리가 생각하는 다른 장점이 있어 비제어 컴포넌트 방식을 고려했는지 궁금합니다~

제어 컴포넌트와 비제어 컴포넌트 각각의 장점이 있겠지만, 대부분 상황에서 제어 컴포넌트를 사용할 것 같아요. 그렇지만, 비제어 컴포넌트의 가능성을 아에 닫아두는것 보단, 일단 열어두고 사용할 수 있다면 사용할 수 있는게 맞다고 판단했습니다

추후에 비제어로 접근할 수도 있으니 열어둔다는 의미였군요. Checkbox는 공통 컴포넌트이니깐 사용성을 열어둔다.. 좋은 것 같아요!

jinhokim98 · 2025-01-20T03:24:42Z

client/src/components/Design/components/Checkbox/Checkbox.tsx

+        e.preventDefault();
+        const input = e.currentTarget.querySelector('input');
+        if (input) {
+          input.click();


오 내부의 input을 찾아서 클릭 좋네요

jinhokim98 · 2025-01-20T03:25:02Z

client/src/components/Design/components/Checkbox/Checkbox.tsx

+    };
+
+    return (
+      <label css={checkboxStyle} role="checkbox" aria-checked={checked}>


접근성까지 야무지게 챙겨가는 토다리 좋습니다~

github-actions · 2025-01-20T04:22:45Z

🚀 storybook: https://66a9dea5db27afc2b8f91ba7-jwnffogiow.chromatic.com/

github-actions · 2025-01-20T04:35:58Z

🚀 storybook: https://66a9dea5db27afc2b8f91ba7-dvotflbcus.chromatic.com/

pakxe

오와우 이제 체크박스가 이쁘게 작동하네요. 너무너무 죠습니다 👍 고생많았어요.

pakxe · 2025-01-21T03:31:59Z

client/src/components/Design/components/Checkbox/Checkbox.tsx

+import {CheckboxProps} from './Checkbox.type';
+
+const Checkbox = forwardRef<HTMLInputElement, CheckboxProps>(
+  ({right, checked: controlledChecked, onChange, defaultChecked = false, disabled, ...props}, ref) => {


오 ref를 사용할 때가 어떤 상황이 있나요?

pakxe · 2025-01-21T03:33:29Z

client/src/components/Design/components/Checkbox/Checkbox.tsx

+            {...props}
+          />
+        </div>
+        {right ?? null}


여기 널리쉬를 안쓰면 문제가 생기나요?

pakxe · 2025-01-21T03:33:57Z

client/src/components/Design/components/Checkbox/Checkbox.tsx

+      onChange?.(e);
+    };
+
+    const handleKeyDown = (e: React.KeyboardEvent) => {


접근성까지...! 굿굿스입니다 👍

soi-ha

망가졌던 체크박스를 수정한 것도 모자라 더 멋지게 디벨롭해주셔서 감사합니다!
토다리의 공용으로 사용하는 컴포넌트를 제작할 때 뇌가 너무 탐나네요..

soi-ha · 2025-01-21T05:32:18Z

client/src/components/Design/components/Checkbox/Checkbox.tsx

+    const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
+      if (!isControlled) {
+        setInternalChecked(e.target.checked);
+      }


나중을 위해 비제어까지 열어서 구현하신게 정말..👍

soi-ha · 2025-01-21T05:34:46Z

client/src/components/Design/components/Checkbox/Checkbox.tsx

+            onChange={handleChange}
+            disabled={disabled}
+            css={invisibleInputStyle}
+            aria-label="체크박스"


어떤 체크박스인지 aria-label에 넣어주면 더 좋을 것 같아요!

예를 들어, [x] 소하
라는 체크박스가 존재할 경우, aria-labe이 '소하 체크박스'라고 읽을 수 있게요!

스토리북에서 확인했을 때는 '소하 체크박스' 이런식으로 잘 읽히네요! 대신에 '체크박스 소하 체크박스' 이런식으로 두번 읽히네욤!
아, 그리고 스토리북이랑 실제 서비스에 사용될때랑 스크린리더가 읽는게 조금 다른 것 같아서 서비스에서도 잘 되는지 확인해봐야 할 것 같습니다!

soi-ha

checkbox 사용이 불가능한 에러가 존재해서 RC드려요! 확인 부탁드려용~

soi-ha · 2025-01-21T05:55:15Z

client/src/components/Design/components/Checkbox/Checkbox.tsx

+
+    return (
+      <label css={checkboxStyle} role="checkbox" aria-checked={checked}>
+        <div css={boxStyle({theme, checked, disabled})} aria-hidden="true" tabIndex={0} onKeyDown={handleKeyDown}>


checkbox가 잘 작동하는지 확인하기 위해서 작업해주신 branch를 받아서 테스트를 해봤는데 checkbox를 사용할 수가 없네요! 이유는 aria-hidden 때문인 것 같습니당.

스크린리더를 사용하지 않아도 checkbox 사용이 불가능하고 스크린리더를 사용해도 사용이 불가능하더라구요! 스토리북에서는 이런 에러가 발생하진 않았어서 모르셨을 것 같아요!

checkbox_._._aria_hidden.mov

fix: Checkbox 변경

12ce466

Todari added 🖥️ FE Frontend ⚙️ feat feature 🚨 bug bug labels Jan 19, 2025

Todari added this to the v3.1.4 milestone Jan 19, 2025

Todari requested review from pakxe, soi-ha and jinhokim98 January 19, 2025 19:10

Todari self-assigned this Jan 19, 2025

Todari added 2 commits January 20, 2025 05:04

feat: 키보드 접근성 기능 추가

cb50c44

style: lint 적용

a5519c6

Todari added 2 commits January 20, 2025 11:04

fix: Checkbox component 수정

6fbcde7

fix: 변경된 checkbox 적용

6ec3919

jinhokim98 approved these changes Jan 20, 2025

View reviewed changes

fix: CheckboxProps interface에서 type prop 제거

e93b2bd

Todari added 3 commits January 20, 2025 13:31

fix: disabled 상태 추가

026d567

docs: storybook 수정

4acb7dc

style: 사용하지 않는 코드 제거

e30774a

pakxe approved these changes Jan 21, 2025

View reviewed changes

soi-ha approved these changes Jan 21, 2025

View reviewed changes

soi-ha requested changes Jan 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FE] Checkbox component 변경 #960

[FE] Checkbox component 변경 #960

Todari commented Jan 19, 2025

github-actions bot commented Jan 19, 2025

github-actions bot commented Jan 19, 2025

github-actions bot commented Jan 20, 2025

jinhokim98 left a comment

jinhokim98 Jan 20, 2025

Todari Jan 20, 2025

jinhokim98 Jan 20, 2025

jinhokim98 Jan 20, 2025

jinhokim98 Jan 20, 2025

Todari Jan 20, 2025

jinhokim98 Jan 20, 2025 •

edited

Loading

jinhokim98 Jan 20, 2025

jinhokim98 Jan 20, 2025

github-actions bot commented Jan 20, 2025

github-actions bot commented Jan 20, 2025

pakxe left a comment

pakxe Jan 21, 2025

pakxe Jan 21, 2025

pakxe Jan 21, 2025

soi-ha left a comment

soi-ha Jan 21, 2025

soi-ha Jan 21, 2025

soi-ha Jan 21, 2025

soi-ha left a comment

soi-ha Jan 21, 2025

[FE] Checkbox component 변경 #960

Are you sure you want to change the base?

[FE] Checkbox component 변경 #960

Conversation

Todari commented Jan 19, 2025

issue

구현 목적

구현 사항

중점적으로 리뷰받고 싶은 부분(선택)

논의하고 싶은 부분(선택)

🫡 참고사항

github-actions bot commented Jan 19, 2025

github-actions bot commented Jan 19, 2025

github-actions bot commented Jan 20, 2025

jinhokim98 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jinhokim98 Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 20, 2025

github-actions bot commented Jan 20, 2025

pakxe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soi-ha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soi-ha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jinhokim98 Jan 20, 2025 •

edited

Loading