Skip to content

Commit 2e16025

Browse files
authored
linkbutton: make either href or to required (#91649)
Refactor types to be a bit stronger for LinkButton by requiring either to or href
1 parent e769bbb commit 2e16025

File tree

5 files changed

+69
-41
lines changed

5 files changed

+69
-41
lines changed

static/app/components/core/button/linkButton.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,15 @@ interface BaseLinkButtonProps extends DO_NOT_USE_CommonButtonProps, LinkElementP
2929
}
3030

3131
interface LinkButtonPropsWithHref extends BaseLinkButtonProps {
32+
href: string;
3233
/**
3334
* Determines if the link is external and should open in a new tab.
3435
*/
3536
external?: boolean;
36-
href?: string;
3737
}
3838

3939
interface LinkButtonPropsWithTo extends BaseLinkButtonProps {
40+
to: string | LocationDescriptor;
4041
/**
4142
* If true, the link will not reset the scroll position of the page when clicked.
4243
*/
@@ -45,7 +46,6 @@ interface LinkButtonPropsWithTo extends BaseLinkButtonProps {
4546
* Determines if the link should replace the current history entry.
4647
*/
4748
replace?: boolean;
48-
to?: string | LocationDescriptor;
4949
}
5050

5151
export type LinkButtonProps = LinkButtonPropsWithHref | LinkButtonPropsWithTo;
@@ -68,7 +68,9 @@ export function LinkButton({
6868
aria-disabled={disabled}
6969
size={size}
7070
{...props}
71+
// @ts-expect-error set href as undefined to force "disabled" state.
7172
href={disabled ? undefined : 'href' in props ? props.href : undefined}
73+
// @ts-expect-error set to as undefined to force "disabled" state
7274
to={disabled ? undefined : 'to' in props ? props.to : undefined}
7375
onClick={handleClick}
7476
>

static/app/components/createAlertButton.tsx

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import type {LocationDescriptor} from 'history';
2+
13
import {
24
addErrorMessage,
35
addLoadingMessage,
@@ -26,7 +28,7 @@ import {
2628
DEFAULT_WIZARD_TEMPLATE,
2729
} from 'sentry/views/alerts/wizard/options';
2830

29-
type CreateAlertFromViewButtonProps = Omit<LinkButtonProps, 'aria-label'> & {
31+
type CreateAlertFromViewButtonProps = Omit<LinkButtonProps, 'aria-label' | 'to'> & {
3032
/**
3133
* Discover query used to create the alert
3234
*/
@@ -40,19 +42,19 @@ type CreateAlertFromViewButtonProps = Omit<LinkButtonProps, 'aria-label'> & {
4042
* We currently do a few checks on metrics data on performance pages and this passes the decision onward to alerts.
4143
*/
4244
disableMetricDataset?: boolean;
45+
4346
/**
4447
* Called when the user is redirected to the alert builder
4548
*/
4649
onClick?: () => void;
47-
4850
referrer?: string;
4951
};
5052

5153
/**
5254
* Provide a button that can create an alert from an event view.
5355
* Emits incompatible query issues on click
5456
*/
55-
function CreateAlertFromViewButton({
57+
export function CreateAlertFromViewButton({
5658
projects,
5759
eventView,
5860
organization,
@@ -121,7 +123,8 @@ type CreateAlertButtonProps = {
121123
projectSlug?: string;
122124
referrer?: string;
123125
showPermissionGuide?: boolean;
124-
} & LinkButtonProps;
126+
to?: string | LocationDescriptor;
127+
} & Omit<LinkButtonProps, 'to'>;
125128

126129
export default function CreateAlertButton({
127130
organization,
@@ -132,6 +135,7 @@ export default function CreateAlertButton({
132135
showPermissionGuide,
133136
alertOption,
134137
onEnter,
138+
to,
135139
...buttonProps
136140
}: CreateAlertButtonProps) {
137141
const router = useRouter();
@@ -189,7 +193,7 @@ export default function CreateAlertButton({
189193
disabled={!hasAccess}
190194
title={hasAccess ? undefined : permissionTooltipText}
191195
icon={!hideIcon && <IconSiren {...iconProps} />}
192-
to={projectSlug ? createAlertUrl(projectSlug) : undefined}
196+
to={to ?? (projectSlug ? createAlertUrl(projectSlug) : '')}
193197
tooltipProps={{
194198
isHoverable: true,
195199
position: 'top',
@@ -222,5 +226,3 @@ export default function CreateAlertButton({
222226
renderButton(canCreateAlert)
223227
);
224228
}
225-
226-
export {CreateAlertFromViewButton};

static/gsApp/components/profiling/alerts.tsx

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import styled from '@emotion/styled';
33

44
import {Alert} from 'sentry/components/core/alert';
55
import {Button} from 'sentry/components/core/button';
6-
import {LinkButton} from 'sentry/components/core/button/linkButton';
76
import {IconClose, IconInfo, IconWarning} from 'sentry/icons';
87
import {t, tct} from 'sentry/locale';
98
import {space} from 'sentry/styles/space';
@@ -87,7 +86,6 @@ interface GraceAlertProps {
8786
action: {
8887
label: string;
8988
onClick?: () => void;
90-
to?: string | Record<PropertyKey, unknown>;
9189
};
9290
children: React.ReactNode;
9391
dismiss: undefined | (() => void);
@@ -98,14 +96,9 @@ interface GraceAlertProps {
9896
function GraceAlert({children, action, dismiss, type, disableAction}: GraceAlertProps) {
9997
const trailingItems = (
10098
<Fragment>
101-
<LinkButton
102-
size="xs"
103-
onClick={action.onClick}
104-
to={action.to}
105-
disabled={disableAction}
106-
>
99+
<Button size="xs" onClick={action.onClick} disabled={disableAction}>
107100
{action.label}
108-
</LinkButton>
101+
</Button>
109102
{dismiss ? (
110103
<StyledButton priority="link" size="sm" onClick={dismiss}>
111104
<IconClose color="gray500" size="sm" />

static/gsApp/components/startTrialButton.tsx

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import type {LinkButtonProps} from 'sentry/components/core/button/linkButton';
2-
import {LinkButton} from 'sentry/components/core/button/linkButton';
1+
import {Button, type ButtonProps} from 'sentry/components/core/button';
2+
import {LinkButton, type LinkButtonProps} from 'sentry/components/core/button/linkButton';
33
import {t} from 'sentry/locale';
44
import IndicatorStore from 'sentry/stores/indicatorStore';
55
import type {Organization} from 'sentry/types/organization';
@@ -15,7 +15,7 @@ type Props = React.PropsWithChildren<
1515
onTrialFailed?: () => void;
1616
onTrialStarted?: () => void;
1717
requestData?: Record<string, unknown>;
18-
} & LinkButtonProps
18+
} & (ButtonProps | LinkButtonProps)
1919
>;
2020

2121
function StartTrialButton({
@@ -40,19 +40,40 @@ function StartTrialButton({
4040
onTrialStarted={onTrialStarted}
4141
requestData={requestData}
4242
>
43-
{({startTrial, trialStarting, trialStarted}) => (
44-
<LinkButton
45-
disabled={trialStarting || trialStarted}
46-
data-test-id="start-trial-button"
47-
onClick={() => {
48-
handleClick?.();
49-
startTrial();
50-
}}
51-
{...buttonProps}
52-
>
53-
{children || t('Start trial')}
54-
</LinkButton>
55-
)}
43+
{({startTrial, trialStarting, trialStarted}) => {
44+
if (
45+
('to' in buttonProps && buttonProps.to !== undefined) ||
46+
('href' in buttonProps && buttonProps.href !== undefined)
47+
) {
48+
return (
49+
<LinkButton
50+
disabled={trialStarting || trialStarted}
51+
data-test-id="start-trial-button"
52+
onClick={() => {
53+
handleClick?.();
54+
startTrial();
55+
}}
56+
{...buttonProps}
57+
>
58+
{children || t('Start trial')}
59+
</LinkButton>
60+
);
61+
}
62+
63+
return (
64+
<Button
65+
disabled={trialStarting || trialStarted}
66+
data-test-id="start-trial-button"
67+
onClick={() => {
68+
handleClick?.();
69+
startTrial();
70+
}}
71+
{...buttonProps}
72+
>
73+
{children || t('Start trial')}
74+
</Button>
75+
);
76+
}}
5677
</TrialStarter>
5778
);
5879
}

static/gsApp/components/upgradeOrTrialButton.tsx

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ type ChildRenderProps = {
2222

2323
type ChildRenderFunction = (options: ChildRenderProps) => React.ReactNode;
2424

25-
interface Props
26-
extends Omit<ButtonProps & LinkButtonProps, 'to' | 'onClick' | 'busy' | 'children'> {
25+
interface BaseButtonProps {
2726
api: Client;
2827
organization: Organization;
2928
source: string;
@@ -61,7 +60,8 @@ function UpgradeOrTrialButton({
6160
upgradePriority = 'primary',
6261
trialPriority = 'primary',
6362
...props
64-
}: Props) {
63+
}: BaseButtonProps &
64+
(Omit<LinkButtonProps, 'children'> | Omit<ButtonProps, 'children'>)) {
6565
const {slug} = organization;
6666
const hasAccess = organization.access.includes('org:billing');
6767

@@ -131,15 +131,20 @@ function UpgradeOrTrialButton({
131131
onTrialStarted={handleSuccess}
132132
requestData={requestData}
133133
priority={buttonPriority}
134-
{...props}
134+
{...(props as LinkButtonProps)}
135135
>
136136
{childComponent || t('Start %s-Day Trial', getTrialLength(organization))}
137137
</StartTrialButton>
138138
);
139139
}
140140
// non-admin who wants to trial
141141
return (
142-
<Button onClick={handleRequest} busy={busy} priority={buttonPriority} {...props}>
142+
<Button
143+
onClick={handleRequest}
144+
busy={busy}
145+
priority={buttonPriority}
146+
{...(props as ButtonProps)}
147+
>
143148
{childComponent || t('Request Trial')}
144149
</Button>
145150
);
@@ -156,14 +161,19 @@ function UpgradeOrTrialButton({
156161
onClick={handleSuccess}
157162
to={`${baseUrl}?referrer=upgrade-${source}`}
158163
priority={buttonPriority}
159-
{...props}
164+
{...(props as LinkButtonProps)}
160165
>
161166
{childComponent || t('Upgrade now')}
162167
</LinkButton>
163168
);
164169
}
165170
return (
166-
<Button onClick={handleRequest} busy={busy} priority={buttonPriority} {...props}>
171+
<Button
172+
onClick={handleRequest}
173+
busy={busy}
174+
priority={buttonPriority}
175+
{...(props as ButtonProps)}
176+
>
167177
{childComponent || t('Request Upgrade')}
168178
</Button>
169179
);

0 commit comments

Comments
 (0)