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

[FEAT] Arrange 버튼 퍼블리싱 #48

Merged
merged 14 commits into from
Jul 8, 2024
Merged

[FEAT] Arrange 버튼 퍼블리싱 #48

merged 14 commits into from
Jul 8, 2024

Conversation

wrryu09
Copy link
Member

@wrryu09 wrryu09 commented Jul 7, 2024

작업 내용 🧑‍💻

버튼을 만들었습니다.

알게된 점 🚀

기록하며 개발하기!

svg 색상 바꾸기 currentColor 로만 바꿔왔었는데 지원언니가 알려준대로 path{} rect{} 등으로 하니까 적용 잘 되네요!! 배워갑니당!!

리뷰 요구사항 💬

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

disabled 상태에서도 호버랑 pressed 허용할건지?
컴포넌트 하나로 끝장내고 싶어서 이렇게 제작했는데 props를 좀 과하게 받나 싶기도 합니다,,
theme 바로 적용하려고 props 일관성을 좀 못 지킨 것 같은데 얘네 한번씩 봐주시면 감사하겠습니다!!

관련 이슈

close #43

스크린샷

Jul-08-2024 02-05-42

@wrryu09 wrryu09 self-assigned this Jul 7, 2024
@wrryu09 wrryu09 linked an issue Jul 7, 2024 that may be closed by this pull request
Copy link
Member

@seong-hui seong-hui left a comment

Choose a reason for hiding this comment

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

버튼이 굉장히 스타일 분기가 많았는데 이를 잘 나누신 것 같습니당 수고 많으셧습니당 👍

};
function ArrangeBtn({ type, mode, color, size }: ArrangeBtnType) {
const IconComponent = iconMap[type];
const StyledIcon = styled(IconComponent)<{ size: string; color: string; mode: string }>`
Copy link
Member

Choose a reason for hiding this comment

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

iconMap[type]을 선언하고 타입에 대한 스타일을 만들어서 내보내는 방식 굉장히 좋은 것 같아요 배워갑니당

@@ -0,0 +1,10 @@
import { css } from '@emotion/react';

export const smallSize = css`
Copy link
Member

Choose a reason for hiding this comment

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

이렇게 반복되는 스타일은 따로 빼는 거 좋으네용 짱짱

@@ -0,0 +1,6 @@
export interface ArrangeBtnType {
type: 'right' | 'left' | 'set' | 'calendar';
mode: 'DISABLED' | 'DEFAULT';
Copy link
Member

Choose a reason for hiding this comment

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

light라는 색상도 추가돼서 mode에 light도 넣으면 좋을 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

ArrangeBtn 들에는 LIGHT 가 없기도 하고 LIGHT 있는 쪽 버튼들 DEFAULT 가 다르게 사용된다고 해서 ... 다른 버튼에는 타입 새로 만들어야 할 것 같습니다!

Copy link
Member

@jeeminyi jeeminyi left a comment

Choose a reason for hiding this comment

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

코드를 너무 컴팩트하게 잘 짜신 것 같습니다.. 저는 미친 조건문의 굴레에 빠졌었는데 이렇게 효율적으로 구조를 잡을 수도 있군요! prop가 어차피 많아야 하는 컴포넌트였어서 좋은 구조인 것 같습니다 ㅎㅎ 많이 배우고 갑니다!! 수고하셨습니닷 👍💜

@wrryu09 wrryu09 merged commit 0564ff7 into develop Jul 8, 2024
2 checks passed
@wrryu09 wrryu09 deleted the feat/#43-arrange-btn branch July 8, 2024 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[FEAT] Arrange 버튼 퍼블리싱
3 participants