[REFACTOR] 버튼 공통 컴포넌트 수정 #59

useon · 2024-07-23T18:47:26Z

Issue Number

#49

As-Is

공통 버튼 컴포넌트의 재사용성 한계

To-Be

자주 사용하는 버튼 UI를 간단하게 정의하여 사용할 수 있도록 수정

공통 버튼 컴포넌트 size, radius, fontSize, style prop을 추가하고 'small', 'medium', 'large' 옵션 부여
디자인 토큰으로 borderRadius, typography 추가
부여된 옵션에 따른 조건부 스타일링 적용
공통 버튼 컴포넌트 스토리북 테스트 추가

Check List

테스트가 전부 통과되었나요?
모든 commit이 push 되었나요?
merge할 branch를 확인했나요?
Assignee를 지정했나요?
Label을 지정했나요?

Test Screenshot

스토리북으로 확인해보시면 옵션에 따른 버튼의 변화를 바로 확인하실 수 있습니다 ^.^

(Optional) Additional Description

스토리북때문에 지금 통과가 안 되는 것 같아요 🥲

novice0840 · 2024-07-23T23:36:18Z

CI가 실패한 것 같습니다. 확인 부탁해용

rbgksqkr · 2024-07-24T00:05:44Z

ci가 실패했는데, 지운 active 속성을 사용해서 그런 것 같아요! 확인부탁드립니당

rbgksqkr

천재만재 썬뎃 고생하셨습니다아~!~ 몇가지 코멘트 남겼으니 확인부탁드려용

rbgksqkr · 2024-07-24T04:05:30Z

frontend/src/components/common/Button/Button.styled.ts

