modify default value of Button's disabled props to follow loading props

[jintak0401]

Self Checklist

• I wrote a PR title in English and added an appropriate label to the PR.
• I wrote the commit message in English and to follow the Conventional Commits specification.
• I added the changeset about the changes that needed to be released. (or didn't have to)
• I wrote or updated documentation related to the changes. (or didn't have to)
• I wrote or updated tests related to the changes. (or didn't have to)
• I tested the changes in various browsers. (or didn't have to)
  • Windows: Chrome, Edge, (Optional) Firefox
  • macOS: Chrome, Edge, Safari, (Optional) Firefox

Related Issue

#2299

Summary

• Button 의 disabled 의 기본값을 false 에서 loading 을 따라가도록 수정합니다.

Details

• loading: true, disabled: undefined -> disabled: true
• loading: true, disabled: false -> disabled: false

Nov-28-2024.22-23-21.mp4

Breaking change? (Yes/No)

No

References

[changeset-bot]

🦋 Changeset detected

Latest commit: cda3bac

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@channel.io/bezier-react	Patch
bezier-figma-plugin	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[channeltalk]

https://desk.channel.io/#/channels/1/team_chats/groups/1237/67486f07a48a9df98387

[jintak0401]

이에 대한 설명을 적거나 해야할 곳이 있을까요?

[codecov]

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 82.28%. Comparing base (1761705) to head (cda3bac).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
...bezier-react/src/components/AlphaButton/Button.tsx	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2510      +/-   ##
==========================================
- Coverage   82.31%   82.28%   -0.03%     
==========================================
  Files         143      143              
  Lines        2985     2987       +2     
  Branches      919      925       +6     
==========================================
+ Hits         2457     2458       +1     
- Misses        497      498       +1     
  Partials       31       31              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Chromatic Report

🚀 Congratulations! Your build was successful!

• Storybook Preview
• Chromatic Build

[sungik-choi]

loading = true, disabled = false 일 때는 disabled 상태가 아니게 되는데, loading 상태일 때는 항상 disabled 상태가 되는 게 기획 의도라고 생각했어요. 그래서 오버라이드 가능한 매개변수 기본값으로 처리하기보다 loading 상태라면 disabled 상태가 오버라이드 불가능하도록 처리하면 좋을 거 같습니다.

[jintak0401]

저는 loading = true, disabled = false 일 때에는 disabled 가 안되게 하는게 의도였습니다. 어떠한 경우일지는 모르겠지만, 의도적으로 이런 props 를 넣어줬다면 loading 스피너는 돌되 disabled는 안되게 만드는 것이 필요한 상황일 거라고 생각했거든요.

[sungik-choi]

베지어 컴포넌트 스펙에 없는, 아마도 존재하지 않을 케이스에 대해서 선택지를 열어놓는 게 좋은지 잘 모르겠어요. 물론 style과 className 속성같이 불가피한 경우에 사용처에서 오버라이드하는 속성들이 있긴 하지만, 왠만하면 미리 특수한 케이스를 위해서 선택지를 열어둘 필요는 없다고 생각해요. 또한 선택지를 좁혀놓고 늘리는 건 스펙 확장이기에 Breaking change가 아니지만, 선택지를 늘려놓고 좁히는 건 Breaking change라서 경우에 따라 이후 대응하기 까다로운 면도 있다고 생각합니다.

이에 대한 설명을 적거나 해야할 곳이 있을까요?

관련 이슈가 있어서 PR 설명에 연결해주시면 될 거 같아요 (#2299)

[yangwooseong]

Button.types.ts 에 있는 jsdoc 에 추가해도 좋을 것 같습니다.
DisableProps 를 extends 하면서 disabled 에 대한 jsdoc 만 삽입하는 방법은 없는 것 같아서, loading 설명에 추가해도 될 것 같아요.

머지 되면 간단하게 공지도 하면 좋을 것 같습니다!

[jintak0401]

선택지를 좁혀놓고 늘리는 건 스펙 확장이기에 Breaking change가 아니지만, 선택지를 늘려놓고 좁히는 건 Breaking change라서 경우에 따라 이후 대응하기 까다로운 면도 있다고 생각합니다.

@sungik-choi 이 말에 공감이 많이 가네요. 그러면 말씀해주신대로 수정하도록 하겠습니다.

@yangwooseong 🫡

[jintak0401]

• f1def31: Button 에 코멘트 반영
• 16a1ec6: AlphaButton 도 loading props 따라가도록 수정
• cda3bac: Button.types 에 jsdoc 설명 추가

[yangwooseong]

AlphaButton 이외에도 AlphaIconButton, AlphaFloatingButton, AlphaFloatingIconButton 까지 적용 부탁드릴게요..!

[jintak0401]

Button이 되게 많군요? 🤔