Skip to content

feat(cell-action): add external linking as action #95892

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions static/app/utils/discover/fieldRenderers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -322,15 +322,19 @@ export const FIELD_FORMATTERS: FieldFormatters = {
},
string: {
isSortable: true,
renderFunc: (field, data) => {
renderFunc: (field, data, baggage) => {
// Some fields have long arrays in them, only show the tail of the data.
const value = Array.isArray(data[field])
? data[field].slice(-1)
: defined(data[field])
? data[field]
: emptyValue;

if (isUrl(value)) {
// In the future, external linking will be done through CellAction component instead of the default renderer
if (
!baggage?.organization.features.includes('discover-cell-actions-v2') &&
isUrl(value)
) {
return (
<Tooltip title={value} containerDisplayMode="block" showOnlyOnOverflow>
<Container>
Expand Down Expand Up @@ -523,13 +527,7 @@ const SPECIAL_FIELDS: Record<string, SpecialField> = {
showOnlyOnOverflow
maxWidth={400}
>
<Container>
{isUrl(value) ? (
<ExternalLink href={value}>{value}</ExternalLink>
) : (
nullableValue(value)
)}
</Container>
<Container>{nullableValue(value)}</Container>
</Tooltip>
);
},
Expand Down
68 changes: 48 additions & 20 deletions static/app/views/discover/table/cellAction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import {t, tct} from 'sentry/locale';
import {defined} from 'sentry/utils';
import type {TableDataRow} from 'sentry/utils/discover/discoverQuery';
import {
fieldAlignment,
isEquationAlias,
isRelativeSpanOperationBreakdownField,
} from 'sentry/utils/discover/fields';
import getDuration from 'sentry/utils/duration/getDuration';
import {isUrl} from 'sentry/utils/string/isUrl';
import type {MutableSearch} from 'sentry/utils/tokenizeSearch';
import useOrganization from 'sentry/utils/useOrganization';

Expand All @@ -27,19 +29,7 @@ export enum Actions {
DRILLDOWN = 'drilldown',
EDIT_THRESHOLD = 'edit_threshold',
COPY_TO_CLIPBOARD = 'copy_to_clipboard',
}

export function copyToClipBoard(data: any) {
function stringifyValue(value: any): string {
if (!value) return '';
if (typeof value !== 'object') {
return value.toString();
}
return JSON.stringify(value) ?? value.toString();
}
navigator.clipboard.writeText(stringifyValue(data)).catch(_ => {
addErrorMessage('Error copying to clipboard');
});
OPEN_EXTERNAL_LINK = 'open_external_link',
}

export function updateQuery(
Expand Down Expand Up @@ -97,14 +87,12 @@ export function updateQuery(
}
// these actions do not modify the query in any way,
// instead they have side effects
case Actions.COPY_TO_CLIPBOARD:
copyToClipBoard(value);
break;
case Actions.RELEASE:
case Actions.DRILLDOWN:
break;
default:
throw new Error(`Unknown action type. ${action}`);
if (!handleCellActionFallback(action, value))
throw new Error(`Unknown action type. ${action}`);
}
}

Expand Down Expand Up @@ -251,6 +239,8 @@ function makeCellActions({

if (value) addMenuItem(Actions.COPY_TO_CLIPBOARD, t('Copy to clipboard'));

if (isUrl(value)) addMenuItem(Actions.OPEN_EXTERNAL_LINK, t('Open external link'));

if (actions.length === 0) {
return null;
}
Expand All @@ -262,10 +252,11 @@ type Props = React.PropsWithoutRef<CellActionsOpts>;

function CellAction(props: Props) {
const organization = useOrganization();
const {children} = props;
const {children, column} = props;
const cellActions = makeCellActions(props);
const align = fieldAlignment(column.key as string, column.type);

if (organization.features.includes('organizations:discover-cell-actions-v2'))
if (organization.features.includes('discover-cell-actions-v2'))
return (
<Container
data-test-id={cellActions === null ? undefined : 'cell-action-container'}
Expand All @@ -276,10 +267,11 @@ function CellAction(props: Props) {
usePortal
size="sm"
offset={4}
position="bottom-start"
position={align === 'left' ? 'bottom-start' : 'bottom-end'}
preventOverflowOptions={{padding: 4}}
flipOptions={{
fallbackPlacements: [
'bottom-start',
'bottom-end',
'top',
'right-start',
Expand Down Expand Up @@ -337,6 +329,42 @@ function CellAction(props: Props) {
);
}

/**
* A fallback that has default operations for some actions. E.g., copying to clipboard by default copies raw text, opening external link opens a new tab
* @param action
* @param value
* @returns true if a default action was executed, false otherwise
*/
export function handleCellActionFallback(
action: Actions,
value: string | number | string[]
): boolean {
switch (action) {
case Actions.COPY_TO_CLIPBOARD: {
function stringifyValue(val: any): string {
if (!val) return '';
if (typeof val !== 'object') {
return val.toString();
}
return JSON.stringify(val) ?? val.toString();
}
navigator.clipboard.writeText(stringifyValue(value)).catch(_ => {
addErrorMessage('Error copying to clipboard');
});
return true;
}
case Actions.OPEN_EXTERNAL_LINK:
if (typeof value === 'string' && isUrl(value)) {
window.open(value, '_blank', 'noopener,noreferrer');
} else {
addErrorMessage('Could not open link');
}
return true;
default:
return false;
}
}

export default CellAction;

const Container = styled('div')`
Expand Down
6 changes: 1 addition & 5 deletions static/app/views/discover/table/tableView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ import {makeReleasesPathname} from 'sentry/views/releases/utils/pathnames';

import {QuickContextHoverWrapper} from './quickContext/quickContextWrapper';
import {ContextType} from './quickContext/utils';
import CellAction, {Actions, copyToClipBoard, updateQuery} from './cellAction';
import CellAction, {Actions, updateQuery} from './cellAction';
import ColumnEditModal, {modalCss} from './columnEditModal';
import TableActions from './tableActions';
import {TopResultsIndicator} from './topResultsIndicator';
Expand Down Expand Up @@ -609,10 +609,6 @@ function TableView(props: TableViewProps) {

return;
}
case Actions.COPY_TO_CLIPBOARD: {
copyToClipBoard(value);
break;
}
default: {
// Some custom perf metrics have units.
// These custom perf metrics need to be adjusted to the correct value.
Expand Down
1 change: 1 addition & 0 deletions static/app/views/explore/components/table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export const ALLOWED_CELL_ACTIONS: Actions[] = [
Actions.SHOW_GREATER_THAN,
Actions.SHOW_LESS_THAN,
Actions.COPY_TO_CLIPBOARD,
Actions.OPEN_EXTERNAL_LINK,
];

const MINIMUM_COLUMN_WIDTH = COL_WIDTH_MINIMUM;
Expand Down
6 changes: 2 additions & 4 deletions static/app/views/explore/logs/tables/logsTableRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import useOrganization from 'sentry/utils/useOrganization';
import useProjectFromId from 'sentry/utils/useProjectFromId';
import CellAction, {
Actions,
copyToClipBoard,
handleCellActionFallback,
} from 'sentry/views/discover/table/cellAction';
import type {TableColumn} from 'sentry/views/discover/table/types';
import {AttributesTree} from 'sentry/views/explore/components/traceItemAttributes/attributesTree';
Expand Down Expand Up @@ -304,10 +304,8 @@ export const LogRowContent = memo(function LogRowContent({
negated: true,
});
break;
case Actions.COPY_TO_CLIPBOARD:
copyToClipBoard(cellValue);
break;
default:
handleCellActionFallback(actions, cellValue);
break;
}
}}
Expand Down
16 changes: 4 additions & 12 deletions static/app/views/explore/tables/fieldRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import styled from '@emotion/styled';
import {Link} from 'sentry/components/core/link';
import {Tooltip} from 'sentry/components/core/tooltip';
import ProjectBadge from 'sentry/components/idBadge/projectBadge';
import ExternalLink from 'sentry/components/links/externalLink';
import TimeSince from 'sentry/components/timeSince';
import {space} from 'sentry/styles/space';
import type {Project} from 'sentry/types/project';
Expand All @@ -18,7 +17,6 @@ import {Container} from 'sentry/utils/discover/styles';
import {generateLinkToEventInTraceView} from 'sentry/utils/discover/urls';
import {getShortEventId} from 'sentry/utils/events';
import {generateProfileFlamechartRouteWithQuery} from 'sentry/utils/profiling/routes';
import {isUrl} from 'sentry/utils/string/isUrl';
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
import {useLocation} from 'sentry/utils/useLocation';
import useOrganization from 'sentry/utils/useOrganization';
Expand Down Expand Up @@ -146,7 +144,7 @@ function BaseExploreFieldRenderer({
source: TraceViewSources.TRACES,
});

rendered = <Link to={target}>{rendered}</Link>;
return <Link to={target}>{rendered}</Link>;
}

if (['id', 'span_id', 'transaction.id'].includes(field)) {
Expand All @@ -162,7 +160,7 @@ function BaseExploreFieldRenderer({
source: TraceViewSources.TRACES,
});

rendered = <Link to={target}>{rendered}</Link>;
return <Link to={target}>{rendered}</Link>;
}

if (field === 'profile.id') {
Expand All @@ -171,7 +169,7 @@ function BaseExploreFieldRenderer({
projectSlug: data.project,
profileId: data['profile.id'],
});
rendered = <Link to={target}>{rendered}</Link>;
return <Link to={target}>{rendered}</Link>;
}

return (
Expand Down Expand Up @@ -238,13 +236,7 @@ function spanDescriptionRenderFunc(projects: Record<string, Project>) {
hideName
/>
)}
<WrappingText>
{isUrl(value) ? (
<ExternalLink href={value}>{value}</ExternalLink>
) : (
nullableValue(value)
)}
</WrappingText>
<WrappingText>{nullableValue(value)}</WrappingText>
</Description>
</Tooltip>
</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ function EventsTable({
Actions.EXCLUDE,
Actions.SHOW_GREATER_THAN,
Actions.SHOW_LESS_THAN,
Actions.OPEN_EXTERNAL_LINK,
];

if (['attachments', 'minidump'].includes(field)) {
Expand Down
Loading