AlphaIcon component
#2566
AlphaIcon component
#2566
Conversation
🦋 Changeset detectedLatest commit: 125c175 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2566 +/- ##
==========================================
+ Coverage 81.86% 81.90% +0.04%
==========================================
Files 145 147 +2
Lines 2889 2913 +24
Branches 920 917 -3
==========================================
+ Hits 2365 2386 +21
- Misses 494 497 +3
Partials 30 30 ☔ View full report in Codecov by Sentry. |
Chromatic Report🚀 Congratulations! Your build was successful! |
sungik-choi
force-pushed
the
web-4647-2
branch
from
bbcd45b to
c4795fe
Compare
<!-- How to write a good PR title: - Follow [the Conventional Commits specification](https://www.conventionalcommits.org/en/v1.0.0/). - Give as much context as necessary and as little as possible - Prefix it with [WIP] while it’s a work in progress --> ## Self Checklist - [x] I wrote a PR title in **English** and added an appropriate **label** to the PR. - [x] I wrote the commit message in **English** and to follow [**the Conventional Commits specification**](https://www.conventionalcommits.org/en/v1.0.0/). - [x] I [added the **changeset**](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md) about the changes that needed to be released. (or didn't have to) - [x] I wrote or updated **documentation** related to the changes. (or didn't have to) - [x] I wrote or updated **tests** related to the changes. (or didn't have to) - [x] 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 <!-- Please link to issue if one exists --> <!-- Fixes #0000 --> ## Summary <!-- Please brief explanation of the changes made --> bezier-tokens에서 scss 파일을 내보냅니다. ## Details <!-- Please elaborate description of the changes --> - 일부 유틸 클래스를 만드는 CSS 모듈에서 토큰에 대한 종류를 모두 알고있어야하는 불편함이 있었습니다. 이번에 #2566 PR 작업을 진행하면서 동일한 작업이 반복되는걸 느껴 이를 함께 수정합니다. - bezier-tokens에서 [map-deep](https://styledictionary.com/reference/hooks/formats/predefined/#scssmap-deep) 포맷으로 scss output을 만들도록 합니다. 자바스크립트 케이스처럼 `index.scss` 를 만드는 과정을 추가했습니다. - bezier-token의 `sideEffects` 필드에 scss 파일을 추가합니다. - scss의 `pkg:` Importer 규칙에 따라 bezier-tokens의 conditional export field를 수정했습니다. bezier-react에선 storybook & build 과정 모두 상대 경로로 지정하는 게 잘 동작해서 pkg 프로토콜은 이 PR에서 사용하지 않았습니다 (시간이 좀 더 걸릴듯함) ### Breaking change? (Yes/No) <!-- If Yes, please describe the impact and migration path for users --> No - ev-inner, ev-base 에 대한 유틸 클래스가 추가로 생기지만 사용처 영향은 없습니다 (매우 미미한 스타일 시트 파일 크기 상승) ## References <!-- Please list any other resources or points the reviewer should be aware of --> - https://sass-lang.com/documentation/js-api/classes/nodepackageimporter/ - https://webpack.kr/guides/tree-shaking/
sungik-choi
force-pushed
the
web-4647-2
branch
2 times, most recently
from
025d6a2 to
3f3ef73
Compare
sungik-choi
force-pushed
the
web-4647-2
branch
from
3c61df9 to
cbdccfc
Compare
| @@ -49,5 +40,5 @@ | |||
| position: relative; | |||
| display: flex; | |||
| align-items: center; | |||
| height: var(--b-avatar-group-size); | |||
| height: 100%; | |||
There was a problem hiding this comment.
테스트 시 구체적으로 높이를 지정하지 않아도 잘 동작함
|
d89ac74 이 커밋은 없애는 게 맞겠죠? |
There was a problem hiding this comment.
FloatingIconButton 컴포넌트도 바꿔야할 것 같습니다
이 커밋은 이 PR에 없습니다..!
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Walkthrough이번 변경 사항은 새로운 Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant AlphaIcon
participant BezierIcon
App->>AlphaIcon: props(source, size, color, ...)
AlphaIcon->>BezierIcon: Render SVG with size & color
AlphaIcon->>AlphaIcon: 접근성 속성/스타일 계산
AlphaIcon-->>App: SVG 아이콘 요소 반환
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/bezier-react/src/components/AlphaAvatar/Avatar.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "bezier" to extend from. Please check that the name of the config is correct. The config "bezier" was referenced from the config file in "/packages/bezier-react/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. packages/bezier-react/src/components/AlphaAvatar/Avatar.types.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "bezier" to extend from. Please check that the name of the config is correct. The config "bezier" was referenced from the config file in "/packages/bezier-react/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. packages/bezier-react/src/components/AlphaAvatarGroup/AvatarGroup.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "bezier" to extend from. Please check that the name of the config is correct. The config "bezier" was referenced from the config file in "/packages/bezier-react/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
packages/bezier-react/src/types/alpha-tokens.ts (1)
80-83:SourceSize타입에 간단한 JSDoc 추가 고려타입의 의도가 분명하지만, 추후 유지보수를 위해 “아이콘·아바타 공통 소스 사이즈 토큰”임을 짧게 주석으로 명시해 두면 팀원이 빠르게 이해할 수 있습니다.
+/** + * `source-size` 글로벌 토큰에서 접두사(`alpha-source-size-`)를 제거해 + * 숫자형 문자열(예: "20")만 추출한 타입. + */ export type SourceSize = RemovePrefix< 'alpha-source-size', keyof GlobalToken['source-size'] >.changeset/many-tips-float.md (1)
5-5: Changeset 본문이 너무 간략함변경 사항이 토큰 파일 추가임을 명시했지만, 범위(파일 경로·사용처)까지 한 줄 정도 더 적어두면 릴리스 노트 가독성이 좋아집니다.
.changeset/stupid-countries-yawn.md (1)
5-5: Changeset 상세 설명 보강 제안
AlphaIcon이 어떤 문제를 해결하고 주요 기능이 무엇인지 한두 문장 추가하면 외부 사용자에게 더 친절한 릴리스 노트가 됩니다.packages/bezier-react/src/components/AlphaIcon/Icon.test.tsx (1)
17-17: 접근성 테스트의 getByRole 사용법 개선 필요기본 상태에서
getByRole('img', { hidden: true })를 사용하고 있지만,aria-hidden="true"인 요소는 일반적으로getByRole로 직접 쿼리하지 않습니다. 대신 다른 방법을 고려해보세요.- expect(screen.getByRole('img', { hidden: true })).toBeInTheDocument() + const iconElement = screen.getByTestId('icon') // 또는 다른 선택자 사용 + expect(iconElement).toBeInTheDocument()또는 Icon 컴포넌트에 data-testid를 추가하는 방법을 고려해보세요.
Also applies to: 23-27
packages/bezier-react/src/components/AlphaIconButton/IconButton.tsx (1)
73-78: 테스트 커버리지 개선 필요정적 분석 도구에서 이 라인이 테스트로 커버되지 않았다고 보고했습니다. 새로운 AlphaIcon 컴포넌트를 사용하는 부분에 대한 테스트 추가를 고려해보세요.
IconButton 컴포넌트에서 AlphaIcon을 사용하는 시나리오에 대한 테스트를 추가하는 것을 권장합니다:
it('should render AlphaIcon when content is a bezier icon', () => { render(<IconButton content={SomeIcon} />) expect(screen.getByRole('img', { hidden: true })).toBeInTheDocument() })packages/bezier-react/src/utils/style.ts (1)
64-71: TODO로 표시된 미완성 함수가 있습니다.
alphaTokenCssVar함수가 TODO로 표시되어 있고 실제 구현이 없습니다. 이 함수가 현재 PR에서 사용되지 않는다면 괜찮지만, 사용될 예정이라면 구현을 완료해야 합니다.이 함수의 구현이 필요하시면 도움을 드릴 수 있습니다.
packages/bezier-react/src/components/AlphaIcon/Icon.types.ts (1)
30-32: 컬러 타입 확장 고려
color를 디자인 토큰으로 제한하는 것은 일관성 측면에서는 좋지만, 아이콘 내에서currentColor, CSS 변수 등 표준 CSS 값을 허용하지 않으면 유연성이 떨어질 수 있습니다. 토큰을 기본값으로 두되string도 허용하는 방향을 검토해 주세요.- color?: FunctionalColor | SemanticColor + color?: FunctionalColor | SemanticColor | string
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/bezier-react/src/components/AlphaAvatarGroup/__snapshots__/AvatarGroup.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (22)
.changeset/many-tips-float.md(1 hunks).changeset/stupid-countries-yawn.md(1 hunks)packages/bezier-react/src/components/AlphaAvatar/Avatar.module.scss(0 hunks)packages/bezier-react/src/components/AlphaAvatar/Avatar.tsx(2 hunks)packages/bezier-react/src/components/AlphaAvatar/Avatar.types.ts(2 hunks)packages/bezier-react/src/components/AlphaAvatarGroup/AvatarGroup.module.scss(1 hunks)packages/bezier-react/src/components/AlphaAvatarGroup/AvatarGroup.tsx(3 hunks)packages/bezier-react/src/components/AlphaIcon/AlphaIcon.stories.tsx(1 hunks)packages/bezier-react/src/components/AlphaIcon/Icon.module.scss(1 hunks)packages/bezier-react/src/components/AlphaIcon/Icon.test.tsx(1 hunks)packages/bezier-react/src/components/AlphaIcon/Icon.tsx(1 hunks)packages/bezier-react/src/components/AlphaIcon/Icon.types.ts(1 hunks)packages/bezier-react/src/components/AlphaIcon/index.ts(1 hunks)packages/bezier-react/src/components/AlphaIconButton/IconButton.module.scss(2 hunks)packages/bezier-react/src/components/AlphaIconButton/IconButton.tsx(3 hunks)packages/bezier-react/src/index.ts(1 hunks)packages/bezier-react/src/styles/components/alpha-source-size.module.scss(1 hunks)packages/bezier-react/src/types/alpha-props-helpers.ts(1 hunks)packages/bezier-react/src/types/alpha-tokens.ts(1 hunks)packages/bezier-react/src/utils/style.ts(2 hunks)packages/bezier-tokens/src/alpha/global/source-size.json(1 hunks)packages/bezier-vscode/src/server.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/bezier-react/src/components/AlphaAvatar/Avatar.module.scss
🧰 Additional context used
🧬 Code Graph Analysis (10)
packages/bezier-react/src/types/alpha-tokens.ts (2)
packages/bezier-react/src/types/utils.ts (1)
RemovePrefix(1-4)packages/bezier-react/src/types/tokens.ts (1)
GlobalToken(12-12)
packages/bezier-react/src/components/AlphaAvatar/Avatar.tsx (1)
packages/bezier-react/src/types/alpha-props-helpers.ts (1)
getSourceSizeClassName(8-10)
packages/bezier-react/src/components/AlphaIcon/Icon.test.tsx (1)
packages/bezier-react/src/utils/test.tsx (1)
render(16-25)
packages/bezier-react/src/types/alpha-props-helpers.ts (1)
packages/bezier-react/src/types/alpha-tokens.ts (1)
SourceSize(80-83)
packages/bezier-react/src/components/AlphaIconButton/IconButton.tsx (2)
packages/bezier-react/src/components/AlphaButton/Button.types.ts (1)
ButtonSize(26-26)packages/bezier-react/src/types/alpha-props-helpers.ts (1)
getSourceSizeClassName(8-10)
packages/bezier-react/src/components/AlphaAvatar/Avatar.types.ts (2)
packages/bezier-react/src/components/AlphaAvatar/index.ts (1)
AvatarSize(7-7)packages/bezier-react/src/types/alpha-tokens.ts (1)
SourceSize(80-83)
packages/bezier-react/src/components/AlphaIcon/AlphaIcon.stories.tsx (1)
packages/bezier-react/src/utils/string.ts (1)
camelCase(35-38)
packages/bezier-react/src/utils/style.ts (2)
packages/bezier-react/src/types/tokens.ts (1)
FlattenAllToken(19-19)packages/bezier-react/src/types/alpha-tokens.ts (1)
BaseSemanticColor(33-36)
packages/bezier-react/src/components/AlphaIcon/Icon.tsx (5)
packages/bezier-react/src/components/AlphaIcon/index.ts (2)
Icon(1-1)IconProps(3-3)packages/bezier-react/src/components/AlphaIcon/Icon.types.ts (1)
IconProps(50-54)packages/bezier-react/src/types/props-helpers.ts (2)
splitByMarginProps(21-41)getMarginStyles(133-152)packages/bezier-react/src/utils/style.ts (1)
alphaColorTokenCssVar(73-77)packages/bezier-react/src/types/alpha-props-helpers.ts (1)
getSourceSizeClassName(8-10)
packages/bezier-react/src/components/AlphaIcon/Icon.types.ts (3)
packages/bezier-react/src/components/AlphaIcon/index.ts (2)
IconSize(4-4)IconProps(3-3)packages/bezier-react/src/types/alpha-tokens.ts (2)
SourceSize(80-83)FunctionalColor(56-61)packages/bezier-react/src/types/props.ts (3)
BezierComponentProps(50-55)MarginProps(225-261)SizeProps(101-106)
🪛 GitHub Check: codecov/patch
packages/bezier-react/src/components/AlphaIconButton/IconButton.tsx
[warning] 73-73: packages/bezier-react/src/components/AlphaIconButton/IconButton.tsx#L73
Added line #L73 was not covered by tests
packages/bezier-react/src/utils/style.ts
[warning] 67-67: packages/bezier-react/src/utils/style.ts#L67
Added line #L67 was not covered by tests
[warning] 69-70: packages/bezier-react/src/utils/style.ts#L69-L70
Added lines #L69 - L70 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: codecov/patch
🔇 Additional comments (28)
packages/bezier-tokens/src/alpha/global/source-size.json (1)
1-55: 새 사이즈 토큰 정의 검토 완료값·정렬·단위 모두 일관성 있게 정의되어 있습니다. 디자인 시스템 전반에서 크기 혼선을 줄이는 데 도움이 될 것으로 보입니다.
packages/bezier-react/src/index.ts (1)
14-16:AlphaIcon내보내기 위치 적절ABC 순서와 유관 컴포넌트(
AlphaIconButton) 근접도를 모두 만족합니다. 추가 조정 필요 없어 보입니다.packages/bezier-vscode/src/server.ts (1)
93-95: 내부 토큰 타입 제외 처리가 적절합니다.새로운
source-size토큰이 내부 컴포넌트에서만 사용되므로 VSCode 자동완성에서 제외하는 것이 올바른 접근입니다. 주석도 명확하게 이유를 설명하고 있습니다.packages/bezier-react/src/components/AlphaAvatar/Avatar.tsx (2)
7-7: 헬퍼 함수 import가 적절합니다.새로운
getSourceSizeClassName헬퍼 함수를 import하여 크기 관련 클래스명 생성을 중앙화하는 것이 좋은 접근입니다.
135-135: 크기 클래스명 생성 방식 개선이 우수합니다.직접적인 CSS 클래스 문자열 구성(
styles['size-${size}']) 대신 헬퍼 함수를 사용하여 코드의 일관성과 유지보수성을 향상시켰습니다.packages/bezier-react/src/components/AlphaIcon/Icon.module.scss (1)
1-7: 아이콘 스타일링이 잘 구현되었습니다.CSS 커스텀 프로퍼티를 활용한 색상 테마 지원, flex 속성을 통한 크기 고정, 그리고 부드러운 색상 전환 효과가 적절히 구현되었습니다. 디자인 토큰(
--transition-s)을 사용한 것도 좋은 접근입니다.packages/bezier-react/src/components/AlphaIcon/index.ts (1)
1-5: 모듈 export 구조가 깔끔합니다.컴포넌트와 타입을 Alpha 접두사로 일관되게 재export하는 구조가 좋습니다. 네이밍 컨벤션도 기존 코드베이스와 일치합니다.
packages/bezier-react/src/types/alpha-props-helpers.ts (2)
3-6: ESLint 규칙 비활성화 처리가 적절합니다.CSS 모듈 경로 제한을 위한 ESLint 규칙을 명확한 주석과 함께 적절히 비활성화/재활성화하였습니다. TypeScript 플러그인 제한사항에 대한 설명도 유용합니다.
8-10: 헬퍼 함수 구현이 깔끔합니다.
SourceSize타입을 매개변수로 받아 CSS 클래스명을 반환하는 단순하고 효율적인 구현입니다. 함수명도 명확하고 타입 안전성이 보장됩니다.packages/bezier-react/src/components/AlphaAvatar/Avatar.types.ts (1)
1-1: SourceSize 타입 호환성 확인 필요Avatar.types.ts에서
AvatarSize가 명시적 문자열 리터럴에서 아래와 같이 중앙화된SourceSize타입으로 변경되었습니다:export type SourceSize = RemovePrefix< 'alpha-source-size', keyof GlobalToken['source-size'] >이 변경이 기존 아바타 크기들을 모두 포괄하는지 확인해야 합니다.
• 검토 대상 크기: '16', '20', '24', '30', '36', '42', '48', '72', '90', '120'
• 확인 방법:
@channel.io/bezier-tokens/alpha패키지의tokens.global['source-size']에 위 값들이 모두 키로 정의되어 있는지 확인RemovePrefix<'alpha-source-size', ...>적용 후 최종 유니온 타입에 누락된 값이 없는지 점검위 사항을 검증한 후, 호환성에 문제가 없다면 변경을 승인해 주세요.
packages/bezier-react/src/components/AlphaIcon/Icon.test.tsx (1)
29-36: aria-label 테스트 시나리오 우수함aria-label이 제공될 때 접근성이 올바르게 작동하는지 확인하는 테스트가 잘 구현되어 있습니다. 아이콘이 장식적에서 의미있는 요소로 전환되는 것을 적절히 검증합니다.
packages/bezier-react/src/styles/components/alpha-source-size.module.scss (1)
6-10: 토큰 기반 크기 클래스 생성 우수함SCSS 맵을 사용하여 동적으로 크기 클래스를 생성하는 접근법이 훌륭합니다. 이는 디자인 토큰과의 일관성을 보장하고 중복을 줄입니다.
:where()의사 클래스를 사용하여 특이성을 낮춘 것도 좋은 선택입니다.packages/bezier-react/src/components/AlphaAvatarGroup/AvatarGroup.module.scss (1)
43-43: 100% height 사용이 적절함이전 리뷰에서 언급된 바와 같이, 구체적인 높이를 지정하지 않아도 잘 동작하므로
100%로 설정하는 것이 더 유연하고 적절한 접근입니다. CSS 변수 의존성을 제거하고 단순화한 것도 좋은 개선사항입니다.packages/bezier-react/src/components/AlphaIconButton/IconButton.tsx (3)
8-8: 새로운 헬퍼 함수와 AlphaIcon 도입 우수함
getSourceSizeClassName헬퍼 함수와AlphaIcon컴포넌트 도입으로 크기 토큰 시스템이 잘 통합되었습니다. 코드의 일관성과 재사용성이 향상되었습니다.Also applies to: 10-10
21-26: getIconSize 함수 반환 타입 개선 우수함버튼 크기에서 아이콘 크기로의 매핑이 명확하고, 숫자 문자열로 반환하여 새로운 SourceSize 타입과 일치합니다. 매핑 로직도 적절합니다.
87-87: 크기 클래스명 생성 로직 일관성 확보
getSourceSizeClassName(getIconSize(size))를 사용하여 로더의 크기 클래스를 동적으로 생성하는 접근법이 새로운 토큰 시스템과 잘 통합되어 있습니다. 타입 안전성도 보장됩니다.packages/bezier-react/src/components/AlphaAvatarGroup/AvatarGroup.tsx (2)
40-41: 새로운 아바타 크기 지원이 일관성 있게 구현되었습니다.크기 10, 12, 60에 대한 아이콘 및 타이포그래피 매핑이 기존 패턴을 따라 올바르게 추가되었습니다. 작은 크기들(10, 12)은 기존의 작은 크기들과 동일한 스타일을 사용하고, 큰 크기(60)는 적절한 큰 스타일을 사용합니다.
Also applies to: 49-49, 61-62, 70-70
211-211: CSS 클래스 할당 단순화가 적절합니다.크기별 클래스를 제거하고 기본 클래스만 사용하는 변경사항은 중앙집중식 크기 토큰 시스템으로의 전환을 반영합니다.
packages/bezier-react/src/components/AlphaIcon/Icon.tsx (4)
29-30: 컴포넌트 구조가 React 모범 사례를 잘 따릅니다.
memo와forwardRef를 적절히 사용하여 성능 최적화와 ref 전달을 구현했습니다.
31-32: 마진 props 처리가 기존 패턴과 일관성 있습니다.
splitByMarginProps와getMarginStyles유틸리티를 사용하여 마진 관련 props를 깔끔하게 분리하고 처리합니다.Also applies to: 44-44
46-46: 접근성 구현이 우수합니다.
isDecorative로직이 올바르게 구현되어 있어,aria-label이 없고aria-hidden이 false가 아닌 경우 장식적 아이콘으로 처리합니다. 이는 스크린 리더 사용자에게 적절한 경험을 제공합니다.Also applies to: 52-53
56-56: 새로운 토큰 시스템과의 통합이 잘 되어 있습니다.
alphaColorTokenCssVar와getSourceSizeClassName을 사용하여 새로운 알파 토큰 시스템과 일관성 있게 통합되었습니다.Also applies to: 63-63
packages/bezier-react/src/components/AlphaIcon/AlphaIcon.stories.tsx (2)
31-32: pascalCase 함수 구현이 올바릅니다.
camelCase유틸리티를 사용하여 첫 글자를 대문자로 변환하는 깔끔한 구현입니다.
36-66: IconGallery 스토리가 포괄적이고 유용합니다.모든 아이콘을 그리드 레이아웃으로 표시하고 각 아이콘에 이름을 보여주는 툴팁을 제공하는 것은 개발자와 디자이너에게 매우 유용한 참조 자료가 될 것입니다.
packages/bezier-react/src/utils/style.ts (1)
73-77: alphaColorTokenCssVar 함수가 올바르게 구현되었습니다.알파 컬러 토큰에 대한 CSS 변수를 생성하는 로직이 적절하며, "alpha-color-" 접두사를 올바르게 추가합니다.
정적 분석 도구에서 테스트 커버리지 부족을 지적했습니다. 새로운 함수들에 대한 단위 테스트 추가를 고려해보세요.
packages/bezier-react/src/components/AlphaIconButton/IconButton.module.scss (2)
6-7: CSS 커스텀 속성을 사용한 리팩토링이 우수합니다.
--b-icon-button-padding커스텀 속성을 도입하여 패딩 제어를 중앙집중화한 것은 유지보수성을 크게 향상시킵니다. 각 크기별로 패딩 값을 설정하는 방식이 깔끔하고 일관성 있습니다.Also applies to: 10-10, 15-15, 20-20, 25-25, 30-30, 35-35
264-265: 로더 위치 지정 및 크기 조정이 개선되었습니다.패딩 변수를 사용한 위치 지정과
inherit을 사용한 크기 설정이 새로운 CSS 구조와 잘 맞습니다. 이전의 복잡한 크기 매핑 로직을 단순화한 것도 좋은 개선입니다.Also applies to: 271-274
packages/bezier-react/src/components/AlphaIcon/Icon.types.ts (1)
50-54: 타입 충돌 예방용Omit범위 재확인 요청
BezierComponentProps<'svg'>에는color,role,'aria-*'등이 이미 정의돼 있습니다.keyof IconOwnProps로만Omit하면margin·size관련 중복은 남지 않지만, 향후IconOwnProps에 신규 필드가 추가될 때마다 실수로 중복 정의될 여지가 있습니다.추가 필드가 생기더라도 자동으로 제외되도록
Omit<BezierComponentProps<'svg'>, keyof (IconOwnProps & MarginProps & SizeProps<IconSize>)>같은 패턴을 고려해 보는 것도 방법입니다.
| 'aria-hidden'?: boolean | ||
| /** | ||
| * ARIA role | ||
| * @default "img" | ||
| */ | ||
| role?: string |
There was a problem hiding this comment.
ARIA 타입이 React 정의와 일치하지 않습니다
'aria-hidden' 은 React 타입상 Booleanish(boolean | 'true' | 'false')이고, role 은 React.AriaRole 로 제한되어 있습니다. 현 구현처럼 boolean, string 으로만 한정하면 컴파일 단계에서 JSX 의 기본 타입과 충돌해 사용자가 문자열 "true" 등을 전달할 때 오류가 발생합니다.
+import type React from 'react'
...
- 'aria-hidden'?: boolean
+ 'aria-hidden'?: React.AriaAttributes['aria-hidden']
...
- role?: string
+ role?: React.AriaRole🤖 Prompt for AI Agents
In packages/bezier-react/src/components/AlphaIcon/Icon.types.ts between lines 42
and 47, update the types for 'aria-hidden' and 'role' to match React's
definitions. Change 'aria-hidden' type to React.Booleanish (boolean | 'true' |
'false') and restrict 'role' to React.AriaRole. This alignment prevents type
conflicts and allows valid JSX attribute values without compile errors.
Self Checklist
Related Issue
Summary
AlphaIcon컴포넌트 구현Details
AlphaIcon컴포넌트를 구현합니다. 사이즈 확장과 접근성 속성 추가 외 기존 아이콘과 다른 점은 없습니다.AlphaIconButton에서Icon대신AlphaIcon을 사용하도록 합니다.Breaking change? (Yes/No)
No
References
Summary by CodeRabbit
새로운 기능
버그 수정
스타일
문서화
테스트