-
Notifications
You must be signed in to change notification settings - Fork 47
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
Use AlphaSpinner
in AlphaButton
component set
#2252
Use AlphaSpinner
in AlphaButton
component set
#2252
Conversation
🦋 Changeset detectedLatest commit: 3bf61ef The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2252 +/- ##
==========================================
+ Coverage 84.65% 84.87% +0.22%
==========================================
Files 136 136
Lines 2893 2883 -10
Branches 863 872 +9
==========================================
- Hits 2449 2447 -2
+ Misses 439 407 -32
- Partials 5 29 +24 ☔ View full report in Codecov by Sentry. |
Chromatic Report🚀 Congratulations! Your build was successful! |
@@ -39,14 +41,12 @@ | |||
|
|||
&:where(.size-s) { | |||
--b-spinner-size: 28px; | |||
--b-spinner-stroke-width: 4px; | |||
--b-spinner-stroke-dasharray: 40 9999; | |||
--b-spinner-stroke-width: calc(16 * 4px / 28); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/ 뒤에 있는 수치는 단위가 없는 값이어야 해서 --b-spinner-size
를 그대로 사용하지 못했습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scss 변수를 하나 더 두는 건 어떤가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
단위 없이 값만 같은 --b-spinner-size-value: 28
같은 값을 두는 걸 의미하신건가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--b-spinner-size: #{SIZE_S}px;
같은 식으로 변수를 하나 두고 재사용할 수 있지 않을까 했어요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
calc(1px * number) 를 사용하면 되네요! (311a11d)
1f7ce3c
to
6e62741
Compare
- add missed classname - change vector effect to default and fix storke property to constant
6e62741
to
3bf61ef
Compare
@@ -32,14 +32,14 @@ export const Spinner = forwardRef<HTMLSpanElement, SpinnerProps>( | |||
<circle | |||
cx="8" | |||
cy="8" | |||
r="7" | |||
r="6.5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -124,8 +124,6 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>( | |||
<AlphaSpinner | |||
className={styles.Spinner} | |||
variant="on-overlay" | |||
// NOTE: Spinner size will be overridden by Icon size | |||
size="s" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이걸 없애야 --b-spinner-stroke-width: 2px; 로 고정한 게 잘 적용됩니다. 참고로 AlphaSpinner는 디폴트값이 없습니다.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @channel.io/[email protected] ### Minor Changes - Update icons ([#2256](#2256)) by @yangwooseong Added - fast-forward-filled.svg - fast-rewind-filled.svg - paused.svg - play-filled.svg - skip-filled.svg - split-left-filled.svg - split-right-filled.svg Modified - fast-forward.svg - fast-rewind.svg - play.svg - skip.svg - split-left.svg - split-right.svg - view-off.svg Renamed - pause.svg -> paused-filled.svg ## @channel.io/[email protected] ### Patch Changes - Changes in `Button` component set ([#2252](#2252)) by @yangwooseong - use `AlphaSpinner` instead of `Spinner` component for loading state. - set size of `AlphaSpinner` to be `Icon` size. - Change `white-absolute` to `white` in `AlphaButton` and `AlphaIconButton` color. ([#2278](#2278)) by @yangwooseong - Changes with `Button` component ([#2253](#2253)) by @yangwooseong - Change `white` to `white-absolute` in `color` props. - Apply `white-alpha/transparent` token for `tertiary` button. - Updated dependencies - @channel.io/[email protected] ## @channel.io/[email protected] ### Patch Changes - Add `white-alpha/transparent` token. ([#2253](#2253)) by @yangwooseong - Change `innerShadow` value of `alpha-shadow-input-default` token to reference `color.shadow.medium` ([#2274](#2274)) by @yangwooseong ## [email protected] ### Patch Changes - Updated dependencies - @channel.io/[email protected] - @channel.io/[email protected] Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Self Checklist
Related Issue
Summary
AlphaButton
안에 있는 로더 컴포넌트를 새로 만든AlphaSpinner
로 교체합니다.Details
AlphaSpinner
의 사이즈는 아이콘 사이즈와 일치해야합니다. 따라서 버튼의 스타일 코드가 아이콘의 사이즈 맵(e.g. s - 20, m -24, ...) 을 알 필요가 있습니다. 이런 비슷한 경우가 SegmentedControl - Divider 컴포넌트에서 variables classname을 사용한 경우가 있었는데, 이 PR에서는 스타일 코드를 직접@include
로 가져와서 그 안에 있는 변수에 접근하는 방향으로 해결해봤습니다. 내보내는 쪽과 사용하는 쪽에서 불필요하게 variables classname을 사용하지 않아도 되는 장점이 있어서 이렇게 시도해봤습니다.Breaking change? (Yes/No)
References