+export const buttonLayout = ({ disabled, size, radius, fontSize }: ButtonLayoutProps) => css`
+  display: flex;
+  justify-content: center;
+  width: ${size === 'small' ? '6.9rem' : size === 'medium' ? '12rem' : '32rem'};


L5 - 참고 의견

4px 단위로 수정하면 좋을 것 같아요!

와우 ! 저희가 4px 기준으로 맞추기로 했는데 6.9 .. ? 도대체 무엇 ? 수면 이슈로 인하여 그런 듯 .. 당장 6.8로 수정하도록 하겠습니다 꼼꼼한 마루 짱 ! 👏

rbgksqkr · 2024-07-24T04:08:58Z

frontend/src/styles/Theme.ts

+  },
+  placeholder: {
+    fontSize: '1.2rem',
+    fontWeight: '500',


L4 - 변경 제안

기본 폰트 weight는 400 이라서 강조하는 의도가 아니라면 font-weight를 400으로 수정해도 괜찮을 것 같아요!

좋아요우 바로 반영하겠습니다 😊

rbgksqkr · 2024-07-24T04:15:26Z

frontend/src/components/common/Button/Button.styled.ts


  font-weight: bold;
-  font-size: 2rem;
-  border-radius: 2.4rem;
+  font-size: ${fontSize === 'small'


L4 - 변경 제안

삼항 연산자가 많아서 조금 헷갈리는 것 같아요! 그래서 mapper 방식을 사용해봐도 좋을 것 같아요!

const getSize = (size) => { if (size === 'medium'){ return css` font-size: Theme.typography.headline2.fontSize; border-raduis: Theme.borderRadius.radius20; ` } ... }

안 그래도 저도 .. 마음에 무지하게 걸려서 리팩토링을 바로 해버렸답니다 . 확인부탁드려요 ! ✨

rbgksqkr · 2024-07-24T04:17:25Z

frontend/src/components/common/Button/Button.tsx

+  onClick,
+  disabled = false,
+  size = 'large',
+  radius = 'none',


L4 - 변경 제안

raddius에 none 이라는 타입은 어떻게 쓰이느 건가요?? 속성 없이 안적으면 적용 안되는 걸로 하는 건 어떨까요?

default로 그러면 0을 달아줘야 해서 그것마저 같은 형식으로 관리해서 통일감을 줄까 하는 생각이었는데, 꼭 필요한 것은 아니라고 생각합니다 반영 하겠숨니다👏

novice0840

공통 스타일 로직 분리하느라 수고 많았어요~~~

novice0840 · 2024-07-24T05:47:55Z

frontend/src/components/GameResult/GameResult.tsx

@@ -16,7 +16,7 @@ const GameResult = () => {
      <div css={gameResultTitleWrapper}>
        <h1 css={gameResultTitle}>게임 결과</h1>
      </div>
-      <Button text="확인" active={true} onClick={goToHome} />
+      <Button text="확인" onClick={goToHome} />


L5 - 참고 의견

active 옵션을 지웠는데 그럼 button의 색은 어떤식으로 구분하는 게 좋다고 생각하시나요?

버튼이 button의 disabled는 default가 false이니 true가 되는 경우에만 버튼 색상이 연해지도록 설정하였습니다 😊

오 저도 잘 수정된 것 같아요! 그냥 버튼인데 active: true를 설정해줘야 되는게 어색하더라구요 굿굿

novice0840 · 2024-07-24T05:48:48Z

frontend/src/components/SelectContainer/SelectContainer.tsx

@@ -39,7 +39,7 @@ const SelectContainer = () => {
              handleSelectOption={handleSelectOption}
            />
          </section>
-          <Button text="선택" active={Boolean(selectedId)} onClick={goToRoundResult} />
+          <Button text="선택" disabled={Boolean(selectedId)} onClick={goToRoundResult} />


L5 - 참고 의견

active에 따라 색을 구분하다가 disabled로 구분하도록 바뀐 건가요?

버튼의 색상은 기본적으로 peanut400 이고 disabled 속성이 true인 경우는 peanut300 으로 설정했습니다. 추가로 다른 버튼 색상으로 정의하고 싶은 경우는 style 속성을 통해 backgroundColor: black 이런 식으로 커스텀할 수 있습니다 👏

novice0840 · 2024-07-24T05:50:55Z

frontend/src/styles/Theme.ts

+  slogan: {
+    fontSize: '2.8rem',
+    fontWeight: 'bold',
+  },


L5 - 참고 의견

slogan은 주로 어디에 사용하실 생각이신가요?

저희 피그마에 정의된 타이포그래피를 기준으로 Theme에 디자인 토큰을 정의했습니다. slogan 같은 경우 처음 메인 화면에서 서비스 이름을 띄워줄 때 사용할 것 같아요 !

novice0840 · 2024-07-24T05:51:51Z

frontend/src/components/common/Button/Button.tsx

+interface ButtonProps extends ButtonHTMLAttributes<HTMLButtonElement> {
+  text: string;
  onClick: () => void;
+  disabled?: boolean;
+  style?: React.CSSProperties;
+  size?: 'small' | 'medium' | 'large';
+  radius?: 'none' | 'small' | 'medium' | 'large';
+  fontSize?: 'small' | 'medium' | 'large';
 }


L5 - 참고 의견

이제 더 다양하게 커스텀이 가능하겠네요!

어떻게 하면 자주 쓰는 버튼들을 정의해서 사용할 수 있을까 고민해보았습니다 사용해 보면서 필요하다면 더 수정해 보아요 ~! 🚀✨

novice0840 · 2024-07-24T05:53:22Z

frontend/src/components/common/Button/Button.styled.ts


-  background-color: ${active ? Theme.color.peanut400 : Theme.color.peanut300};
+const getSizeStyles = (size: 'small' | 'medium' | 'large' | undefined) => {


L4 - 변경 제안

or undefiend 보다 size를 size?로 바꾸면 더 간결해질 것 같습니다!

오 ! 좋네요. 좋은 방법 알아갑니다 폼에 ✨

novice0840 · 2024-07-24T05:54:38Z

frontend/src/components/common/Button/Button.styled.ts

+const getFontSize = (fontSize: 'small' | 'medium' | 'large' | undefined) => {
+  switch (fontSize) {
+    case 'small':
+      return Theme.typography.caption.fontSize;
+    case 'medium':
+      return Theme.typography.headline2.fontSize;
+    case 'large':
+    default:
+      return Theme.typography.headline1.fontSize;
+  }
+};


L4 - 변경 제안

getFontSize 같은 함수는 Button에 국한되지 않고 여러 곳에서 사용할 수 있으니까 공통 로직으로 만들면 더 좋을 것 같습니다.

오오 정말 좋은 의견이네요! 다른 로직에서도 충분히 사용이 가능하겠군요. 반영해 보겠습니다 👏

…별도 파일로 분리 #49

rbgksqkr · 2024-07-24T07:24:27Z

frontend/src/components/GameResult/GameResult.tsx

@@ -16,7 +16,7 @@ const GameResult = () => {
      <div css={gameResultTitleWrapper}>
        <h1 css={gameResultTitle}>게임 결과</h1>
      </div>
-      <Button text="확인" active={true} onClick={goToHome} />
+      <Button text="확인" onClick={goToHome} />


오 저도 잘 수정된 것 같아요! 그냥 버튼인데 active: true를 설정해줘야 되는게 어색하더라구요 굿굿

useon added 3 commits July 24, 2024 03:30

feat: borderRadius, typography 디자인 토큰 추가 #49

6e828d2

refactor: 버튼 컴포넌트에 size, radius, fontSize prop 추가 및 조건부 스타일링 적용 #49

ff2f09b

test: 버튼 컴포넌트 스토리북 추가 #49

fcac114

useon added ♻️ refactor 리팩토링 🎨 design 디자인 🫧 FE front end labels Jul 23, 2024

useon added this to the FE Sprint2 milestone Jul 23, 2024

useon requested review from rbgksqkr and novice0840 July 23, 2024 18:47

useon self-assigned this Jul 23, 2024

useon linked an issue Jul 23, 2024 that may be closed by this pull request

[REFACTOR] 버튼 공통 컴포넌트 수정 #49

Closed

2 tasks

useon added 2 commits July 24, 2024 13:15

refactor: 조건부 스타일링 로직 개선 #49

df8e246

refactor: 공통 컴포넌트 수정에 따라 prop 수정 #49

272112c

rbgksqkr reviewed Jul 24, 2024

View reviewed changes

useon added 4 commits July 24, 2024 14:24

refactor: Button 사이즈 단위를 4px 단위로 수정 #49

13c2cd5

refactor: 강조가 아닌 경우 fontWeight를 400으로 수정 #49

b19b4ad

refactor: 필수 속성이 아닌 경우 속성값을 정하지 않도록 수정 #49

3d2ddf4

refactor: 속성이 입력되지 않으면 버튼 스타일에 정의된 기본 속성이 적용되도록 수정 및 radius의 none 삭제 #49

34314ea

novice0840 approved these changes Jul 24, 2024

View reviewed changes

useon added 4 commits July 24, 2024 15:55

chore: push시 rebase하는 설정 삭제 #49

786dd27

refactor: 매개변수의 선택적 타입 선언으로 코드 개선 #49

2f7c6b3

refactor: 스타일 유틸리티 함수(getSizeStyles, getFontSize, getBorderRadius)들을 …

52ef8e4

…별도 파일로 분리 #49

merge: develop 브랜치 충돌 제거 #49

4a88a48

useon force-pushed the refactor/#49 branch from 1686b18 to 4a88a48 Compare July 24, 2024 08:45

Merge branch 'develop' into refactor/#49

8113684

rbgksqkr approved these changes Jul 24, 2024

View reviewed changes

refactor: 버튼 active prop 삭제에 따른 수정 #49

d9046b6

useon merged commit 98b6667 into develop Jul 24, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[REFACTOR] 버튼 공통 컴포넌트 수정 #59

[REFACTOR] 버튼 공통 컴포넌트 수정 #59

useon commented Jul 23, 2024 •

edited

Loading

novice0840 commented Jul 23, 2024

rbgksqkr commented Jul 24, 2024

rbgksqkr left a comment

rbgksqkr Jul 24, 2024

useon Jul 24, 2024

rbgksqkr Jul 24, 2024

useon Jul 24, 2024

rbgksqkr Jul 24, 2024

useon Jul 24, 2024

rbgksqkr Jul 24, 2024

useon Jul 24, 2024

novice0840 left a comment

novice0840 Jul 24, 2024

useon Jul 24, 2024

rbgksqkr Jul 24, 2024

novice0840 Jul 24, 2024

useon Jul 24, 2024

novice0840 Jul 24, 2024

useon Jul 24, 2024

novice0840 Jul 24, 2024

useon Jul 24, 2024

novice0840 Jul 24, 2024

useon Jul 24, 2024

novice0840 Jul 24, 2024

useon Jul 24, 2024

rbgksqkr Jul 24, 2024


		background-color: ${active ? Theme.color.peanut400 : Theme.color.peanut300};
		const getSizeStyles = (size: 'small' \| 'medium' \| 'large' \| undefined) => {

[REFACTOR] 버튼 공통 컴포넌트 수정 #59

[REFACTOR] 버튼 공통 컴포넌트 수정 #59

Conversation

useon commented Jul 23, 2024 • edited Loading

Issue Number

As-Is

To-Be

Check List

Test Screenshot

(Optional) Additional Description

novice0840 commented Jul 23, 2024

rbgksqkr commented Jul 24, 2024

rbgksqkr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

L5 - 참고 의견

Choose a reason for hiding this comment

Choose a reason for hiding this comment

L4 - 변경 제안

Choose a reason for hiding this comment

Choose a reason for hiding this comment

L4 - 변경 제안

Choose a reason for hiding this comment

Choose a reason for hiding this comment

L4 - 변경 제안

Choose a reason for hiding this comment

novice0840 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

L5 - 참고 의견

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

L5 - 참고 의견

Choose a reason for hiding this comment

Choose a reason for hiding this comment

L5 - 참고 의견

Choose a reason for hiding this comment

Choose a reason for hiding this comment

L5 - 참고 의견

Choose a reason for hiding this comment

Choose a reason for hiding this comment

L4 - 변경 제안

Choose a reason for hiding this comment

Choose a reason for hiding this comment

L4 - 변경 제안

Choose a reason for hiding this comment

Choose a reason for hiding this comment

useon commented Jul 23, 2024 •

edited

Loading