Skip to content

ref(button-variant): remove deprecated priority prop from Button#114756

Merged
natemoo-re merged 3 commits intomasterfrom
nm/button-variant/remove-priority
May 4, 2026
Merged

ref(button-variant): remove deprecated priority prop from Button#114756
natemoo-re merged 3 commits intomasterfrom
nm/button-variant/remove-priority

Conversation

@natemoo-re
Copy link
Copy Markdown
Member

Final step of the button-variant codemod: removes the priority prop, ButtonPriority type, and DO_NOT_USE_resolveButtonVariant resolver from the Button component. All callers must use variant instead.

Blocked by #114730.

@natemoo-re natemoo-re requested a review from a team as a code owner May 4, 2026 19:18
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 4, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

📊 Type Coverage Diff

✅ No new type safety issues introduced. Coverage: 93.40%

Copy link
Copy Markdown
Member

@JonasBa JonasBa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@natemoo-re Nice work! Lets send out a PSA to discuss frontend so folks are aware of the change 🙏🏼

Remove the `priority` prop, `ButtonPriority` type, and
`DO_NOT_USE_resolveButtonVariant` resolver from the Button component.
All callers should use `variant` instead.

Blocked by the four codemod(button-variant) PRs landing first.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines 15 to 21
interface DirectEnableButtonProps {
buttonProps: Pick<
React.ComponentProps<typeof AddIntegrationButton>,
'size' | 'priority' | 'disabled' | 'style' | 'data-test-id' | 'icon' | 'buttonText'
'size' | 'variant' | 'disabled' | 'style' | 'data-test-id' | 'icon' | 'buttonText'
>;
providerSlug: string;
userHasAccess: boolean;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The integration install button incorrectly uses the old priority prop instead of the new variant prop, causing it to render with the wrong visual style.
Severity: LOW

Suggested Fix

In integrationDetailedView.tsx, update the buttonProps initialization to use the new variant prop. Specifically, change priority: 'primary' to variant: 'primary'.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location:
static/app/views/settings/organizationIntegrations/directEnableButton.tsx#L15-L21

Potential issue: The `Button` component's API was updated to replace the `priority` prop
with `variant`. However, `integrationDetailedView.tsx` was not updated and continues to
pass `priority: 'primary'` within its `buttonProps`. This prop is now ignored by child
components like `DirectEnableButton`, causing the button to fall back to its default
`'secondary'` variant instead of the intended `'primary'` one. This results in a visual
regression on the integration detail page, where the primary install action is no longer
visually prominent.

Also affects:

  • static/app/views/settings/organizationIntegrations/integrationDetailedView.tsx:313

Did we get this right? 👍 / 👎 to inform future reviews.

@natemoo-re natemoo-re merged commit e4674f0 into master May 4, 2026
72 checks passed
@natemoo-re natemoo-re deleted the nm/button-variant/remove-priority branch May 4, 2026 21:51
cleptric pushed a commit that referenced this pull request May 5, 2026
…4756)

Final step of the [button-variant
codemod](https://github.com/getsentry/design-engineering/tree/main/codemods/button-variant):
removes the `priority` prop, `ButtonPriority` type, and
`DO_NOT_USE_resolveButtonVariant` resolver from the Button component.
All callers must use `variant` instead.

Blocked by #114730.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants