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

[Feature] SwitchGroup 컴포넌트 구현 #32

Closed
wants to merge 17 commits into from

Conversation

ghdtjgus76
Copy link
Collaborator

🎉 변경 사항

SwitchGroup 컴포넌트 구현했습니다.

🚩 관련 이슈

🙏 여기는 꼭 봐주세요!

Switch 폴더 안에 넣을까 하다가 지금 빌드 설정 생성하는 스크립트가 분리되어 있어야 되는 구조로 작동해서.. 일단은 SwitchGroup 폴더 하나 더 생성해서 코드는 올려놨습니다.
혹시나 Switch 폴더 내부에서 같이 관리하는게 낫다고 생각하면 말씀 주세요!
빌드 스크립트랑 같이 변경하겠습니다.

Copy link
Contributor

Copy link
Contributor

Copy link
Member

@hamo-o hamo-o left a comment

Choose a reason for hiding this comment

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

Switch 컴포넌트에 대해서 추가적으로 궁금한 점이 있는데요!!
disabled 되었을 때 focus는 가능한 것 같은데, 혹시 이유가 있을까요? 저도 요렇게 통일해야 하나 싶어서 여쭤봅니다.
++ disabled 시에는 cursor 스타일도 pointer를 없애거나, not-allowed로 바꾸면 어떨까 싶습니다!

packages/wow-ui/src/components/Switch/index.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Jun 2, 2024

@ghdtjgus76
Copy link
Collaborator Author

Switch 컴포넌트에 대해서 추가적으로 궁금한 점이 있는데요!! disabled 되었을 때 focus는 가능한 것 같은데, 혹시 이유가 있을까요? 저도 요렇게 통일해야 하나 싶어서 여쭤봅니다. ++ disabled 시에는 cursor 스타일도 pointer를 없애거나, not-allowed로 바꾸면 어떨까 싶습니다!

disabled 됐을 때 focus도 안 되는게 맞는데 제가 속성을 빼먹어서...
기존 input에는 aria-disabled 속성만 있었는데 focus 되지 않도록 disabled 속성을 추가해뒀습니다.
cursor 스타일도 default로 같이 변경해놨어요!

Copy link
Contributor

github-actions bot commented Jun 2, 2024

Copy link
Collaborator

@SeieunYoo SeieunYoo left a comment

Choose a reason for hiding this comment

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

수고 많으셨어용~~

packages/wow-ui/src/components/SwitchGroup/index.tsx Outdated Show resolved Hide resolved
packages/wow-ui/src/components/SwitchGroup/index.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Jun 2, 2024

Copy link
Contributor

github-actions bot commented Jun 2, 2024

Copy link
Member

@hamo-o hamo-o left a comment

Choose a reason for hiding this comment

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

굿굿 조씁니다 👍
아직 Switch 상태 변경 시 SwtichGroup 전체가 렌더링되는 문제는 해결되지 않은 것 같은데, 한번 확인 부탁드려요!!

Comment on lines +58 to +63
children: (
<>
<Switch text="Switch 1" />
<Switch isDisabled text="Switch 2" />
<Switch text="Switch 3" />
</>
Copy link
Member

Choose a reason for hiding this comment

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

image

p2;
children 관련해서 타입 에러가 발생합니다.

Suggested change
children: (
<>
<Switch text="Switch 1" />
<Switch isDisabled text="Switch 2" />
<Switch text="Switch 3" />
</>
children: [
<Switch text="Switch 1" />,
<Switch isDisabled text="Switch 2" />,
<Switch text="Switch 3" />,
],

export const ControlledState: Story = {
render: () => <ControlledSwitchGroup />,
args: {
children: null,
Copy link
Member

Choose a reason for hiding this comment

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

p2;
children 관련해서 타입 에러가 발생합니다. 관련 렌더링 코드를 삭제하고 바로 export하면 어떨까요?

Suggested change
children: null,
export const ControlledSwitchGroup = () => {
const [isCheckedState, setIsCheckedState] = useState<boolean[]>([
false,
true,
false,
]);
const handleChange = (index: number) => {
setIsCheckedState((prev) =>
prev.map((prevState, i) => (index === i ? !prevState : prevState))
);
};
return (
<SwitchGroup value={isCheckedState} onChange={handleChange}>
<Switch isDisabled text="Switch 1" />
<Switch text="Switch 2" />
<Switch text="Switch 3" />
</SwitchGroup>
);
};

@ghdtjgus76 ghdtjgus76 closed this Jun 10, 2024
@ghdtjgus76 ghdtjgus76 deleted the feature/switch-group-component branch June 10, 2024 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants