From b93a49692cd0a4999f938a8d8d6fcba196c02b02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Thu, 7 May 2026 14:53:14 -0400 Subject: [PATCH 01/16] feat(ourlogs): implement pinned logs with sticky header --- .../app/views/discover/table/cellAction.tsx | 17 ++- .../views/explore/logs/pinning/PinnedLogs.tsx | 88 +++++++++++++++ .../explore/logs/pinning/useLogsPinning.tsx | 103 ++++++++++++++++++ static/app/views/explore/logs/styles.tsx | 34 +++++- .../explore/logs/tables/logsInfiniteTable.tsx | 40 ++++++- .../explore/logs/tables/logsTableRow.tsx | 64 +++++++---- 6 files changed, 319 insertions(+), 27 deletions(-) create mode 100644 static/app/views/explore/logs/pinning/PinnedLogs.tsx create mode 100644 static/app/views/explore/logs/pinning/useLogsPinning.tsx diff --git a/static/app/views/discover/table/cellAction.tsx b/static/app/views/discover/table/cellAction.tsx index 05637018c5b6de..452eb52e8dc93d 100644 --- a/static/app/views/discover/table/cellAction.tsx +++ b/static/app/views/discover/table/cellAction.tsx @@ -332,11 +332,13 @@ export enum ActionTriggerType { } type Props = React.PropsWithoutRef> & { + pin?: React.ReactNode; triggerType?: ActionTriggerType; usePortalOnDropdown?: boolean; }; export function CellAction({ + pin, triggerType = ActionTriggerType.BOLD_HOVER, allowActions, usePortalOnDropdown, @@ -412,12 +414,16 @@ export function CellAction({ ) : ( children )} + {pin} ); } return ( - + {children} {cellActions?.length && ( )} + {pin} ); } -const Container = styled('div')` +const Container = styled('div')<{containsPin?: boolean}>` + --logsPinButtonArea: 2rem; position: relative; - width: 100%; + width: ${p => + p.containsPin + ? `calc(100% - var(--logsPinButtonArea) + ${p.theme.space.md})` + : `100%`}; height: 100%; display: flex; flex-direction: column; diff --git a/static/app/views/explore/logs/pinning/PinnedLogs.tsx b/static/app/views/explore/logs/pinning/PinnedLogs.tsx new file mode 100644 index 00000000000000..2d0c4e09559c93 --- /dev/null +++ b/static/app/views/explore/logs/pinning/PinnedLogs.tsx @@ -0,0 +1,88 @@ +import {Fragment, useEffect, useState} from 'react'; +import styled from '@emotion/styled'; + +import {Button} from '@sentry/scraps/button'; +import {Flex} from '@sentry/scraps/layout'; + +import {GridRow} from 'sentry/components/tables/gridEditable/styles'; +import {IconChevron, IconClose} from 'sentry/icons'; +import {t} from 'sentry/locale'; +import {TableBody} from 'sentry/views/explore/components/table'; +import {useLogsPinning} from 'sentry/views/explore/logs/pinning/useLogsPinning'; +import {OurLogKnownFieldKey} from 'sentry/views/explore/logs/types'; +import type {LogTableRowItem} from 'sentry/views/explore/logs/utils'; + +interface Props { + allRows: LogTableRowItem[]; + renderRow: (dataRow: LogTableRowItem) => React.ReactNode; +} + +export function PinnedLogs({allRows, renderRow}: Props) { + const logsPinning = useLogsPinning(); + const [expanded, setExpanded] = useState(true); + const pinsCount = logsPinning.pinnedRows.size; + + useEffect(() => { + if (!pinsCount) { + setExpanded(true); + } + }, [pinsCount]); + + if (!pinsCount) { + return null; + } + + return ( + + {expanded && + Array.from(logsPinning.pinnedRows).map(rowId => { + const dataRow = allRows.find(datum => datum[OurLogKnownFieldKey.ID] === rowId); + + // TODO: this is not correct yet because the virtualizer might not have found it yet. + // Will have to manually re-fetch data. + if (!dataRow) { + return null; + } + + return {renderRow(dataRow)}; + })} + + + + + + + + + + ); +} + +const PinnedTableBody = styled(TableBody)` + border-bottom: 1px solid ${p => p.theme.tokens.border.primary}; + height: max-content; + overflow-y: auto; + scrollbar-gutter: stable; + scrollbar-width: thin; +`; + +const PinnedGridBodyCell = styled('td')` + grid-column: 1 / -1; + padding: ${p => p.theme.space.sm}; +`; diff --git a/static/app/views/explore/logs/pinning/useLogsPinning.tsx b/static/app/views/explore/logs/pinning/useLogsPinning.tsx new file mode 100644 index 00000000000000..29ccaacf7af802 --- /dev/null +++ b/static/app/views/explore/logs/pinning/useLogsPinning.tsx @@ -0,0 +1,103 @@ +import type {ReactNode} from 'react'; +import { + createContext, + useCallback, + useContext, + useEffect, + useMemo, + useState, +} from 'react'; + +import {decodeList} from 'sentry/utils/queryString'; +import {useLocation} from 'sentry/utils/useLocation'; + +const LOGS_PINNED_KEY = 'logsPinned'; + +interface LogsPinning { + clearPinnedRows: () => void; + hoveringRow: string | undefined; + pinnedRows: Set; + togglePinnedRow: (id: string) => void; + updateHoveringRow: (hovering: boolean, rowId: string | undefined) => void; +} + +const LogsPinningContext = createContext(undefined); + +export function LogsPinningProvider({children}: {children: ReactNode}) { + const location = useLocation(); + + const [pinnedRows, setPinnedRows] = useState>(() => { + return new Set(decodeList(location.query?.[LOGS_PINNED_KEY]).filter(Boolean)); + }); + + const [hoveringRow, setHoveringRow] = useState(undefined); + + const updateHoveringRow = useCallback( + (hovering: boolean, rowId: string | undefined) => { + setHoveringRow(previous => { + if (rowId || !previous || previous === rowId) { + return hovering ? rowId : undefined; + } + return; + }); + }, + [] + ); + + const togglePinnedRow = useCallback((id: string) => { + setPinnedRows(previous => { + const next = new Set(previous); + if (next.has(id)) { + next.delete(id); + } else { + next.add(id); + } + return next; + }); + }, []); + + const clearPinnedRows = useCallback(() => { + setPinnedRows(new Set()); + }, []); + + useEffect(() => { + const params = new URLSearchParams(window.location.search); + params.delete(LOGS_PINNED_KEY); + + for (const id of pinnedRows) { + params.append(LOGS_PINNED_KEY, id); + } + + const newSearch = params.toString(); + const newUrl = newSearch + ? `${window.location.pathname}?${newSearch}${window.location.hash}` + : `${window.location.pathname}${window.location.hash}`; + + window.history.replaceState(window.history.state, '', newUrl); + }, [pinnedRows]); + + const value = useMemo( + () => ({ + clearPinnedRows, + hoveringRow, + pinnedRows, + updateHoveringRow, + togglePinnedRow, + }), + [clearPinnedRows, hoveringRow, pinnedRows, updateHoveringRow, togglePinnedRow] + ); + + return ( + {children} + ); +} + +export function useLogsPinning() { + const context = useContext(LogsPinningContext); + + if (!context) { + throw new Error('LogsPinningContext must be used within LogsPinningProvider'); + } + + return context; +} diff --git a/static/app/views/explore/logs/styles.tsx b/static/app/views/explore/logs/styles.tsx index bb6e8ec607b96a..f318b93a419628 100644 --- a/static/app/views/explore/logs/styles.tsx +++ b/static/app/views/explore/logs/styles.tsx @@ -66,6 +66,25 @@ export const LogTableRow = styled(TableRow)` } } + &[data-row-pinned='true']:not(thead > &) { + background-color: ${p => p.theme.tokens.background.transparent.accent.muted}; + + &:hover { + background-color: ${p => + p.theme.tokens.interactive.transparent.accent.selected.background.active}; + } + } + + &[data-row-hover-linked='true']:not(thead > &) { + background-color: ${p => + p.theme.tokens.interactive.transparent.accent.selected.background.active}; + + &:hover { + background-color: ${p => + p.theme.tokens.interactive.transparent.accent.selected.background.active}; + } + } + &.beforeHoverTime + &.afterHoverTime:before { border-top: 1px solid ${p => p.theme.tokens.border.accent.moderate}; content: ''; @@ -122,7 +141,7 @@ export const LogTableBodyCell = styled(TableBodyCell)` } &:last-child { - padding: 2px ${p => p.theme.space.xl}; + padding: 2px ${p => p.theme.space.md}; } `; @@ -545,3 +564,16 @@ export const TraceIconStyleWrapper = styled(Flex)` fill: #ffffff; } `; + +export const PinActionButton = styled(Button)<{isPinned: boolean}>` + position: absolute; + right: calc(-1 * var(--logsPinButtonArea)); + opacity: ${p => (p.isPinned ? 1 : 0)}; + transition: opacity 0.1s; + z-index: 1; + + ${LogTableRow}:focus-within, + ${LogTableRow}:hover & { + opacity: 1; + } +`; diff --git a/static/app/views/explore/logs/tables/logsInfiniteTable.tsx b/static/app/views/explore/logs/tables/logsInfiniteTable.tsx index 4a940bdf289c35..e656919367f0f9 100644 --- a/static/app/views/explore/logs/tables/logsInfiniteTable.tsx +++ b/static/app/views/explore/logs/tables/logsInfiniteTable.tsx @@ -45,6 +45,8 @@ import { MINIMUM_INFINITE_SCROLL_FETCH_COOLDOWN_MS, QUANTIZE_MINUTES, } from 'sentry/views/explore/logs/constants'; +import {PinnedLogs} from 'sentry/views/explore/logs/pinning/PinnedLogs'; +import {LogsPinningProvider} from 'sentry/views/explore/logs/pinning/useLogsPinning'; import { FirstTableHeadCell, FloatingBackToTopContainer, @@ -477,7 +479,7 @@ export function LogsInfiniteTable({ } return ( - + )} + {!isPending && ( + { + const pinnedId = dataRow[OurLogKnownFieldKey.ID]; + const pinnedExpandKey = `pinned-${pinnedId}`; + return ( + + ); + }} + /> + )} @@ -592,7 +626,7 @@ export function LogsInfiniteTable({ ) : null} - + ); } diff --git a/static/app/views/explore/logs/tables/logsTableRow.tsx b/static/app/views/explore/logs/tables/logsTableRow.tsx index 01829397f406f4..02129283bfef82 100644 --- a/static/app/views/explore/logs/tables/logsTableRow.tsx +++ b/static/app/views/explore/logs/tables/logsTableRow.tsx @@ -11,7 +11,7 @@ import {EmptyStreamWrapper} from 'sentry/components/emptyStateWarning'; import ProjectBadge from 'sentry/components/idBadge/projectBadge'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {useCaseInsensitivity} from 'sentry/components/searchQueryBuilder/hooks'; -import {IconAdd, IconJson, IconSubtract, IconWarning} from 'sentry/icons'; +import {IconAdd, IconJson, IconPin, IconSubtract, IconWarning} from 'sentry/icons'; import {IconChevron} from 'sentry/icons/iconChevron'; import {t} from 'sentry/locale'; import {defined} from 'sentry/utils'; @@ -57,6 +57,7 @@ import { } from 'sentry/views/explore/logs/fieldRenderers'; import {useLogsFrozenIsFrozen} from 'sentry/views/explore/logs/logsFrozenContext'; import {useLogsAnalyticsPageSource} from 'sentry/views/explore/logs/logsQueryParamsProvider'; +import {useLogsPinning} from 'sentry/views/explore/logs/pinning/useLogsPinning'; import { DetailsBody, DetailsContent, @@ -70,6 +71,7 @@ import { LogsTableBodyFirstCell, LogTableBodyCell, LogTableRow, + PinActionButton, StyledChevronButton, TraceIconStyleWrapper, } from 'sentry/views/explore/logs/styles'; @@ -99,6 +101,7 @@ import {TraceIcons} from 'sentry/views/performance/newTraceDetails/traceIcons'; type LogsRowProps = { dataRow: LogTableRowItem; + expansionKey: string; highlightTerms: string[]; meta: EventsMetaType | undefined; sharedHoverTimeoutRef: React.MutableRefObject; @@ -147,6 +150,7 @@ export const LogRowContent = memo(function LogRowContent({ sharedHoverTimeoutRef, isExpanded, onExpand, + expansionKey, onCollapse, onExpandHeight, blockRowExpanding, @@ -162,16 +166,19 @@ export const LogRowContent = memo(function LogRowContent({ const autorefreshEnabled = useLogsAutoRefreshEnabled(); const setAutorefresh = useSetLogsAutoRefresh(); const measureRef = useRef(null); - const [shouldRenderHoverElements, setShouldRenderHoverElements] = useState(false); + + const rowId = String(dataRow[OurLogKnownFieldKey.ID]); + const logsPinning = useLogsPinning(); + const isHoverLinked = logsPinning.hoveringRow === rowId; + const isPinned = logsPinning.pinnedRows.has(rowId); + + const [shouldRenderHoverElements, setShouldRenderHoverElements] = useState(isPinned); // This only applies in embedded views where clicking doesn't expand row details. function onClick(event: SyntheticEvent) { if (onEmbeddedRowClick && event.nativeEvent instanceof MouseEvent) { event.preventDefault(); - onEmbeddedRowClick( - String(dataRow[OurLogKnownFieldKey.ID]), - event as React.MouseEvent - ); + onEmbeddedRowClick(rowId, event as React.MouseEvent); return; } } @@ -206,9 +213,9 @@ export const LogRowContent = memo(function LogRowContent({ function toggleExpanded() { if (onExpand) { if (isExpanded) { - onCollapse?.(String(dataRow[OurLogKnownFieldKey.ID])); + onCollapse?.(expansionKey); } else { - onExpand?.(String(dataRow[OurLogKnownFieldKey.ID])); + onExpand?.(expansionKey); } } else { setExpanded(e => !e); @@ -218,7 +225,7 @@ export const LogRowContent = memo(function LogRowContent({ } trackAnalytics('logs.table.row_expanded', { - log_id: String(dataRow[OurLogKnownFieldKey.ID]), + log_id: rowId, page_source: analyticsPageSource, organization, }); @@ -226,12 +233,9 @@ export const LogRowContent = memo(function LogRowContent({ useLayoutEffect(() => { if (measureRef.current && isExpanded) { - onExpandHeight?.( - String(dataRow[OurLogKnownFieldKey.ID]), - measureRef.current.clientHeight - ); + onExpandHeight?.(rowId, measureRef.current.clientHeight); } - }, [isExpanded, onExpandHeight, dataRow]); + }, [isExpanded, onExpandHeight, rowId]); const addSearchFilter = useAddSearchFilter(); const theme = useTheme(); @@ -251,7 +255,7 @@ export const LogRowContent = memo(function LogRowContent({ ? DEFAULT_TRACE_ITEM_HOVER_TIMEOUT_WITH_AUTO_REFRESH : DEFAULT_TRACE_ITEM_HOVER_TIMEOUT; const {hoverProps, traceItemsResult} = useFetchTraceItemDetailsOnHover({ - traceItemId: String(dataRow[OurLogKnownFieldKey.ID]), + traceItemId: rowId, projectId: String(dataRow[OurLogKnownFieldKey.PROJECT_ID]), traceId: String(dataRow[OurLogKnownFieldKey.TRACE_ID]), traceItemType: TraceItemDataset.LOGS, @@ -327,13 +331,18 @@ export const LogRowContent = memo(function LogRowContent({ { setShouldRenderHoverElements(true); - if (rowInteractProps.onMouseEnter) { - rowInteractProps.onMouseEnter(e); - } + rowInteractProps.onMouseEnter?.(e); + logsPinning.updateHoveringRow(true, rowId); + }} + onMouseLeave={e => { + rowInteractProps.onMouseLeave?.(e); + logsPinning.updateHoveringRow(false, rowId); }} > @@ -370,11 +379,25 @@ export const LogRowContent = memo(function LogRowContent({ )} - {fields?.map(field => { + {fields?.map((field, index) => { + const pin = + index === fields.length - 1 ? ( + } + isPinned={isPinned} + onClick={(e: React.MouseEvent) => { + e.stopPropagation(); + logsPinning.togglePinnedRow(rowId); + }} + size="zero" + variant={isPinned ? 'primary' : undefined} + /> + ) : null; const value = (dataRow as OurLogsResponseItem)[field]; if (!defined(value)) { - return ; + return {pin}; } const renderedField = ( @@ -434,6 +457,7 @@ export const LogRowContent = memo(function LogRowContent({ } }} allowActions={ALLOWED_CELL_ACTIONS} + pin={pin} triggerType={ActionTriggerType.ELLIPSIS} > {renderedField} From 94ad61a7f27fad253ca741580166abf190a3e4ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Thu, 7 May 2026 15:06:42 -0400 Subject: [PATCH 02/16] added feature flag / query param usage --- static/app/views/explore/logs/pinning/PinnedLogs.tsx | 4 +++- .../app/views/explore/logs/tables/logsTableRow.tsx | 4 +++- .../explore/logs/tables/useOurLogsPinning.tsx.tsx | 12 ++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 static/app/views/explore/logs/tables/useOurLogsPinning.tsx.tsx diff --git a/static/app/views/explore/logs/pinning/PinnedLogs.tsx b/static/app/views/explore/logs/pinning/PinnedLogs.tsx index 2d0c4e09559c93..36f5aa2cba6250 100644 --- a/static/app/views/explore/logs/pinning/PinnedLogs.tsx +++ b/static/app/views/explore/logs/pinning/PinnedLogs.tsx @@ -9,6 +9,7 @@ import {IconChevron, IconClose} from 'sentry/icons'; import {t} from 'sentry/locale'; import {TableBody} from 'sentry/views/explore/components/table'; import {useLogsPinning} from 'sentry/views/explore/logs/pinning/useLogsPinning'; +import {useOurLogsPinningEnabled} from 'sentry/views/explore/logs/tables/useOurLogsPinning.tsx'; import {OurLogKnownFieldKey} from 'sentry/views/explore/logs/types'; import type {LogTableRowItem} from 'sentry/views/explore/logs/utils'; @@ -21,6 +22,7 @@ export function PinnedLogs({allRows, renderRow}: Props) { const logsPinning = useLogsPinning(); const [expanded, setExpanded] = useState(true); const pinsCount = logsPinning.pinnedRows.size; + const enabled = useOurLogsPinningEnabled(); useEffect(() => { if (!pinsCount) { @@ -28,7 +30,7 @@ export function PinnedLogs({allRows, renderRow}: Props) { } }, [pinsCount]); - if (!pinsCount) { + if (!enabled || !pinsCount) { return null; } diff --git a/static/app/views/explore/logs/tables/logsTableRow.tsx b/static/app/views/explore/logs/tables/logsTableRow.tsx index 02129283bfef82..4573894c914238 100644 --- a/static/app/views/explore/logs/tables/logsTableRow.tsx +++ b/static/app/views/explore/logs/tables/logsTableRow.tsx @@ -75,6 +75,7 @@ import { StyledChevronButton, TraceIconStyleWrapper, } from 'sentry/views/explore/logs/styles'; +import {useOurLogsPinningEnabled} from 'sentry/views/explore/logs/tables/useOurLogsPinning.tsx'; import { OurLogKnownFieldKey, type OurLogsResponseItem, @@ -171,6 +172,7 @@ export const LogRowContent = memo(function LogRowContent({ const logsPinning = useLogsPinning(); const isHoverLinked = logsPinning.hoveringRow === rowId; const isPinned = logsPinning.pinnedRows.has(rowId); + const logsPinningEnabled = useOurLogsPinningEnabled(); const [shouldRenderHoverElements, setShouldRenderHoverElements] = useState(isPinned); @@ -381,7 +383,7 @@ export const LogRowContent = memo(function LogRowContent({ {fields?.map((field, index) => { const pin = - index === fields.length - 1 ? ( + logsPinningEnabled && index === fields.length - 1 ? ( } diff --git a/static/app/views/explore/logs/tables/useOurLogsPinning.tsx.tsx b/static/app/views/explore/logs/tables/useOurLogsPinning.tsx.tsx new file mode 100644 index 00000000000000..17cbc6d3659d65 --- /dev/null +++ b/static/app/views/explore/logs/tables/useOurLogsPinning.tsx.tsx @@ -0,0 +1,12 @@ +import {useLocation} from 'sentry/utils/useLocation'; +import {useOrganization} from 'sentry/utils/useOrganization'; + +export function useOurLogsPinningEnabled() { + const organization = useOrganization(); + const location = useLocation(); + + return ( + organization.features.includes('ourlogs-pinning') || + location.query.logsPinning === 'true' + ); +} From 42cc1a86b6ca26d990ce4ff15dd33a78e053f87e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Fri, 8 May 2026 10:49:43 -0400 Subject: [PATCH 03/16] touchups: right actions; solid pin icons --- .../views/explore/logs/pinning/PinnedLogs.tsx | 6 ++--- .../useOurLogsPinning.tsx.tsx | 0 static/app/views/explore/logs/styles.tsx | 27 ++++++++++--------- .../explore/logs/tables/logsTableRow.tsx | 14 +++++----- 4 files changed, 24 insertions(+), 23 deletions(-) rename static/app/views/explore/logs/{tables => pinning}/useOurLogsPinning.tsx.tsx (100%) diff --git a/static/app/views/explore/logs/pinning/PinnedLogs.tsx b/static/app/views/explore/logs/pinning/PinnedLogs.tsx index 36f5aa2cba6250..623ef8eb9e0d93 100644 --- a/static/app/views/explore/logs/pinning/PinnedLogs.tsx +++ b/static/app/views/explore/logs/pinning/PinnedLogs.tsx @@ -9,7 +9,7 @@ import {IconChevron, IconClose} from 'sentry/icons'; import {t} from 'sentry/locale'; import {TableBody} from 'sentry/views/explore/components/table'; import {useLogsPinning} from 'sentry/views/explore/logs/pinning/useLogsPinning'; -import {useOurLogsPinningEnabled} from 'sentry/views/explore/logs/tables/useOurLogsPinning.tsx'; +import {useOurLogsPinningEnabled} from 'sentry/views/explore/logs/pinning/useOurLogsPinning.tsx'; import {OurLogKnownFieldKey} from 'sentry/views/explore/logs/types'; import type {LogTableRowItem} from 'sentry/views/explore/logs/utils'; @@ -40,7 +40,7 @@ export function PinnedLogs({allRows, renderRow}: Props) { Array.from(logsPinning.pinnedRows).map(rowId => { const dataRow = allRows.find(datum => datum[OurLogKnownFieldKey.ID] === rowId); - // TODO: this is not correct yet because the virtualizer might not have found it yet. + // TODO(LOGS-781): this is not correct yet because the virtualizer might not have found it yet. // Will have to manually re-fetch data. if (!dataRow) { return null; @@ -50,7 +50,7 @@ export function PinnedLogs({allRows, renderRow}: Props) { })} - + , + }); + + expect(screen.getByRole('button', {name: 'pin me'})).toBeInTheDocument(); + }); + + it('renders the pin element with the ellipsis trigger', () => { + renderComponent({ + eventView: view, + triggerType: ActionTriggerType.ELLIPSIS, + pin: , + }); + + expect(screen.getByRole('button', {name: 'pin me'})).toBeInTheDocument(); + }); + }); }); describe('updateQuery()', () => { diff --git a/static/app/views/explore/logs/pinning/PinnedLogs.spec.tsx b/static/app/views/explore/logs/pinning/PinnedLogs.spec.tsx new file mode 100644 index 00000000000000..19bcd94bccdaa8 --- /dev/null +++ b/static/app/views/explore/logs/pinning/PinnedLogs.spec.tsx @@ -0,0 +1,135 @@ +import {LogFixture} from 'sentry-fixture/log'; +import {OrganizationFixture} from 'sentry-fixture/organization'; + +import { + render, + screen, + userEvent, + type RenderOptions, +} from 'sentry-test/reactTestingLibrary'; + +import {PinnedLogs} from 'sentry/views/explore/logs/pinning/PinnedLogs'; +import {LogsPinningProvider} from 'sentry/views/explore/logs/pinning/useLogsPinning'; +import {OurLogKnownFieldKey} from 'sentry/views/explore/logs/types'; +import type {LogTableRowItem} from 'sentry/views/explore/logs/utils'; + +const allRows: LogTableRowItem[] = [ + LogFixture({ + [OurLogKnownFieldKey.ID]: 'log-1', + [OurLogKnownFieldKey.PROJECT_ID]: '1', + [OurLogKnownFieldKey.ORGANIZATION_ID]: 1, + [OurLogKnownFieldKey.MESSAGE]: 'first pinned log', + }), + LogFixture({ + [OurLogKnownFieldKey.ID]: 'log-2', + [OurLogKnownFieldKey.PROJECT_ID]: '1', + [OurLogKnownFieldKey.ORGANIZATION_ID]: 1, + [OurLogKnownFieldKey.MESSAGE]: 'second pinned log', + }), +]; + +const renderRow = (dataRow: LogTableRowItem) => ( + + {dataRow[OurLogKnownFieldKey.MESSAGE]} + +); + +function renderPinnedLogs(options: RenderOptions = {}) { + return render( + + + + +
, + { + organization: OrganizationFixture({features: ['ourlogs-pinning']}), + ...options, + } + ); +} + +describe('PinnedLogs', () => { + it('renders nothing when the feature is disabled even if rows are pinned', () => { + renderPinnedLogs({ + organization: OrganizationFixture({features: []}), + initialRouterConfig: { + location: {pathname: '/', query: {logsPinned: 'log-1'}}, + }, + }); + + expect(screen.queryByRole('toolbar')).not.toBeInTheDocument(); + }); + + it('renders nothing when no rows are pinned', () => { + renderPinnedLogs(); + + expect(screen.queryByRole('toolbar')).not.toBeInTheDocument(); + }); + + it('renders the pinned row when its id is present in allRows', () => { + renderPinnedLogs({ + initialRouterConfig: { + location: {pathname: '/', query: {logsPinned: 'log-1'}}, + }, + }); + + expect(screen.getByTestId('pinned-row-log-1')).toBeInTheDocument(); + }); + + it('does not render a row when the pinned id is missing from allRows', () => { + renderPinnedLogs({ + initialRouterConfig: { + location: {pathname: '/', query: {logsPinned: 'missing-log'}}, + }, + }); + + expect(screen.queryByTestId('pinned-row-missing-log')).not.toBeInTheDocument(); + }); + + it('shows the count of pinned rows in the collapse toggle label', () => { + renderPinnedLogs({ + initialRouterConfig: { + location: {pathname: '/', query: {logsPinned: ['log-1', 'log-2']}}, + }, + }); + + expect(screen.getByRole('button', {name: 'Collapse 2 pinned'})).toBeInTheDocument(); + }); + + it('hides the rendered pinned rows when the collapse button is clicked', async () => { + renderPinnedLogs({ + initialRouterConfig: { + location: {pathname: '/', query: {logsPinned: 'log-1'}}, + }, + }); + + await userEvent.click(screen.getByRole('button', {name: 'Collapse 1 pinned'})); + + expect(screen.queryByTestId('pinned-row-log-1')).not.toBeInTheDocument(); + }); + + it('shows the rendered pinned rows again when the toggle button is clicked twice', async () => { + renderPinnedLogs({ + initialRouterConfig: { + location: {pathname: '/', query: {logsPinned: 'log-1'}}, + }, + }); + + await userEvent.click(screen.getByRole('button', {name: 'Collapse 1 pinned'})); + await userEvent.click(screen.getByRole('button', {name: 'Expand 1 pinned'})); + + expect(screen.getByTestId('pinned-row-log-1')).toBeInTheDocument(); + }); + + it('removes the rendered pinned rows when the Clear all button is clicked', async () => { + renderPinnedLogs({ + initialRouterConfig: { + location: {pathname: '/', query: {logsPinned: 'log-1'}}, + }, + }); + + await userEvent.click(screen.getByRole('button', {name: 'Clear all pins'})); + + expect(screen.queryByTestId('pinned-row-log-1')).not.toBeInTheDocument(); + }); +}); diff --git a/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx b/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx new file mode 100644 index 00000000000000..5e920182365e8c --- /dev/null +++ b/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx @@ -0,0 +1,186 @@ +import {act, renderHookWithProviders} from 'sentry-test/reactTestingLibrary'; + +import { + LogsPinningProvider, + useLogsPinning, +} from 'sentry/views/explore/logs/pinning/useLogsPinning'; + +let replaceStateSpy: jest.SpyInstance; + +beforeEach(() => { + replaceStateSpy = jest.spyOn(window.history, 'replaceState'); +}); + +afterEach(() => { + replaceStateSpy.mockRestore(); +}); + +describe('useLogsPinning', () => { + it('throws when no LogsPinningProvider wraps the consumer', () => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + expect(() => renderHookWithProviders(() => useLogsPinning())).toThrow( + 'LogsPinningContext must be used within LogsPinningProvider' + ); + + consoleErrorSpy.mockRestore(); + }); + + it('starts with an empty pinnedRows when the location has no logsPinned query', () => { + const {result} = renderHookWithProviders(() => useLogsPinning(), { + additionalWrapper: LogsPinningProvider, + }); + + expect(result.current.pinnedRows).toEqual(new Set()); + }); + + it('starts with a single id in pinnedRows when the location has a single logsPinned value', () => { + const {result} = renderHookWithProviders(() => useLogsPinning(), { + additionalWrapper: LogsPinningProvider, + initialRouterConfig: { + location: {pathname: '/', query: {logsPinned: 'log-1'}}, + }, + }); + + expect(result.current.pinnedRows).toEqual(new Set(['log-1'])); + }); + + it('starts with multiple ids in pinnedRows when the location has multiple logsPinned values', () => { + const {result} = renderHookWithProviders(() => useLogsPinning(), { + additionalWrapper: LogsPinningProvider, + initialRouterConfig: { + location: {pathname: '/', query: {logsPinned: ['log-1', 'log-2']}}, + }, + }); + + expect(result.current.pinnedRows).toEqual(new Set(['log-1', 'log-2'])); + }); + + it('filters out empty values from the logsPinned query when initializing pinnedRows', () => { + const {result} = renderHookWithProviders(() => useLogsPinning(), { + additionalWrapper: LogsPinningProvider, + initialRouterConfig: { + location: {pathname: '/', query: {logsPinned: ['log-1', '']}}, + }, + }); + + expect(result.current.pinnedRows).toEqual(new Set(['log-1'])); + }); + + it('adds the id to pinnedRows when togglePinnedRow is called for an unpinned id', () => { + const {result} = renderHookWithProviders(() => useLogsPinning(), { + additionalWrapper: LogsPinningProvider, + }); + + act(() => { + result.current.togglePinnedRow('log-1'); + }); + + expect(result.current.pinnedRows).toEqual(new Set(['log-1'])); + }); + + it('removes the id from pinnedRows when togglePinnedRow is called for a pinned id', () => { + const {result} = renderHookWithProviders(() => useLogsPinning(), { + additionalWrapper: LogsPinningProvider, + initialRouterConfig: { + location: {pathname: '/', query: {logsPinned: 'log-1'}}, + }, + }); + + act(() => { + result.current.togglePinnedRow('log-1'); + }); + + expect(result.current.pinnedRows).toEqual(new Set()); + }); + + it('empties pinnedRows when clearPinnedRows is called', () => { + const {result} = renderHookWithProviders(() => useLogsPinning(), { + additionalWrapper: LogsPinningProvider, + initialRouterConfig: { + location: {pathname: '/', query: {logsPinned: ['log-1', 'log-2']}}, + }, + }); + + act(() => { + result.current.clearPinnedRows(); + }); + + expect(result.current.pinnedRows).toEqual(new Set()); + }); + + it('writes the pinned id to the URL query string when togglePinnedRow is called', () => { + const {result} = renderHookWithProviders(() => useLogsPinning(), { + additionalWrapper: LogsPinningProvider, + }); + + replaceStateSpy.mockClear(); + + act(() => { + result.current.togglePinnedRow('log-1'); + }); + + const lastCall = replaceStateSpy.mock.calls.at(-1); + expect(lastCall?.[2]).toContain('logsPinned=log-1'); + }); + + it('removes the logsPinned key from the URL when clearPinnedRows is called', () => { + const {result} = renderHookWithProviders(() => useLogsPinning(), { + additionalWrapper: LogsPinningProvider, + initialRouterConfig: { + location: {pathname: '/', query: {logsPinned: 'log-1'}}, + }, + }); + + replaceStateSpy.mockClear(); + + act(() => { + result.current.clearPinnedRows(); + }); + + const lastCall = replaceStateSpy.mock.calls.at(-1); + expect(lastCall?.[2]).not.toContain('logsPinned'); + }); + + it('sets hoveringRow when updateHoveringRow is called and no row is currently hovered', () => { + const {result} = renderHookWithProviders(() => useLogsPinning(), { + additionalWrapper: LogsPinningProvider, + }); + + act(() => { + result.current.updateHoveringRow(true, 'log-1'); + }); + + expect(result.current.hoveringRow).toBe('log-1'); + }); + + it('clears hoveringRow when updateHoveringRow is called for the same hovered row leaving', () => { + const {result} = renderHookWithProviders(() => useLogsPinning(), { + additionalWrapper: LogsPinningProvider, + }); + + act(() => { + result.current.updateHoveringRow(true, 'log-1'); + }); + act(() => { + result.current.updateHoveringRow(false, 'log-1'); + }); + + expect(result.current.hoveringRow).toBeUndefined(); + }); + + it('clears hoveringRow when updateHoveringRow is called for a different row while one is already hovered', () => { + const {result} = renderHookWithProviders(() => useLogsPinning(), { + additionalWrapper: LogsPinningProvider, + }); + + act(() => { + result.current.updateHoveringRow(true, 'log-1'); + }); + act(() => { + result.current.updateHoveringRow(true, 'log-2'); + }); + + expect(result.current.hoveringRow).toBeUndefined(); + }); +}); diff --git a/static/app/views/explore/logs/pinning/useOurLogsPinning.spec.tsx b/static/app/views/explore/logs/pinning/useOurLogsPinning.spec.tsx new file mode 100644 index 00000000000000..86b283f4cadaa2 --- /dev/null +++ b/static/app/views/explore/logs/pinning/useOurLogsPinning.spec.tsx @@ -0,0 +1,48 @@ +import {OrganizationFixture} from 'sentry-fixture/organization'; + +import {renderHookWithProviders} from 'sentry-test/reactTestingLibrary'; + +import {useOurLogsPinningEnabled} from 'sentry/views/explore/logs/pinning/useOurLogsPinning'; + +describe('useOurLogsPinningEnabled', () => { + it('returns true when the organization has the ourlogs-pinning feature', () => { + const {result} = renderHookWithProviders(() => useOurLogsPinningEnabled(), { + organization: OrganizationFixture({features: ['ourlogs-pinning']}), + }); + + expect(result.current).toBe(true); + }); + + it('returns true when the location query has logsPinning set to true', () => { + const {result} = renderHookWithProviders(() => useOurLogsPinningEnabled(), { + organization: OrganizationFixture({features: []}), + initialRouterConfig: { + location: {pathname: '/', query: {logsPinning: 'true'}}, + }, + }); + + expect(result.current).toBe(true); + }); + + it('returns false when neither the feature nor the query are set', () => { + const {result} = renderHookWithProviders(() => useOurLogsPinningEnabled(), { + organization: OrganizationFixture({features: []}), + initialRouterConfig: { + location: {pathname: '/'}, + }, + }); + + expect(result.current).toBe(false); + }); + + it('returns false when the location query has logsPinning set to a value other than true', () => { + const {result} = renderHookWithProviders(() => useOurLogsPinningEnabled(), { + organization: OrganizationFixture({features: []}), + initialRouterConfig: { + location: {pathname: '/', query: {logsPinning: 'false'}}, + }, + }); + + expect(result.current).toBe(false); + }); +}); diff --git a/static/app/views/explore/logs/tables/logsInfiniteTable.spec.tsx b/static/app/views/explore/logs/tables/logsInfiniteTable.spec.tsx index cc848224584795..1c8856528df1fa 100644 --- a/static/app/views/explore/logs/tables/logsInfiniteTable.spec.tsx +++ b/static/app/views/explore/logs/tables/logsInfiniteTable.spec.tsx @@ -264,11 +264,7 @@ describe('LogsInfiniteTable', () => { const actionsButton = within(cell).queryByRole('button', { name: 'Actions', }); - if (field === 'timestamp') { - expect(actionsButton).toBeNull(); - } else { - expect(actionsButton).toBeInTheDocument(); - } + expect(actionsButton).toBeInTheDocument(); } } for (const mock of traceItemMocks) { @@ -476,4 +472,65 @@ describe('LogsInfiniteTable', () => { await screen.findByText('abc123de'); await screen.findByText('abc123ee'); }); + + it('renders a pin button on a hovered row when ourlogs-pinning is enabled', async () => { + mockUseLocation.mockReturnValue( + LocationFixture({ + pathname: `/organizations/${organization.slug}/explore/logs/?end=2025-04-10T20%3A04%3A51&project=${project.id}&start=2025-04-10T14%3A37%3A55`, + query: { + [LOGS_FIELDS_KEY]: visibleColumnFields, + [LOGS_SORT_BYS_KEY]: '-timestamp', + [LOGS_QUERY_KEY]: 'severity:error', + logsPinning: 'true', + }, + }) + ); + + renderWithProviders( + + ); + + const [firstRow] = await screen.findAllByTestId('log-table-row'); + await userEvent.hover(firstRow!); + + expect( + await within(firstRow!).findByRole('button', {name: 'Pin log row'}) + ).toBeInTheDocument(); + }); + + it('does not render a pin button when ourlogs-pinning is disabled', async () => { + renderWithProviders( + + ); + + const [firstRow] = await screen.findAllByTestId('log-table-row'); + await userEvent.hover(firstRow!); + + expect( + within(firstRow!).queryByRole('button', {name: 'Pin log row'}) + ).not.toBeInTheDocument(); + }); + + it('marks the row as pinned when its id is in the logsPinned query', async () => { + mockUseLocation.mockReturnValue( + LocationFixture({ + pathname: `/organizations/${organization.slug}/explore/logs/?end=2025-04-10T20%3A04%3A51&project=${project.id}&start=2025-04-10T14%3A37%3A55`, + query: { + [LOGS_FIELDS_KEY]: visibleColumnFields, + [LOGS_SORT_BYS_KEY]: '-timestamp', + [LOGS_QUERY_KEY]: 'severity:error', + logsPinning: 'true', + logsPinned: '1', + }, + }) + ); + + renderWithProviders( + + ); + + const [firstRow] = await screen.findAllByTestId('log-table-row'); + + expect(firstRow).toHaveAttribute('data-row-pinned', 'true'); + }); }); diff --git a/tests/js/sentry-test/reactTestingLibrary.tsx b/tests/js/sentry-test/reactTestingLibrary.tsx index 8fea9514de5f14..c71de455d71b6d 100644 --- a/tests/js/sentry-test/reactTestingLibrary.tsx +++ b/tests/js/sentry-test/reactTestingLibrary.tsx @@ -90,7 +90,7 @@ export interface RouterConfig { routes?: string[]; } -interface RenderOptions extends rtl.RenderOptions, ProviderOptions { +export interface RenderOptions extends rtl.RenderOptions, ProviderOptions { initialRouterConfig?: RouterConfig; outletContext?: Record; } From 49713dfa666fb7749ec4a1925b803090d79a5603 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Fri, 8 May 2026 12:13:11 -0400 Subject: [PATCH 12/16] fix: add containsPin to BOLD_HOVER container --- static/app/views/discover/table/cellAction.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/static/app/views/discover/table/cellAction.tsx b/static/app/views/discover/table/cellAction.tsx index 452eb52e8dc93d..c2645a9cea0735 100644 --- a/static/app/views/discover/table/cellAction.tsx +++ b/static/app/views/discover/table/cellAction.tsx @@ -359,6 +359,7 @@ export function CellAction({ if (triggerType === ActionTriggerType.BOLD_HOVER) { return ( {cellActions?.length ? ( From 21d96c4454312921efd1997c438909df9a748e24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Fri, 8 May 2026 12:45:27 -0400 Subject: [PATCH 13/16] Make it not do anything without context --- .../views/explore/logs/pinning/PinnedLogs.tsx | 6 +-- .../logs/pinning/useLogsPinning.spec.tsx | 54 +++++++++---------- .../explore/logs/pinning/useLogsPinning.tsx | 8 ++- static/app/views/explore/logs/styles.tsx | 2 +- .../explore/logs/tables/logsTableRow.tsx | 12 ++--- 5 files changed, 38 insertions(+), 44 deletions(-) diff --git a/static/app/views/explore/logs/pinning/PinnedLogs.tsx b/static/app/views/explore/logs/pinning/PinnedLogs.tsx index 5e469e5c68b1ff..a3938975a9d298 100644 --- a/static/app/views/explore/logs/pinning/PinnedLogs.tsx +++ b/static/app/views/explore/logs/pinning/PinnedLogs.tsx @@ -9,7 +9,6 @@ import {IconChevron, IconClose} from 'sentry/icons'; import {t} from 'sentry/locale'; import {TableBody} from 'sentry/views/explore/components/table'; import {useLogsPinning} from 'sentry/views/explore/logs/pinning/useLogsPinning'; -import {useOurLogsPinningEnabled} from 'sentry/views/explore/logs/pinning/useOurLogsPinning'; import {OurLogKnownFieldKey} from 'sentry/views/explore/logs/types'; import type {LogTableRowItem} from 'sentry/views/explore/logs/utils'; @@ -21,8 +20,7 @@ interface Props { export function PinnedLogs({allRows, renderRow}: Props) { const logsPinning = useLogsPinning(); const [expanded, setExpanded] = useState(true); - const pinsCount = logsPinning.pinnedRows.size; - const enabled = useOurLogsPinningEnabled(); + const pinsCount = logsPinning?.pinnedRows.size; useEffect(() => { if (!pinsCount) { @@ -30,7 +28,7 @@ export function PinnedLogs({allRows, renderRow}: Props) { } }, [pinsCount]); - if (!enabled || !pinsCount) { + if (!logsPinning || !pinsCount) { return null; } diff --git a/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx b/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx index 5e920182365e8c..6c12b641c9946a 100644 --- a/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx +++ b/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx @@ -15,15 +15,15 @@ afterEach(() => { replaceStateSpy.mockRestore(); }); -describe('useLogsPinning', () => { - it('throws when no LogsPinningProvider wraps the consumer', () => { - const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); +jest.mock('sentry/views/explore/logs/pinning/useOurLogsPinning', () => ({ + useOurLogsPinningEnabled: () => true, +})); - expect(() => renderHookWithProviders(() => useLogsPinning())).toThrow( - 'LogsPinningContext must be used within LogsPinningProvider' - ); +describe('useLogsPinning', () => { + it('returns undefined when no LogsPinningProvider wraps the consumer', () => { + const {result} = renderHookWithProviders(() => useLogsPinning()); - consoleErrorSpy.mockRestore(); + expect(result.current).toBeUndefined(); }); it('starts with an empty pinnedRows when the location has no logsPinned query', () => { @@ -31,7 +31,7 @@ describe('useLogsPinning', () => { additionalWrapper: LogsPinningProvider, }); - expect(result.current.pinnedRows).toEqual(new Set()); + expect(result.current?.pinnedRows).toEqual(new Set()); }); it('starts with a single id in pinnedRows when the location has a single logsPinned value', () => { @@ -42,7 +42,7 @@ describe('useLogsPinning', () => { }, }); - expect(result.current.pinnedRows).toEqual(new Set(['log-1'])); + expect(result.current?.pinnedRows).toEqual(new Set(['log-1'])); }); it('starts with multiple ids in pinnedRows when the location has multiple logsPinned values', () => { @@ -53,7 +53,7 @@ describe('useLogsPinning', () => { }, }); - expect(result.current.pinnedRows).toEqual(new Set(['log-1', 'log-2'])); + expect(result.current?.pinnedRows).toEqual(new Set(['log-1', 'log-2'])); }); it('filters out empty values from the logsPinned query when initializing pinnedRows', () => { @@ -64,7 +64,7 @@ describe('useLogsPinning', () => { }, }); - expect(result.current.pinnedRows).toEqual(new Set(['log-1'])); + expect(result.current?.pinnedRows).toEqual(new Set(['log-1'])); }); it('adds the id to pinnedRows when togglePinnedRow is called for an unpinned id', () => { @@ -73,10 +73,10 @@ describe('useLogsPinning', () => { }); act(() => { - result.current.togglePinnedRow('log-1'); + result.current?.togglePinnedRow('log-1'); }); - expect(result.current.pinnedRows).toEqual(new Set(['log-1'])); + expect(result.current?.pinnedRows).toEqual(new Set(['log-1'])); }); it('removes the id from pinnedRows when togglePinnedRow is called for a pinned id', () => { @@ -88,10 +88,10 @@ describe('useLogsPinning', () => { }); act(() => { - result.current.togglePinnedRow('log-1'); + result.current?.togglePinnedRow('log-1'); }); - expect(result.current.pinnedRows).toEqual(new Set()); + expect(result.current?.pinnedRows).toEqual(new Set()); }); it('empties pinnedRows when clearPinnedRows is called', () => { @@ -103,10 +103,10 @@ describe('useLogsPinning', () => { }); act(() => { - result.current.clearPinnedRows(); + result.current?.clearPinnedRows(); }); - expect(result.current.pinnedRows).toEqual(new Set()); + expect(result.current?.pinnedRows).toEqual(new Set()); }); it('writes the pinned id to the URL query string when togglePinnedRow is called', () => { @@ -117,7 +117,7 @@ describe('useLogsPinning', () => { replaceStateSpy.mockClear(); act(() => { - result.current.togglePinnedRow('log-1'); + result.current?.togglePinnedRow('log-1'); }); const lastCall = replaceStateSpy.mock.calls.at(-1); @@ -135,7 +135,7 @@ describe('useLogsPinning', () => { replaceStateSpy.mockClear(); act(() => { - result.current.clearPinnedRows(); + result.current?.clearPinnedRows(); }); const lastCall = replaceStateSpy.mock.calls.at(-1); @@ -148,10 +148,10 @@ describe('useLogsPinning', () => { }); act(() => { - result.current.updateHoveringRow(true, 'log-1'); + result.current?.updateHoveringRow(true, 'log-1'); }); - expect(result.current.hoveringRow).toBe('log-1'); + expect(result.current?.hoveringRow).toBe('log-1'); }); it('clears hoveringRow when updateHoveringRow is called for the same hovered row leaving', () => { @@ -160,13 +160,13 @@ describe('useLogsPinning', () => { }); act(() => { - result.current.updateHoveringRow(true, 'log-1'); + result.current?.updateHoveringRow(true, 'log-1'); }); act(() => { - result.current.updateHoveringRow(false, 'log-1'); + result.current?.updateHoveringRow(false, 'log-1'); }); - expect(result.current.hoveringRow).toBeUndefined(); + expect(result.current?.hoveringRow).toBeUndefined(); }); it('clears hoveringRow when updateHoveringRow is called for a different row while one is already hovered', () => { @@ -175,12 +175,12 @@ describe('useLogsPinning', () => { }); act(() => { - result.current.updateHoveringRow(true, 'log-1'); + result.current?.updateHoveringRow(true, 'log-1'); }); act(() => { - result.current.updateHoveringRow(true, 'log-2'); + result.current?.updateHoveringRow(true, 'log-2'); }); - expect(result.current.hoveringRow).toBeUndefined(); + expect(result.current?.hoveringRow).toBeUndefined(); }); }); diff --git a/static/app/views/explore/logs/pinning/useLogsPinning.tsx b/static/app/views/explore/logs/pinning/useLogsPinning.tsx index b9cc0399c1f996..3594cb5df4a803 100644 --- a/static/app/views/explore/logs/pinning/useLogsPinning.tsx +++ b/static/app/views/explore/logs/pinning/useLogsPinning.tsx @@ -10,6 +10,7 @@ import { import {decodeList} from 'sentry/utils/queryString'; import {useLocation} from 'sentry/utils/useLocation'; +import {useOurLogsPinningEnabled} from 'sentry/views/explore/logs/pinning/useOurLogsPinning'; const LOGS_PINNED_KEY = 'logsPinned'; @@ -90,11 +91,8 @@ export function LogsPinningProvider({children}: {children: ReactNode}) { } export function useLogsPinning() { + const logsPinningEnabled = useOurLogsPinningEnabled(); const context = useContext(LogsPinningContext); - if (!context) { - throw new Error('LogsPinningContext must be used within LogsPinningProvider'); - } - - return context; + return logsPinningEnabled ? context : undefined; } diff --git a/static/app/views/explore/logs/styles.tsx b/static/app/views/explore/logs/styles.tsx index 80b363c97ebe0b..1c10ada102580d 100644 --- a/static/app/views/explore/logs/styles.tsx +++ b/static/app/views/explore/logs/styles.tsx @@ -308,7 +308,7 @@ export const LogsFilteredHelperText = styled('span')` background-color: ${p => p.theme.colors.gray200}; `; -export const LogPinButton = styled(Button)<{isPinned: boolean}>` +export const LogPinButton = styled(Button)<{isPinned: boolean | undefined}>` position: absolute; right: calc(-1 * var(--logsPinButtonArea)); opacity: ${p => (p.isPinned ? 1 : 0)}; diff --git a/static/app/views/explore/logs/tables/logsTableRow.tsx b/static/app/views/explore/logs/tables/logsTableRow.tsx index 52bd97a7848908..44f4943c7a1fd7 100644 --- a/static/app/views/explore/logs/tables/logsTableRow.tsx +++ b/static/app/views/explore/logs/tables/logsTableRow.tsx @@ -58,7 +58,6 @@ import { import {useLogsFrozenIsFrozen} from 'sentry/views/explore/logs/logsFrozenContext'; import {useLogsAnalyticsPageSource} from 'sentry/views/explore/logs/logsQueryParamsProvider'; import {useLogsPinning} from 'sentry/views/explore/logs/pinning/useLogsPinning'; -import {useOurLogsPinningEnabled} from 'sentry/views/explore/logs/pinning/useOurLogsPinning'; import { DetailsBody, DetailsContent, @@ -171,9 +170,8 @@ export const LogRowContent = memo(function LogRowContent({ const rowId = String(dataRow[OurLogKnownFieldKey.ID]); const expansionKey = expansionKeyProp ?? rowId; const logsPinning = useLogsPinning(); - const isHoverLinked = logsPinning.hoveringRow === rowId; - const isPinned = logsPinning.pinnedRows.has(rowId); - const logsPinningEnabled = useOurLogsPinningEnabled(); + const isHoverLinked = logsPinning?.hoveringRow === rowId; + const isPinned = logsPinning?.pinnedRows.has(rowId); const [shouldRenderHoverElements, setShouldRenderHoverElements] = useState(isPinned); @@ -341,11 +339,11 @@ export const LogRowContent = memo(function LogRowContent({ onMouseEnter={e => { setShouldRenderHoverElements(true); rowInteractProps.onMouseEnter?.(e); - logsPinning.updateHoveringRow(true, rowId); + logsPinning?.updateHoveringRow(true, rowId); }} onMouseLeave={e => { rowInteractProps.onMouseLeave?.(e); - logsPinning.updateHoveringRow(false, rowId); + logsPinning?.updateHoveringRow(false, rowId); }} > @@ -384,7 +382,7 @@ export const LogRowContent = memo(function LogRowContent({ {fields?.map((field, index) => { const pin = - logsPinningEnabled && index === fields.length - 1 ? ( + logsPinning && index === fields.length - 1 ? ( Date: Wed, 13 May 2026 09:39:58 -0400 Subject: [PATCH 14/16] Remove linked hovering --- .../logs/pinning/useLogsPinning.spec.tsx | 42 ------------------- .../explore/logs/pinning/useLogsPinning.tsx | 17 +------- static/app/views/explore/logs/styles.tsx | 10 ----- .../explore/logs/tables/logsInfiniteTable.tsx | 3 -- .../explore/logs/tables/logsTableRow.tsx | 4 -- 5 files changed, 1 insertion(+), 75 deletions(-) diff --git a/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx b/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx index 6c12b641c9946a..776b89d2128c9f 100644 --- a/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx +++ b/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx @@ -141,46 +141,4 @@ describe('useLogsPinning', () => { const lastCall = replaceStateSpy.mock.calls.at(-1); expect(lastCall?.[2]).not.toContain('logsPinned'); }); - - it('sets hoveringRow when updateHoveringRow is called and no row is currently hovered', () => { - const {result} = renderHookWithProviders(() => useLogsPinning(), { - additionalWrapper: LogsPinningProvider, - }); - - act(() => { - result.current?.updateHoveringRow(true, 'log-1'); - }); - - expect(result.current?.hoveringRow).toBe('log-1'); - }); - - it('clears hoveringRow when updateHoveringRow is called for the same hovered row leaving', () => { - const {result} = renderHookWithProviders(() => useLogsPinning(), { - additionalWrapper: LogsPinningProvider, - }); - - act(() => { - result.current?.updateHoveringRow(true, 'log-1'); - }); - act(() => { - result.current?.updateHoveringRow(false, 'log-1'); - }); - - expect(result.current?.hoveringRow).toBeUndefined(); - }); - - it('clears hoveringRow when updateHoveringRow is called for a different row while one is already hovered', () => { - const {result} = renderHookWithProviders(() => useLogsPinning(), { - additionalWrapper: LogsPinningProvider, - }); - - act(() => { - result.current?.updateHoveringRow(true, 'log-1'); - }); - act(() => { - result.current?.updateHoveringRow(true, 'log-2'); - }); - - expect(result.current?.hoveringRow).toBeUndefined(); - }); }); diff --git a/static/app/views/explore/logs/pinning/useLogsPinning.tsx b/static/app/views/explore/logs/pinning/useLogsPinning.tsx index 3594cb5df4a803..a997a267830c3d 100644 --- a/static/app/views/explore/logs/pinning/useLogsPinning.tsx +++ b/static/app/views/explore/logs/pinning/useLogsPinning.tsx @@ -16,10 +16,8 @@ const LOGS_PINNED_KEY = 'logsPinned'; interface LogsPinning { clearPinnedRows: () => void; - hoveringRow: string | undefined; pinnedRows: Set; togglePinnedRow: (id: string) => void; - updateHoveringRow: (hovering: boolean, rowId: string) => void; } const LogsPinningContext = createContext(undefined); @@ -31,17 +29,6 @@ export function LogsPinningProvider({children}: {children: ReactNode}) { return new Set(decodeList(location.query?.[LOGS_PINNED_KEY]).filter(Boolean)); }); - const [hoveringRow, setHoveringRow] = useState(undefined); - - const updateHoveringRow = useCallback((hovering: boolean, rowId: string) => { - setHoveringRow(previous => { - if (!previous || previous === rowId) { - return hovering ? rowId : undefined; - } - return; - }); - }, []); - const togglePinnedRow = useCallback((id: string) => { setPinnedRows(previous => { const next = new Set(previous); @@ -77,12 +64,10 @@ export function LogsPinningProvider({children}: {children: ReactNode}) { const value = useMemo( () => ({ clearPinnedRows, - hoveringRow, pinnedRows, - updateHoveringRow, togglePinnedRow, }), - [clearPinnedRows, hoveringRow, pinnedRows, updateHoveringRow, togglePinnedRow] + [clearPinnedRows, pinnedRows, togglePinnedRow] ); return ( diff --git a/static/app/views/explore/logs/styles.tsx b/static/app/views/explore/logs/styles.tsx index 1c10ada102580d..38a3952a6ad781 100644 --- a/static/app/views/explore/logs/styles.tsx +++ b/static/app/views/explore/logs/styles.tsx @@ -78,16 +78,6 @@ export const LogTableRow = styled(TableRow)` } } - &[data-row-hover-linked='true']:not(thead > &) { - background-color: ${p => - p.theme.tokens.interactive.transparent.accent.selected.background.active}; - - &:hover { - background-color: ${p => - p.theme.tokens.interactive.transparent.accent.selected.background.active}; - } - } - &.beforeHoverTime + &.afterHoverTime:before { border-top: 1px solid ${p => p.theme.tokens.border.accent.moderate}; content: ''; diff --git a/static/app/views/explore/logs/tables/logsInfiniteTable.tsx b/static/app/views/explore/logs/tables/logsInfiniteTable.tsx index e656919367f0f9..d86d763b465097 100644 --- a/static/app/views/explore/logs/tables/logsInfiniteTable.tsx +++ b/static/app/views/explore/logs/tables/logsInfiniteTable.tsx @@ -502,9 +502,6 @@ export function LogsInfiniteTable({ {!isPending && ( { const pinnedId = dataRow[OurLogKnownFieldKey.ID]; const pinnedExpandKey = `pinned-${pinnedId}`; diff --git a/static/app/views/explore/logs/tables/logsTableRow.tsx b/static/app/views/explore/logs/tables/logsTableRow.tsx index 44f4943c7a1fd7..230e359d27a8fa 100644 --- a/static/app/views/explore/logs/tables/logsTableRow.tsx +++ b/static/app/views/explore/logs/tables/logsTableRow.tsx @@ -170,7 +170,6 @@ export const LogRowContent = memo(function LogRowContent({ const rowId = String(dataRow[OurLogKnownFieldKey.ID]); const expansionKey = expansionKeyProp ?? rowId; const logsPinning = useLogsPinning(); - const isHoverLinked = logsPinning?.hoveringRow === rowId; const isPinned = logsPinning?.pinnedRows.has(rowId); const [shouldRenderHoverElements, setShouldRenderHoverElements] = useState(isPinned); @@ -333,17 +332,14 @@ export const LogRowContent = memo(function LogRowContent({ data-test-id="log-table-row" data-row-highlighted={isPseudoRow} data-row-pinned={isPinned} - data-row-hover-linked={isHoverLinked} {...omit(rowInteractProps, 'className')} className={classNames(rowInteractProps.className, replayTimeClasses)} onMouseEnter={e => { setShouldRenderHoverElements(true); rowInteractProps.onMouseEnter?.(e); - logsPinning?.updateHoveringRow(true, rowId); }} onMouseLeave={e => { rowInteractProps.onMouseLeave?.(e); - logsPinning?.updateHoveringRow(false, rowId); }} > From a689e46a15117cd973b6cf843c24a3817d9b4c51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Wed, 13 May 2026 11:14:06 -0400 Subject: [PATCH 15/16] onCollapse/onExpand expansionKey, not rowId --- 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 5fbb58b79c2723..133bcbb5bb88de 100644 --- a/static/app/views/explore/logs/tables/logsTableRow.tsx +++ b/static/app/views/explore/logs/tables/logsTableRow.tsx @@ -272,9 +272,9 @@ export const LogRowContent = memo(function LogRowContent({ function toggleExpanded() { if (onExpand) { if (isExpanded) { - onCollapse?.(rowId); + onCollapse?.(expansionKey); } else { - onExpand?.(rowId); + onExpand?.(expansionKey); } } else { setExpanded(e => !e); From ca0602a2eea8db8130612638d0c27a41ce84e7d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Wed, 13 May 2026 11:22:20 -0400 Subject: [PATCH 16/16] this single issue has convinced me that fine-grained reactivity is better --- .../logs/pinning/useLogsPinning.spec.tsx | 24 +++------------ .../explore/logs/pinning/useLogsPinning.tsx | 29 ++++++++++--------- 2 files changed, 19 insertions(+), 34 deletions(-) diff --git a/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx b/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx index 776b89d2128c9f..9851ae310bd376 100644 --- a/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx +++ b/static/app/views/explore/logs/pinning/useLogsPinning.spec.tsx @@ -5,16 +5,6 @@ import { useLogsPinning, } from 'sentry/views/explore/logs/pinning/useLogsPinning'; -let replaceStateSpy: jest.SpyInstance; - -beforeEach(() => { - replaceStateSpy = jest.spyOn(window.history, 'replaceState'); -}); - -afterEach(() => { - replaceStateSpy.mockRestore(); -}); - jest.mock('sentry/views/explore/logs/pinning/useOurLogsPinning', () => ({ useOurLogsPinningEnabled: () => true, })); @@ -110,35 +100,29 @@ describe('useLogsPinning', () => { }); it('writes the pinned id to the URL query string when togglePinnedRow is called', () => { - const {result} = renderHookWithProviders(() => useLogsPinning(), { + const {result, router} = renderHookWithProviders(() => useLogsPinning(), { additionalWrapper: LogsPinningProvider, }); - replaceStateSpy.mockClear(); - act(() => { result.current?.togglePinnedRow('log-1'); }); - const lastCall = replaceStateSpy.mock.calls.at(-1); - expect(lastCall?.[2]).toContain('logsPinned=log-1'); + expect(router.location.query.logsPinned).toContain('log-1'); }); it('removes the logsPinned key from the URL when clearPinnedRows is called', () => { - const {result} = renderHookWithProviders(() => useLogsPinning(), { + const {result, router} = renderHookWithProviders(() => useLogsPinning(), { additionalWrapper: LogsPinningProvider, initialRouterConfig: { location: {pathname: '/', query: {logsPinned: 'log-1'}}, }, }); - replaceStateSpy.mockClear(); - act(() => { result.current?.clearPinnedRows(); }); - const lastCall = replaceStateSpy.mock.calls.at(-1); - expect(lastCall?.[2]).not.toContain('logsPinned'); + expect(router.location.query.logsPinned).toHaveLength(0); }); }); diff --git a/static/app/views/explore/logs/pinning/useLogsPinning.tsx b/static/app/views/explore/logs/pinning/useLogsPinning.tsx index a997a267830c3d..894e52cdb00513 100644 --- a/static/app/views/explore/logs/pinning/useLogsPinning.tsx +++ b/static/app/views/explore/logs/pinning/useLogsPinning.tsx @@ -10,6 +10,7 @@ import { import {decodeList} from 'sentry/utils/queryString'; import {useLocation} from 'sentry/utils/useLocation'; +import {useNavigate} from 'sentry/utils/useNavigate'; import {useOurLogsPinningEnabled} from 'sentry/views/explore/logs/pinning/useOurLogsPinning'; const LOGS_PINNED_KEY = 'logsPinned'; @@ -24,6 +25,7 @@ const LogsPinningContext = createContext(undefined); export function LogsPinningProvider({children}: {children: ReactNode}) { const location = useLocation(); + const navigate = useNavigate(); const [pinnedRows, setPinnedRows] = useState>(() => { return new Set(decodeList(location.query?.[LOGS_PINNED_KEY]).filter(Boolean)); @@ -46,20 +48,19 @@ export function LogsPinningProvider({children}: {children: ReactNode}) { }, []); useEffect(() => { - const params = new URLSearchParams(window.location.search); - params.delete(LOGS_PINNED_KEY); - - for (const id of pinnedRows) { - params.append(LOGS_PINNED_KEY, id); - } - - const newSearch = params.toString(); - const newUrl = newSearch - ? `${window.location.pathname}?${newSearch}${window.location.hash}` - : `${window.location.pathname}${window.location.hash}`; - - window.history.replaceState(window.history.state, '', newUrl); - }, [pinnedRows]); + navigate( + { + ...location, + query: { + ...location.query, + [LOGS_PINNED_KEY]: Array.from(pinnedRows), + }, + }, + {replace: true} + ); + // location is intentionally omitted — we only want to sync pinnedRows to the URL. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [navigate, pinnedRows]); const value = useMemo( () => ({