Skip to content

Commit 7196fc7

Browse files
authored
link: cleanup link (#91687)
Cleanup Link component and some of the variants. - LocationDescriptor type is already LocationDescriptor | string, so there is no need for extra intersection - Removed LinkWithConfirmation as it is a low value abstraction that is trivial to implement - move link styles from styles.tsx to link.tsx Some open questions and other fixes to do here: - Do we want ExternalLink to be it's own component, or should we make this `<Link external href=''/>`, which would make this consistent with the implementation of LinkButton - If so, is Anchor component still needed or can we standardize on Link - This component accepts `disabled` boolean prop, however, unlike LinkButton, it does not set `href` or `to` prop to undefined if disabled is set to true. Links cannot be disabled.
1 parent b93eef1 commit 7196fc7

File tree

13 files changed

+51
-104
lines changed

13 files changed

+51
-104
lines changed

static/app/components/events/eventDataSection.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import styled from '@emotion/styled';
22

33
import {DataSection} from 'sentry/components/events/styles';
4-
import Anchor from 'sentry/components/links/anchor';
4+
import {Anchor} from 'sentry/components/links/link';
55
import QuestionTooltip from 'sentry/components/questionTooltip';
66
import {IconLink} from 'sentry/icons';
77
import {space} from 'sentry/styles/space';

static/app/components/links/anchor.tsx

Lines changed: 0 additions & 13 deletions
This file was deleted.

static/app/components/links/externalLink.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import Anchor from './anchor';
1+
import {Anchor} from './link';
22

33
interface ExternalLinkProps
44
extends Omit<React.AnchorHTMLAttributes<HTMLAnchorElement>, 'target'> {

static/app/components/links/link.tsx

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,34 @@ import {
22
Link as RouterLink,
33
type LinkProps as ReactRouterLinkProps,
44
} from 'react-router-dom';
5+
import isPropValid from '@emotion/is-prop-valid';
6+
import {css, type Theme} from '@emotion/react';
57
import styled from '@emotion/styled';
68
import type {LocationDescriptor} from 'history';
79

810
import {locationDescriptorToTo} from 'sentry/utils/reactRouter6Compat/location';
911
import normalizeUrl from 'sentry/utils/url/normalizeUrl';
1012
import {useLocation} from 'sentry/utils/useLocation';
1113

12-
import {linkStyles} from './styles';
14+
export const linkStyles = ({disabled, theme}: {theme: Theme; disabled?: boolean}) => css`
15+
/* @TODO(jonasbadalic) This was defined on theme and only used here */
16+
border-radius: 2px;
17+
18+
&:focus-visible {
19+
box-shadow: ${theme.linkFocus} 0 0 0 2px;
20+
text-decoration: none;
21+
outline: none;
22+
}
23+
24+
${disabled &&
25+
css`
26+
color: ${theme.disabled};
27+
pointer-events: none;
28+
:hover {
29+
color: ${theme.disabled};
30+
}
31+
`}
32+
`;
1333

1434
export interface LinkProps
1535
extends Omit<
@@ -35,7 +55,6 @@ export interface LinkProps
3555
*/
3656
disabled?: boolean;
3757
preventScrollReset?: ReactRouterLinkProps['preventScrollReset'];
38-
ref?: React.Ref<HTMLAnchorElement>;
3958
replace?: ReactRouterLinkProps['replace'];
4059
state?: ReactRouterLinkProps['state'];
4160
}
@@ -44,19 +63,23 @@ export interface LinkProps
4463
* A context-aware version of Link (from react-router) that falls
4564
* back to <a> if there is no router present
4665
*/
47-
function BaseLink({disabled, to, ...props}: LinkProps): React.ReactElement {
66+
const Link = styled(({disabled, to, ...props}: LinkProps) => {
4867
const location = useLocation();
4968
to = normalizeUrl(to, location);
5069

5170
if (!disabled && location) {
5271
return <RouterLink to={locationDescriptorToTo(to)} {...props} />;
5372
}
5473

55-
return <a href={typeof to === 'string' ? to : ''} {...props} />;
56-
}
74+
return <Anchor href={typeof to === 'string' ? to : ''} {...props} />;
75+
})`
76+
${linkStyles}
77+
`;
5778

58-
// Re-assign to Link to make auto-importing smarter
59-
const Link = styled(BaseLink)`
79+
export const Anchor = styled('a', {
80+
shouldForwardProp: prop =>
81+
typeof prop === 'string' && isPropValid(prop) && prop !== 'disabled',
82+
})<{disabled?: boolean}>`
6083
${linkStyles}
6184
`;
6285

static/app/components/links/linkWithConfirmation.tsx

Lines changed: 0 additions & 35 deletions
This file was deleted.

static/app/components/links/styles.tsx

Lines changed: 0 additions & 21 deletions
This file was deleted.

static/app/components/quickTrace/styles.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ type DropdownItemProps = {
100100
children: React.ReactNode;
101101
allowDefaultEvent?: boolean;
102102
onSelect?: (eventKey: any) => void;
103-
to?: string | LocationDescriptor;
103+
to?: LocationDescriptor;
104104
width?: 'small' | 'large';
105105
};
106106

static/app/types/group.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -978,7 +978,7 @@ export type KeyValueListDataItem = {
978978
key: string;
979979
subject: string;
980980
action?: {
981-
link?: string | LocationDescriptor;
981+
link?: LocationDescriptor;
982982
};
983983
actionButton?: React.ReactNode;
984984
/**

static/app/views/admin/adminRelays.tsx

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ import {Component} from 'react';
22
import moment from 'moment-timezone';
33

44
import type {Client} from 'sentry/api';
5-
import LinkWithConfirmation from 'sentry/components/links/linkWithConfirmation';
5+
import Confirm from 'sentry/components/confirm';
6+
import {Button} from 'sentry/components/core/button';
67
import ResultGrid from 'sentry/components/resultGrid';
8+
import {IconDelete} from 'sentry/icons';
79
import {t} from 'sentry/locale';
810
import type {RouteComponentProps} from 'sentry/types/legacyReactRouter';
911
import withApi from 'sentry/utils/withApi';
@@ -52,14 +54,14 @@ class AdminRelays extends Component<Props, State> {
5254
</td>,
5355
<td key="tools" style={{textAlign: 'right'}}>
5456
<span className="editor-tools">
55-
<LinkWithConfirmation
56-
className="danger"
57-
title="Remove"
57+
<Confirm
5858
message={t('Are you sure you wish to delete this relay?')}
5959
onConfirm={() => this.onDelete(row.id)}
6060
>
61-
{t('Remove')}
62-
</LinkWithConfirmation>
61+
<Button priority="danger" size="sm" icon={<IconDelete />}>
62+
{t('Remove Relay')}
63+
</Button>
64+
</Confirm>
6365
</span>
6466
</td>,
6567
];

static/app/views/nav/primary/components.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@ import {Button} from 'sentry/components/core/button';
66
import {Tooltip} from 'sentry/components/core/tooltip';
77
import {DropdownMenu, type MenuItemProps} from 'sentry/components/dropdownMenu';
88
import InteractionStateLayer from 'sentry/components/interactionStateLayer';
9-
import Link from 'sentry/components/links/link';
10-
import {linkStyles} from 'sentry/components/links/styles';
9+
import Link, {linkStyles} from 'sentry/components/links/link';
1110
import {SIDEBAR_NAVIGATION_SOURCE} from 'sentry/components/sidebar/utils';
1211
import {IconDefaultsProvider} from 'sentry/icons/useIconDefaults';
1312
import {space} from 'sentry/styles/space';

static/app/views/settings/organizationApiKeys/index.spec.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe('OrganizationApiKeys', function () {
4242
render(<OrganizationApiKeys />);
4343
renderGlobalModal();
4444

45-
await userEvent.click(await screen.findByRole('link', {name: 'Remove API Key?'}));
45+
await userEvent.click(await screen.findByRole('button', {name: 'Remove API Key'}));
4646
expect(deleteMock).toHaveBeenCalledTimes(0);
4747

4848
await userEvent.click(screen.getByTestId('confirm-button'));

static/app/views/settings/organizationApiKeys/organizationApiKeysList.spec.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe('OrganizationApiKeysList', function () {
2525
renderGlobalModal();
2626

2727
// Click remove button
28-
await userEvent.click(await screen.findByTitle('Remove API Key?'));
28+
await userEvent.click(await screen.findByRole('button', {name: 'Remove API Key'}));
2929

3030
expect(screen.getByRole('dialog')).toBeInTheDocument();
3131
});

static/app/views/settings/organizationApiKeys/organizationApiKeysList.tsx

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
import {Fragment} from 'react';
2-
import {css} from '@emotion/react';
32
import styled from '@emotion/styled';
43

4+
import Confirm from 'sentry/components/confirm';
55
import {AlertLink} from 'sentry/components/core/alert/alertLink';
66
import {Button} from 'sentry/components/core/button';
77
import ExternalLink from 'sentry/components/links/externalLink';
88
import Link from 'sentry/components/links/link';
9-
import LinkWithConfirmation from 'sentry/components/links/linkWithConfirmation';
109
import {PanelTable} from 'sentry/components/panels/panelTable';
1110
import TextCopyInput from 'sentry/components/textCopyInput';
1211
import {IconAdd, IconDelete} from 'sentry/icons';
@@ -101,21 +100,14 @@ function OrganizationApiKeysList({
101100
</TextCopyInput>
102101

103102
<Cell>
104-
<LinkWithConfirmation
105-
aria-label={t('Remove API Key')}
106-
className="btn btn-default btn-sm"
103+
<Confirm
107104
onConfirm={() => onRemove(id)}
108105
message={t('Are you sure you want to remove this API key?')}
109-
title={t('Remove API Key?')}
110106
>
111-
<IconDelete
112-
size="xs"
113-
css={css`
114-
position: relative;
115-
top: 2px;
116-
`}
117-
/>
118-
</LinkWithConfirmation>
107+
<Button priority="danger" size="sm" icon={<IconDelete />}>
108+
{t('Remove API Key')}
109+
</Button>
110+
</Confirm>
119111
</Cell>
120112
</Fragment>
121113
);

0 commit comments

Comments
 (0)