Skip to content

Conversation

@almighty55555
Copy link
Collaborator

#️⃣연관된 이슈

ex) #이슈번호, #이슈번호
Closes #24

📝 PR 유형

해당하는 유형에 'x'로 체크해주세요.

  • 기능 추가 (Feature)
  • 버그 수정 (Bug Fix)
  • 코드 개선 (Refactoring)
  • 스타일 변경 (UI/UX)
  • 문서 작업 (Documentation)
  • 환경 설정 (Configuration)
  • 기타 (Other)

📝작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
버튼 컴포넌트를 생성했습니다. 자세한 내용은 위키 문서를 참고해주시면 됩니다.

스크린샷 (선택)

image

💬리뷰 요구사항(선택)

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

ex) 메서드 XXX의 이름을 더 잘 짓고 싶은데 혹시 좋은 명칭이 있을까요?

@netlify
Copy link

netlify bot commented Apr 24, 2025

Deploy Preview for thejulge1 ready!

Name Link
🔨 Latest commit 8141fc0
🔍 Latest deploy log https://app.netlify.com/sites/thejulge1/deploys/680cfff1da7747000870113c
😎 Deploy Preview https://deploy-preview-25--thejulge1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@jeonghwanJay jeonghwanJay left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 !!

@@ -0,0 +1,61 @@
import React from "react";

type ButtonVariant = "primary" | "white" | "disabled";
Copy link
Collaborator

Choose a reason for hiding this comment

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

disabled는 버튼의 상태를 나타내는 속성으로 봐도 좋을 것 같습니다 !
그래서 속성값으로 넣어도 좋을 것 같아요 !

export type ButtonVariant = 'primary' | 'white';

interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> {
  variant?: ButtonVariant;
  textSize?: Record<ButtonTextSize, string>;
  fullWidth?: boolean;
  disabled?: boolean; // disabled 속성 분리
}

Copy link
Collaborator

@cozy-ito cozy-ito left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다! 👍

Comment on lines 29 to 37
if (variant === "primary") {
variantClass =
"bg-[#EA3C12] text-white hover:bg-[#ca3f2a] active:bg-[#aa3523]";
} else if (variant === "white") {
variantClass =
"bg-white text-[#EA3C12] border border-[#EA3C12] hover:bg-[#fff5f3] active:bg-[#ffe5e0]";
} else if (variant === "disabled") {
variantClass = "bg-gray-40 text-white cursor-not-allowed";
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

variantClass의 조건부 스타일 적용도
textSizeClassMap처럼 별도로 분리하시지 않으신 이유가 있을까요? 🤔
분리하면 반복적인 If 구문을 제거할 수 있을 것 같아요! 😆

Suggested change
if (variant === "primary") {
variantClass =
"bg-[#EA3C12] text-white hover:bg-[#ca3f2a] active:bg-[#aa3523]";
} else if (variant === "white") {
variantClass =
"bg-white text-[#EA3C12] border border-[#EA3C12] hover:bg-[#fff5f3] active:bg-[#ffe5e0]";
} else if (variant === "disabled") {
variantClass = "bg-gray-40 text-white cursor-not-allowed";
}
const variantClassMap: Record<ButtonVariant, string> = {
primary: "bg-[#EA3C12] text-white hover:bg-[#ca3f2a] active:bg-[#aa3523]",
white:
"bg-white text-[#EA3C12] border border-[#EA3C12] hover:bg-[#fff5f3] active:bg-[#ffe5e0]",
disabled: "bg-gray-400 text-white cursor-not-allowed",
};
...
const finalClassName = [
  ...
  variantClassMap[variant]
 ...
] ...

@almighty55555 almighty55555 merged commit 641c1c8 into dev Apr 27, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature (기능 추가) 기능을 추가합니다.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

공통 컴포넌트 구현 (Button)

4 participants