feat(InlineMessage): remove support for sx prop #6813
Conversation
🦋 Changeset detectedLatest commit: 87ffd4e 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 |
There was a problem hiding this comment.
Pull Request Overview
This PR removes support for the sx prop from the InlineMessage component as part of a major release. The change simplifies the component by removing the styling prop interface and refactoring icon handling to use static objects instead of functions.
Key changes:
- Remove
sxprop support and related type definitions - Replace function-based icon lookup with static object-based approach
- Switch from
BoxWithFallbackto nativedivelement
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/react/src/InlineMessage/InlineMessage.tsx | Remove sx prop support, refactor icon handling to use objects, and replace BoxWithFallback with div |
| .changeset/shy-flies-marry.md | Add changeset entry documenting the breaking change |
| const icons: Record<MessageVariant, React.ReactNode> = { | ||
| warning: <AlertIcon className={classes.InlineMessageIcon} />, | ||
| critical: <AlertIcon className={classes.InlineMessageIcon} />, | ||
| success: <CheckCircleIcon className={classes.InlineMessageIcon} />, | ||
| unavailable: <AlertIcon className={classes.InlineMessageIcon} />, | ||
| } | ||
|
|
||
| const variantToSmallIcon = (variant: MessageVariant): React.ReactNode => { | ||
| const icons = { | ||
| warning: <AlertFillIcon className={classes.InlineMessageIcon} size={12} />, | ||
| critical: <AlertFillIcon className={classes.InlineMessageIcon} size={12} />, | ||
| success: <CheckCircleFillIcon className={classes.InlineMessageIcon} size={12} />, | ||
| unavailable: <AlertFillIcon className={classes.InlineMessageIcon} size={12} />, | ||
| } | ||
| return icons[variant] | ||
| const smallIcons: Record<MessageVariant, React.ReactNode> = { | ||
| warning: <AlertFillIcon className={classes.InlineMessageIcon} size={12} />, | ||
| critical: <AlertFillIcon className={classes.InlineMessageIcon} size={12} />, | ||
| success: <CheckCircleFillIcon className={classes.InlineMessageIcon} size={12} />, | ||
| unavailable: <AlertFillIcon className={classes.InlineMessageIcon} size={12} />, | ||
| } | ||
|
|
||
| export function InlineMessage({children, className, size = 'medium', variant, ...rest}: InlineMessageProps) { | ||
| const icon = size === 'small' ? variantToSmallIcon(variant) : variantToIcon(variant) | ||
| const icon = size === 'small' ? smallIcons[variant] : icons[variant] | ||
|
|
||
| return ( | ||
| <BoxWithFallback | ||
| className={clsx(className, classes.InlineMessage)} | ||
| {...rest} | ||
| data-size={size} | ||
| data-variant={variant} | ||
| > | ||
| <div {...rest} className={clsx(className, classes.InlineMessage)} data-size={size} data-variant={variant}> | ||
| {icon} |
There was a problem hiding this comment.
Creating React elements at module level means these icons will be created on every module load rather than when needed. Consider using factory functions or lazy initialization to avoid unnecessary React element creation.
See below for a potential fix:
const icons: Record<MessageVariant, React.ComponentType<{className: string}>> = {
warning: AlertIcon,
critical: AlertIcon,
success: CheckCircleIcon,
unavailable: AlertIcon,
}
const smallIcons: Record<MessageVariant, React.ComponentType<{className: string; size: number}>> = {
warning: AlertFillIcon,
critical: AlertFillIcon,
success: CheckCircleFillIcon,
unavailable: AlertFillIcon,
}
export function InlineMessage({children, className, size = 'medium', variant, ...rest}: InlineMessageProps) {
const IconComponent = size === 'small' ? smallIcons[variant] : icons[variant]
return (
<div {...rest} className={clsx(className, classes.InlineMessage)} data-size={size} data-variant={variant}>
{size === 'small' ? (
<IconComponent className={classes.InlineMessageIcon} size={12} />
) : (
<IconComponent className={classes.InlineMessageIcon} />
)}
|
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
github-actions
bot
added
the
integration-tests: recommended
size-limit report 📦
|
github-actions
bot
added
integration-tests: passing
github-actions
bot
added
the
integration-tests: recommended
|
👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks! |
github-actions
bot
added
integration-tests: failing
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/2718 |
|
🟢 ci completed with status |
github-actions
bot
added
integration-tests: passing
Closes #6791
Changelog
New
Changed
Removed
sxfromInlineMessageRollout strategy
This component has no
sxusage downstream and is safe to remove