From f3dd8528a9f54160b0b8012e5cb2a855385b39fd Mon Sep 17 00:00:00 2001 From: lzhao-sentry Date: Fri, 18 Jul 2025 13:02:30 -0400 Subject: [PATCH 1/7] feat(cell-action): add external linking as action --- static/app/utils/discover/fieldRenderers.tsx | 16 +++---- .../app/views/discover/table/cellAction.tsx | 45 ++++++++++++++++--- static/app/views/explore/components/table.tsx | 1 + .../explore/logs/tables/logsTableRow.tsx | 6 +-- .../views/explore/tables/fieldRenderer.tsx | 16 ++----- .../transactionEvents/eventsTable.tsx | 1 + 6 files changed, 53 insertions(+), 32 deletions(-) diff --git a/static/app/utils/discover/fieldRenderers.tsx b/static/app/utils/discover/fieldRenderers.tsx index 6decdcac8f9c2d..feff053096f2d7 100644 --- a/static/app/utils/discover/fieldRenderers.tsx +++ b/static/app/utils/discover/fieldRenderers.tsx @@ -322,7 +322,7 @@ 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) @@ -330,7 +330,11 @@ export const FIELD_FORMATTERS: FieldFormatters = { ? 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 ( @@ -523,13 +527,7 @@ const SPECIAL_FIELDS: Record = { showOnlyOnOverflow maxWidth={400} > - - {isUrl(value) ? ( - {value} - ) : ( - nullableValue(value) - )} - + {nullableValue(value)} ); }, diff --git a/static/app/views/discover/table/cellAction.tsx b/static/app/views/discover/table/cellAction.tsx index 38fc45879332fc..6cf9af6521d0ce 100644 --- a/static/app/views/discover/table/cellAction.tsx +++ b/static/app/views/discover/table/cellAction.tsx @@ -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'; @@ -27,6 +29,7 @@ export enum Actions { DRILLDOWN = 'drilldown', EDIT_THRESHOLD = 'edit_threshold', COPY_TO_CLIPBOARD = 'copy_to_clipboard', + OPEN_EXTERNAL_LINK = 'open_external_link', } export function copyToClipBoard(data: any) { @@ -97,14 +100,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}`); } } @@ -251,6 +252,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; } @@ -262,10 +265,11 @@ type Props = React.PropsWithoutRef; 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 ( {rendered}; + return {rendered}; } if (['id', 'span_id', 'transaction.id'].includes(field)) { @@ -162,7 +160,7 @@ function BaseExploreFieldRenderer({ source: TraceViewSources.TRACES, }); - rendered = {rendered}; + return {rendered}; } if (field === 'profile.id') { @@ -171,7 +169,7 @@ function BaseExploreFieldRenderer({ projectSlug: data.project, profileId: data['profile.id'], }); - rendered = {rendered}; + return {rendered}; } return ( @@ -238,13 +236,7 @@ function spanDescriptionRenderFunc(projects: Record) { hideName /> )} - - {isUrl(value) ? ( - {value} - ) : ( - nullableValue(value) - )} - + {nullableValue(value)} diff --git a/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx b/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx index 08cae0acdbd361..d2e32e9cbba506 100644 --- a/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx +++ b/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx @@ -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)) { From 67fcfc245197c5feb051feb59ca6d496c8aa6056 Mon Sep 17 00:00:00 2001 From: lzhao-sentry Date: Fri, 18 Jul 2025 14:28:53 -0400 Subject: [PATCH 2/7] refactor copy to clipboard handler --- .../app/views/discover/table/cellAction.tsx | 29 +++++++++---------- static/app/views/discover/table/tableView.tsx | 6 +--- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/static/app/views/discover/table/cellAction.tsx b/static/app/views/discover/table/cellAction.tsx index 6cf9af6521d0ce..099583fd49d46c 100644 --- a/static/app/views/discover/table/cellAction.tsx +++ b/static/app/views/discover/table/cellAction.tsx @@ -32,19 +32,6 @@ export enum Actions { OPEN_EXTERNAL_LINK = 'open_external_link', } -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'); - }); -} - export function updateQuery( results: MutableSearch, action: Actions, @@ -343,7 +330,7 @@ function CellAction(props: Props) { } /** - * A fallback that has default operations for some actions. E.g., copying to clipboard by default copies raw text, opening link by default opens it in a new link + * 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 @@ -353,9 +340,19 @@ export function handleCellActionFallback( value: string | number | string[] ): boolean { switch (action) { - case Actions.COPY_TO_CLIPBOARD: - copyToClipBoard(value); + 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'); diff --git a/static/app/views/discover/table/tableView.tsx b/static/app/views/discover/table/tableView.tsx index 250a490ec1a200..5199a10d4239b2 100644 --- a/static/app/views/discover/table/tableView.tsx +++ b/static/app/views/discover/table/tableView.tsx @@ -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'; @@ -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. From 49b6fb3ec2b21b5b1214201cb6530a61d5a17d10 Mon Sep 17 00:00:00 2001 From: lzhao-sentry Date: Tue, 22 Jul 2025 09:54:19 -0400 Subject: [PATCH 3/7] add feature flag check back, better type notation --- static/app/utils/discover/fieldRenderers.tsx | 11 +++++++-- .../app/views/discover/table/cellAction.tsx | 2 +- .../views/explore/tables/fieldRenderer.tsx | 24 +++++++++++++++---- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/static/app/utils/discover/fieldRenderers.tsx b/static/app/utils/discover/fieldRenderers.tsx index feff053096f2d7..2d066516681a41 100644 --- a/static/app/utils/discover/fieldRenderers.tsx +++ b/static/app/utils/discover/fieldRenderers.tsx @@ -500,7 +500,7 @@ const SPECIAL_FIELDS: Record = { }, 'span.description': { sortField: 'span.description', - renderFunc: data => { + renderFunc: (data, {organization}) => { const value = data[SpanFields.SPAN_DESCRIPTION]; const op: string = data[SpanFields.SPAN_OP]; const projectId = @@ -527,7 +527,14 @@ const SPECIAL_FIELDS: Record = { showOnlyOnOverflow maxWidth={400} > - {nullableValue(value)} + + {!organization.features.includes('discover-cell-actions-v2') && + isUrl(value) ? ( + {value} + ) : ( + nullableValue(value) + )} + ); }, diff --git a/static/app/views/discover/table/cellAction.tsx b/static/app/views/discover/table/cellAction.tsx index 099583fd49d46c..f52847bc41cca9 100644 --- a/static/app/views/discover/table/cellAction.tsx +++ b/static/app/views/discover/table/cellAction.tsx @@ -341,7 +341,7 @@ export function handleCellActionFallback( ): boolean { switch (action) { case Actions.COPY_TO_CLIPBOARD: { - function stringifyValue(val: any): string { + function stringifyValue(val: string | number | string[]): string { if (!val) return ''; if (typeof val !== 'object') { return val.toString(); diff --git a/static/app/views/explore/tables/fieldRenderer.tsx b/static/app/views/explore/tables/fieldRenderer.tsx index 166922ef05bf14..521f04608a6ee1 100644 --- a/static/app/views/explore/tables/fieldRenderer.tsx +++ b/static/app/views/explore/tables/fieldRenderer.tsx @@ -5,8 +5,10 @@ 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 {Organization} from 'sentry/types/organization'; import type {Project} from 'sentry/types/project'; import {defined} from 'sentry/utils'; import type {TableDataRow} from 'sentry/utils/discover/discoverQuery'; @@ -17,6 +19,7 @@ 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'; @@ -120,7 +123,7 @@ function BaseExploreFieldRenderer({ const field = String(column.key); - const renderer = getExploreFieldRenderer(field, meta, projectsMap); + const renderer = getExploreFieldRenderer(field, meta, projectsMap, organization); let rendered = renderer(data, { location, @@ -190,13 +193,14 @@ function BaseExploreFieldRenderer({ function getExploreFieldRenderer( field: string, meta: MetaType, - projects: Record + projects: Record, + organization: Organization ): ReturnType { if (field === 'id' || field === 'span_id') { return eventIdRenderFunc(field); } if (field === 'span.description') { - return spanDescriptionRenderFunc(projects); + return spanDescriptionRenderFunc(projects, organization); } return getFieldRenderer(field, meta, false); } @@ -213,7 +217,10 @@ function eventIdRenderFunc(field: string) { return renderer; } -function spanDescriptionRenderFunc(projects: Record) { +function spanDescriptionRenderFunc( + projects: Record, + organization: Organization +) { function renderer(data: EventData) { const project = projects[data.project]; @@ -236,7 +243,14 @@ function spanDescriptionRenderFunc(projects: Record) { hideName /> )} - {nullableValue(value)} + + {!organization.features.includes('discover-cell-actions-v2') && + isUrl(value) ? ( + {value} + ) : ( + nullableValue(value) + )} + From b6fea45d255a459e5376ed1d2b54e2024501b21c Mon Sep 17 00:00:00 2001 From: lzhao-sentry Date: Tue, 22 Jul 2025 10:44:38 -0400 Subject: [PATCH 4/7] add separate functions for copying and opening external links --- .../app/views/discover/table/cellAction.tsx | 74 +++++++++---------- .../explore/logs/tables/logsTableRow.tsx | 10 ++- 2 files changed, 44 insertions(+), 40 deletions(-) diff --git a/static/app/views/discover/table/cellAction.tsx b/static/app/views/discover/table/cellAction.tsx index f52847bc41cca9..6dd6becc1b57b2 100644 --- a/static/app/views/discover/table/cellAction.tsx +++ b/static/app/views/discover/table/cellAction.tsx @@ -87,12 +87,17 @@ 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.OPEN_EXTERNAL_LINK: + openExternalLink(value); + break; case Actions.RELEASE: case Actions.DRILLDOWN: break; default: - if (!handleCellActionFallback(action, value)) - throw new Error(`Unknown action type. ${action}`); + throw new Error(`Unknown action type. ${action}`); } } @@ -139,6 +144,35 @@ export function excludeFromFilter( oldFilter.addFilterValues(negation, value); } +/** + * Copies the provided value to a user's clipboard. + * @param value + */ +export function copyToClipboard(value: string | number | string[]) { + function stringifyValue(val: string | number | string[]): 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'); + }); +} + +/** + * If provided value is a valid url, opens the url in a new tab + * @param value + */ +export function openExternalLink(value: string | number | string[]) { + if (typeof value === 'string' && isUrl(value)) { + window.open(value, '_blank', 'noopener,noreferrer'); + } else { + addErrorMessage('Could not open link'); + } +} + type CellActionsOpts = { column: TableColumn; dataRow: TableDataRow; @@ -329,42 +363,6 @@ 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: string | number | string[]): 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')` diff --git a/static/app/views/explore/logs/tables/logsTableRow.tsx b/static/app/views/explore/logs/tables/logsTableRow.tsx index 67c9d9115968ee..6af429b4eeade5 100644 --- a/static/app/views/explore/logs/tables/logsTableRow.tsx +++ b/static/app/views/explore/logs/tables/logsTableRow.tsx @@ -17,7 +17,8 @@ import useOrganization from 'sentry/utils/useOrganization'; import useProjectFromId from 'sentry/utils/useProjectFromId'; import CellAction, { Actions, - handleCellActionFallback, + copyToClipboard, + openExternalLink, } from 'sentry/views/discover/table/cellAction'; import type {TableColumn} from 'sentry/views/discover/table/types'; import {AttributesTree} from 'sentry/views/explore/components/traceItemAttributes/attributesTree'; @@ -304,8 +305,13 @@ export const LogRowContent = memo(function LogRowContent({ negated: true, }); break; + case Actions.COPY_TO_CLIPBOARD: + copyToClipboard(value); + break; + case Actions.OPEN_EXTERNAL_LINK: + openExternalLink(value); + break; default: - handleCellActionFallback(actions, cellValue); break; } }} From 406cdee02f74b854ad712bd24af07aa0f66cfe64 Mon Sep 17 00:00:00 2001 From: lzhao-sentry Date: Tue, 22 Jul 2025 10:48:23 -0400 Subject: [PATCH 5/7] use cellValue in logsTable --- static/app/views/explore/logs/tables/logsTableRow.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/static/app/views/explore/logs/tables/logsTableRow.tsx b/static/app/views/explore/logs/tables/logsTableRow.tsx index 6af429b4eeade5..a142e214827ced 100644 --- a/static/app/views/explore/logs/tables/logsTableRow.tsx +++ b/static/app/views/explore/logs/tables/logsTableRow.tsx @@ -306,10 +306,10 @@ export const LogRowContent = memo(function LogRowContent({ }); break; case Actions.COPY_TO_CLIPBOARD: - copyToClipboard(value); + copyToClipboard(cellValue); break; case Actions.OPEN_EXTERNAL_LINK: - openExternalLink(value); + openExternalLink(cellValue); break; default: break; From 8deae8fbf77c037e75f5dadcf721a7b96837f1a8 Mon Sep 17 00:00:00 2001 From: lzhao-sentry Date: Thu, 24 Jul 2025 09:51:22 -0400 Subject: [PATCH 6/7] maintain ellipsis trigger type for logs --- .../app/views/discover/table/cellAction.tsx | 20 ++++++++++++++++--- .../explore/logs/tables/logsTableRow.tsx | 2 ++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/static/app/views/discover/table/cellAction.tsx b/static/app/views/discover/table/cellAction.tsx index 6dd6becc1b57b2..a4d31af9323a47 100644 --- a/static/app/views/discover/table/cellAction.tsx +++ b/static/app/views/discover/table/cellAction.tsx @@ -282,15 +282,29 @@ function makeCellActions({ return actions; } -type Props = React.PropsWithoutRef; +/** + * Potentially temporary as design and product need more time to determine how logs table should trigger the dropdown. + * Currently, the agreed default for every table should be bold hover. Logs is the only table to use the ellipsis trigger. + */ +export enum ActionTriggerType { + ELLIPSIS = 'ellipsis', + BOLD_HOVER = 'bold_hover', +} -function CellAction(props: Props) { +type Props = React.PropsWithoutRef & { + triggerType?: ActionTriggerType; +}; + +function CellAction({triggerType = ActionTriggerType.BOLD_HOVER, ...props}: Props) { const organization = useOrganization(); const {children, column} = props; const cellActions = makeCellActions(props); const align = fieldAlignment(column.key as string, column.type); - if (organization.features.includes('discover-cell-actions-v2')) + if ( + organization.features.includes('discover-cell-actions-v2') && + triggerType === ActionTriggerType.BOLD_HOVER + ) return ( {renderedField} From daba2ea31342b8d038e04fb6697c745d2bfa34af Mon Sep 17 00:00:00 2001 From: lzhao-sentry Date: Thu, 24 Jul 2025 10:13:55 -0400 Subject: [PATCH 7/7] filter out open external link if flag is off --- .../app/views/discover/table/cellAction.tsx | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/static/app/views/discover/table/cellAction.tsx b/static/app/views/discover/table/cellAction.tsx index a4d31af9323a47..2136887a1e3339 100644 --- a/static/app/views/discover/table/cellAction.tsx +++ b/static/app/views/discover/table/cellAction.tsx @@ -295,16 +295,25 @@ type Props = React.PropsWithoutRef & { triggerType?: ActionTriggerType; }; -function CellAction({triggerType = ActionTriggerType.BOLD_HOVER, ...props}: Props) { +function CellAction({ + triggerType = ActionTriggerType.BOLD_HOVER, + allowActions, + ...props +}: Props) { const organization = useOrganization(); const {children, column} = props; - const cellActions = makeCellActions(props); + + const useCellActionsV2 = organization.features.includes('discover-cell-actions-v2'); + let filteredActions = allowActions; + if (!useCellActionsV2) + filteredActions = filteredActions?.filter( + action => action !== Actions.OPEN_EXTERNAL_LINK + ); + + const cellActions = makeCellActions({...props, allowActions: filteredActions}); const align = fieldAlignment(column.key as string, column.type); - if ( - organization.features.includes('discover-cell-actions-v2') && - triggerType === ActionTriggerType.BOLD_HOVER - ) + if (useCellActionsV2 && triggerType === ActionTriggerType.BOLD_HOVER) return (