-
-
Notifications
You must be signed in to change notification settings - Fork 11
feat: migrate BadgeIcon to ADR-0004 shared types (DSYS-479) #1010
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
Changes from all commits
ee7ed4d
d5890a9
2c38fba
2263e67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,27 +1,26 @@ | ||
| import type { BadgeIconPropsShared } from '@metamask/design-system-shared'; | ||
| import type { ComponentProps } from 'react'; | ||
|
|
||
| import type { IconName, IconProps } from '../Icon'; | ||
| import type { IconProps } from '../Icon'; | ||
|
|
||
| /** | ||
| * BadgeIcon component props. | ||
| * BadgeIcon component props (React platform-specific) | ||
| * Extends shared props from @metamask/design-system-shared with React-specific platform concerns | ||
| */ | ||
| export type BadgeIconProps = ComponentProps<'div'> & { | ||
| /** | ||
| * Required prop to specify an icon to show | ||
| */ | ||
| iconName: IconName; | ||
| /** | ||
| * Optional prop to pass additional properties to the icon | ||
| */ | ||
| iconProps?: Omit<IconProps, 'name'>; | ||
| /** | ||
| * Optional prop for additional CSS classes to be applied to the BadgeIcon component. | ||
| * These classes will be merged with the component's default classes using twMerge. | ||
| */ | ||
| className?: string; | ||
| /** | ||
| * Optional CSS styles to be applied to the component. | ||
| * Should be used sparingly and only for dynamic styles that can't be achieved with className. | ||
| */ | ||
| style?: React.CSSProperties; | ||
| }; | ||
| export type BadgeIconProps = ComponentProps<'div'> & | ||
| BadgeIconPropsShared & { | ||
| /** | ||
| * Optional prop to pass additional properties to the icon | ||
| */ | ||
| iconProps?: Omit<IconProps, 'name'>; | ||
| /** | ||
| * Optional prop for additional CSS classes to be applied to the BadgeIcon component. | ||
| * These classes will be merged with the component's default classes using twMerge. | ||
| */ | ||
| className?: string; | ||
| /** | ||
| * Optional CSS styles to be applied to the component. | ||
| * Should be used sparingly and only for dynamic styles that can't be achieved with className. | ||
| */ | ||
| style?: React.CSSProperties; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,6 +86,9 @@ export { | |
| type AvatarTokenPropsShared, | ||
| } from './types/AvatarToken'; | ||
|
|
||
| // BadgeIcon types (ADR-0004) | ||
| export { type BadgeIconPropsShared } from './types/BadgeIcon'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is only a type exported here with no const objects?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This root export is what allows both platform packages to extend BadgeIconPropsShared from the shared package entrypoint instead of reaching into internal paths. It matches the ADR-0004 public export pattern already used by the other migrated shared component types. |
||
|
|
||
| // BannerAlert types (ADR-0003 + ADR-0004) | ||
| export { | ||
| BannerAlertSeverity, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import type { IconName } from '../Icon'; | ||
|
|
||
| /** | ||
| * BadgeIcon component shared props (ADR-0004) | ||
| * Platform-independent properties shared across React and React Native | ||
| */ | ||
| export type BadgeIconPropsShared = { | ||
| /** | ||
| * Required prop to specify an icon to show. | ||
| * Uses shared IconName because the shared package owns icon names. | ||
| */ | ||
| iconName: IconName; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. iconName is the only prop moved into shared because it is part of the design-system API surface rather than platform behavior. IconName now lives in design-system-shared after #1042, so BadgeIcon should consume that shared source of truth directly. |
||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { type BadgeIconPropsShared } from './BadgeIcon.types'; |
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.
iconProps stays platform-local on purpose even after extracting iconName. The rendered Icon still has platform-specific props in each package, so this keeps only the shared API centralized and avoids pulling React-specific Icon props into the shared layer.