Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FE] feat: 공통 컴포넌트인 체크박스와 체크박스 아이템 제작 #240

Merged
merged 3 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions frontend/src/assets/checked.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions frontend/src/assets/unchecked.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
40 changes: 40 additions & 0 deletions frontend/src/components/common/Checkbox/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { ChangeEvent } from 'react';

import CheckedIcon from '@/assets/checked.svg';
import UncheckedIcon from '@/assets/unchecked.svg';

import * as S from './styles';

// NOTE: 공통 컴포넌트에서 이 스타일 속성을 계속 쓰는 것 같은데 이걸 아예 공통 타입으로 빼버릴지 고민
export interface CheckboxStyleProps {
$style?: React.CSSProperties;
}
Comment on lines +8 to +11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$style이 공통적으로 쓰이긴 하지만 다른 prop들과 함께 사용되는 경우도 있어요. 예를 들어 작업 중인 textarea에서는 isError와 함께 전달되고 있어요.

export interface TextareaStyleProps {
  isError: boolean;
  $style?: React.CSSProperties;
}

공통 타입으로의 분리가 오히려 코드의 복잡성을 늘릴 수도 있을 것 같아서, 이 부분은 조금 더 얘기를 해봐도 좋을 것 같아요.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗 이걸 PR 본문에도 적었어야 했는데 😅 캐치해줘서 고마워요! 항상 분리하는 게 능사는 아니죠 다음에 얘기해봐요~


interface CheckboxProps extends CheckboxStyleProps {
id: string;
isChecked: boolean;
onChange: (event: ChangeEvent<HTMLInputElement>) => void;
name?: string;
isDisabled?: boolean;
}

const Checkbox = ({ id, name, isChecked, isDisabled, onChange, $style }: CheckboxProps) => {
return (
<S.CheckboxContainer $style={$style}>
<S.CheckboxLabel>
<input
id={id}
name={name}
checked={isChecked}
onChange={onChange}
disabled={isDisabled}
aria-hidden={false}
type="checkbox"
/>
<img src={isChecked ? CheckedIcon : UncheckedIcon} alt="체크박스" />
</S.CheckboxLabel>
</S.CheckboxContainer>
);
};

export default Checkbox;
29 changes: 29 additions & 0 deletions frontend/src/components/common/Checkbox/styles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import styled from '@emotion/styled';

import { CheckboxStyleProps } from './index';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

중요하진 않지만! 경로 index 빼도 된다는 사실!
import { CheckboxStyleProps } from './';

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

자꾸 오타난 것 같아서 덧붙이게 되네요...🫠


export const CheckboxContainer = styled.div<CheckboxStyleProps>`
padding: 0;

background-color: transparent;
border: none;

input {
display: hidden;

position: absolute;
width: 0;
height: 0;
}
${({ $style }) => $style && { ...$style }};
`;

export const CheckboxLabel = styled.label`
cursor: pointer;
object-fit: contain;

img {
width: 100%;
height: 100%;
}
`;
33 changes: 33 additions & 0 deletions frontend/src/components/common/CheckboxItem/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { ChangeEvent } from 'react';

import Checkbox, { CheckboxStyleProps } from '../Checkbox';

import * as S from './styles';

interface CheckboxItemProps extends CheckboxStyleProps {
id: string;
label: string;
isChecked: boolean;
onChange: (event: ChangeEvent<HTMLInputElement>) => void;
name?: string;
isDisabled?: boolean;
}
const CheckboxItem = ({ id, name, label, isChecked, onChange, isDisabled = false, $style }: CheckboxItemProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const CheckboxItem = ({ id, name, label, isChecked, onChange, isDisabled = false, $style }: CheckboxItemProps) => {
const CheckboxItem = ({ label, ...rest }: CheckboxItemProps) => {
...
<Checkbox {...rest}/>

이렇게 Checkbox에게 넘겨주기 위해 받는 props들은 구조분해할당 이용해서 넘겨주면 제 경험상 Checkbox props가 변경되어도 props넘겨주는 부분은 변경하지 않아도 돼서 좋았어요

만약 나중에 리팩토링을 한다면 이 방법 추천해요

isDisabled의 기본값은 이걸 사용하는 Checkbox에서 설정하면 좋고요

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 js로 rest를 접했을 때는 사용한 속성들을 너무 축약하는 느낌이라 잘 안 쓰게 됐는데 이제 어차피 ts를 쓰니까 인터페이스에서 어떤 속성을 썼는지 찾아볼 수 있겠네요! 꿀팁 감사합니다

return (
<S.CheckboxItem>
<S.CheckboxLabel>
<Checkbox
id={id}
name={name}
isChecked={isChecked}
onChange={onChange}
isDisabled={isDisabled}
$style={$style}
/>
{label}
</S.CheckboxLabel>
</S.CheckboxItem>
);
};

export default CheckboxItem;
10 changes: 10 additions & 0 deletions frontend/src/components/common/CheckboxItem/styles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import styled from '@emotion/styled';

export const CheckboxItem = styled.div``;

export const CheckboxLabel = styled.label`
font-weight: ${({ theme }) => theme.fontWeight.bold};
display: flex;
align-items: center;
gap: 0.7rem;
`;