From 93523fd2f96a1bbce470519ea91f684fd3b91fff Mon Sep 17 00:00:00 2001 From: Chrisandra <103969203+ChrisandraVaz@users.noreply.github.com> Date: Thu, 13 Aug 2026 11:16:09 -0700 Subject: [PATCH 1/2] feat(scraps): Add Tooltip.Header, Body, Row and Footer Tooltip content that is a row of labelled values rather than a sentence currently has to fight the overlay: it hardcodes `padding: md lg` and `text-align: center`, so a card wanting full-width sections has to cancel both with negative margins. That pattern is already hand-rolled twice, in logsTimeTooltip and breadcrumbsTimeline, and RelativeTime was about to be the third. Adds a `padding` prop (defaulting to the current `md lg`, so no existing tooltip changes) and four sections that own their own spacing. `Tooltip.Body` owns the grid and `Tooltip.Row` renders as `display: contents`, so a column stays aligned across rows even when one row's cell is wider than the same cell above it. Also forwards `padding` through InfoText, which is the path TimeSince takes to the tooltip. --- static/app/components/core/info/infoText.tsx | 4 +- .../app/components/core/tooltip/tooltip.mdx | 74 ++++++++ .../components/core/tooltip/tooltip.spec.tsx | 138 +++++++++++++- .../app/components/core/tooltip/tooltip.tsx | 171 +++++++++++++++++- 4 files changed, 381 insertions(+), 6 deletions(-) diff --git a/static/app/components/core/info/infoText.tsx b/static/app/components/core/info/infoText.tsx index 72e42a1ee529..7071c0be476a 100644 --- a/static/app/components/core/info/infoText.tsx +++ b/static/app/components/core/info/infoText.tsx @@ -9,7 +9,7 @@ type InfoTextBaseProps = DistributedOmit, 'title' | 'variant' | 'underline'> & { title: React.ReactNode; variant?: TooltipProps['underlineColor'] | 'inherit'; - } & Pick; + } & Pick; export type InfoTextProps = | (InfoTextBaseProps & {mode?: undefined}) @@ -22,6 +22,7 @@ export function InfoText ``` +## Structured Content + +When a tooltip carries labelled values rather than a sentence, compose it out of +`Tooltip.Header`, `Tooltip.Body` and `Tooltip.Footer`. Each section applies its own +padding so that it spans the full width of the overlay, which means the tooltip itself has +to opt out of the shared content padding with `padding="0"`. + +`Tooltip.Row` renders its children directly into the column tracks declared by +`Tooltip.Body`, so a column stays aligned across every row — below, the two dates line up +even though `PDT` and `UTC` are different widths. A row that owned its own grid would only +align against itself. + + + + Last Seen + + + PDT + Jul 28, 2026 + 11:40 PM + + + UTC + Jul 29, 2026 + 6:40 AM + + + + } + > + + + +```jsx + + Last Seen + + + PDT + Jul 28, 2026 + 11:40 PM + + + UTC + Jul 29, 2026 + 6:40 AM + + + + } +> + + +``` + +Sections deliberately do not set a font size, so they inherit the tooltip's. They do set +their own text alignment, because the tooltip centers content by default — right for a +sentence, wrong for a row of labelled values. + +Headers are written in sentence case. Do not uppercase them. + ## Disabled State Tooltips can be disabled entirely using the `disabled` prop, which prevents them from showing on hover. diff --git a/static/app/components/core/tooltip/tooltip.spec.tsx b/static/app/components/core/tooltip/tooltip.spec.tsx index 12af4f8bfebe..14d5daaf8cdc 100644 --- a/static/app/components/core/tooltip/tooltip.spec.tsx +++ b/static/app/components/core/tooltip/tooltip.spec.tsx @@ -1,6 +1,7 @@ import {act, render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; -import {Tooltip} from '@sentry/scraps/tooltip'; +import {Container} from '@sentry/scraps/layout'; +import {Tooltip, type TooltipProps} from '@sentry/scraps/tooltip'; describe('Tooltip', () => { let originalResizeObserver: typeof window.ResizeObserver; @@ -201,4 +202,139 @@ describe('Tooltip', () => { await userEvent.click(screen.getByText('Copy')); expect(handleAncestorClick).not.toHaveBeenCalled(); }); + + describe('content padding', () => { + // This suite stubs `getComputedStyle` so that it cannot see emotion rules + // (tests/js/setup.ts), which rules out asserting padding directly — and + // makes a negative style assertion pass vacuously. Emotion derives the + // class name from a hash of the serialized styles, so comparing classes + // between two renders is a real assertion about the CSS they produce. + async function paddingClassName(padding?: TooltipProps['padding']) { + const {unmount} = render( + + + + ); + await userEvent.hover(screen.getByText('My Button')); + const className = screen.getByText('test').closest('[data-tooltip]')?.className; + unmount(); + + return className; + } + + it('pads the content by default', async () => { + const byDefault = await paddingClassName(); + + // Every tooltip that has not opted out depends on this default, so it is + // the regression guard for the existing call sites. + expect(byDefault).toBeTruthy(); + expect(byDefault).toBe(await paddingClassName('md lg')); + }); + + it('drops the content padding when opted out', async () => { + expect(await paddingClassName('0')).not.toBe(await paddingClassName()); + }); + }); + + describe('sections', () => { + it('renders a header label alongside its trailing value', async () => { + render( + Last Seen} + > + + + ); + + await userEvent.hover(screen.getByText('My Button')); + + expect(screen.getByText('Last Seen')).toBeInTheDocument(); + expect(screen.getByText('8mo ago')).toBeInTheDocument(); + }); + + it('renders a footer label alongside its trailing value', async () => { + render( + Times shown in} + > + + + ); + + await userEvent.hover(screen.getByText('My Button')); + + expect(screen.getByText('Times shown in')).toBeInTheDocument(); + expect(screen.getByText('UTC')).toBeInTheDocument(); + }); + + it('renders every row into the one body grid', async () => { + render( + + + PDT + Jul 28, 2026 + + + UTC + Jul 29, 2026 + + + } + > + + + ); + + await userEvent.hover(screen.getByText('My Button')); + + // Sharing one grid is what keeps a column aligned between the rows when + // one row's cell is wider than the other's. + const firstRow = screen.getByText('PDT').parentElement; + const secondRow = screen.getByText('UTC').parentElement; + + expect(firstRow).toBeInTheDocument(); + expect(firstRow?.parentElement).toBe(secondRow?.parentElement); + }); + + it('renders a row as a layout-less wrapper so its cells join that grid', async () => { + // A row that established its own layout box would align its columns only + // against itself, so what it renders has to stay `display: contents`. + // Same class as the reference means the same serialized styles. + const reference = render( + + reference cell + + ); + const referenceClassName = + screen.getByText('reference cell').parentElement?.className; + reference.unmount(); + + render( + + + row cell + + + } + > + + + ); + + await userEvent.hover(screen.getByText('My Button')); + + expect(referenceClassName).toBeTruthy(); + expect(screen.getByText('row cell').parentElement?.className).toBe( + referenceClassName + ); + }); + }); }); diff --git a/static/app/components/core/tooltip/tooltip.tsx b/static/app/components/core/tooltip/tooltip.tsx index 9e6215b25c75..481fe9687c53 100644 --- a/static/app/components/core/tooltip/tooltip.tsx +++ b/static/app/components/core/tooltip/tooltip.tsx @@ -5,7 +5,23 @@ import {useTheme} from '@emotion/react'; import styled from '@emotion/styled'; import {AnimatePresence} from 'framer-motion'; +import { + Container, + type ContainerProps, + Flex, + getSpacing, + Grid, + type GridProps, + rc, +} from '@sentry/scraps/layout'; +// Imported from the module rather than the `text` barrel on purpose. That +// barrel also re-exports `Prose`, which reaches `code` -> `codeBlock` -> +// `Button`, and `Button` imports this file — so going through it would close an +// import cycle and leave `Button` undefined at module-eval time. +import {Text} from '@sentry/scraps/text/text'; + import {Overlay, PositionWrapper} from 'sentry/components/overlay'; +import {defined} from 'sentry/utils/defined'; import type {UseHoverOverlayProps} from 'sentry/utils/useHoverOverlay'; import {useHoverOverlay} from 'sentry/utils/useHoverOverlay'; @@ -38,14 +54,25 @@ export interface TooltipProps extends UseHoverOverlayProps { * Additional style rules for the tooltip content. */ overlayStyle?: React.CSSProperties | SerializedStyles; + /** + * Padding around the tooltip content. + * + * Set to `'0'` when composing `Tooltip.Header`, `Tooltip.Body` and + * `Tooltip.Footer` — each section applies its own padding so that it can span + * the full width of the overlay, which a shared outer padding would prevent. + * + * @default 'md lg' + */ + padding?: ContainerProps['padding']; } -export function Tooltip({ +function TooltipComponent({ children, overlayStyle, title, disabled = false, maxWidth, + padding = 'md lg', isHoverable = true, ...hoverOverlayProps }: TooltipProps) { @@ -113,6 +140,7 @@ export function Tooltip({ prop !== 'maxWidth', -})<{maxWidth?: number}>` - padding: ${p => p.theme.space.md} ${p => p.theme.space.lg}; + shouldForwardProp: prop => prop !== 'maxWidth' && prop !== 'padding', +})<{maxWidth?: number; padding?: TooltipProps['padding']}>` + ${p => rc('padding', p.padding, p.theme, getSpacing)}; overflow-wrap: break-word; max-width: ${p => p.maxWidth ?? 225}px; color: ${p => p.theme.tokens.content.primary}; @@ -146,3 +174,138 @@ const TooltipContent = styled(Overlay, { line-height: 1.2; text-align: center; `; + +interface TooltipHeaderProps { + /** + * What the section describes, e.g. "Last Seen". + */ + children: React.ReactNode; + /** + * Optional value pinned to the opposite edge of the header, e.g. the + * relative time the rows below resolve. + */ + trailing?: React.ReactNode; +} + +/** + * Names what the section below it describes. Written in sentence case — weight + * and position carry the hierarchy, so it is deliberately not uppercased and + * carries no bottom border. + */ +function TooltipHeader({children, trailing}: TooltipHeaderProps) { + return ( + + + {children} + + {defined(trailing) && ( + + {trailing} + + )} + + ); +} + +interface TooltipBodyProps { + children: React.ReactNode; + /** + * The column tracks rows are laid out in. `Tooltip.Row` renders its children + * straight into these tracks, so a column stays aligned across every row even + * when one row's cell is wider than the same cell in the row above. + * + * @default '1fr' + */ + columns?: GridProps['columns']; + /** + * @default '2xs sm' + */ + gap?: GridProps['gap']; +} + +/** + * The padded region a tooltip's rows are laid out in, and the grid those rows + * share. + */ +function TooltipBody({children, columns = '1fr', gap = '2xs sm'}: TooltipBodyProps) { + return ( + + {children} + + ); +} + +interface TooltipRowProps { + children: React.ReactNode; +} + +/** + * One row of a `Tooltip.Body`. Renders as `display: contents` so that its + * children become grid items of the body itself rather than of a nested box — + * a row that owned its own grid would align its columns only against itself. + */ +function TooltipRow({children}: TooltipRowProps) { + return {children}; +} + +interface TooltipFooterProps { + children: React.ReactNode; + /** + * Optional value pinned to the opposite edge of the footer. + */ + trailing?: React.ReactNode; +} + +/** + * Trailing note for a tooltip, e.g. what the rows above are qualified by. Muted + * so it reads as secondary to them. + */ +function TooltipFooter({children, trailing}: TooltipFooterProps) { + return ( + + + {children} + + {defined(trailing) && ( + + {trailing} + + )} + + ); +} + +/** + * Tooltips show contextual information about an element on hover. + * + * For content that is a row of labelled values rather than a sentence, compose + * it from the sections rather than passing a block of markup. Each section + * applies its own padding so that it spans the full width of the overlay, which + * means the tooltip has to opt out of the shared content padding: + * + * ```tsx + * + * Last Seen + * + * {cells} + * + * + * } + * > + * {trigger} + * + * ``` + * + * Sections set their own text alignment, because a tooltip centers its content + * by default — right for a sentence, wrong for a row of labelled values. They + * set no font size, so they inherit the tooltip's. + */ +export const Tooltip = Object.assign(TooltipComponent, { + Header: TooltipHeader, + Body: TooltipBody, + Row: TooltipRow, + Footer: TooltipFooter, +}); From b8a3c9df4c34d89251037c4d9222402b3c88900d Mon Sep 17 00:00:00 2001 From: Chrisandra <103969203+ChrisandraVaz@users.noreply.github.com> Date: Fri, 14 Aug 2026 11:57:28 -0700 Subject: [PATCH 2/2] test(scraps): Assert tooltip padding against the generated CSS The padding tests compared emotion class names between two renders, which proved the default matched an explicit `md lg` but never that `md lg` resolves to the values the hardcoded rule used to emit. The comment justifying that also claimed emotion rules are unreadable in tests, which is wrong: sentry-test/utils exports `getEmotionRules`, and infoText.spec.tsx already uses it. Also covers two things that had no test at all: that InfoText forwards `padding` to the tooltip, which is the whole reason it was added and what PR 3 depends on, and that a tooltip composed of sections still associates itself with its trigger for screen readers. --- .../components/core/info/infoText.spec.tsx | 21 ++++++++ .../components/core/tooltip/tooltip.spec.tsx | 50 ++++++++++++------- 2 files changed, 54 insertions(+), 17 deletions(-) diff --git a/static/app/components/core/info/infoText.spec.tsx b/static/app/components/core/info/infoText.spec.tsx index d1ec85720cea..4b7bcad8dc9f 100644 --- a/static/app/components/core/info/infoText.spec.tsx +++ b/static/app/components/core/info/infoText.spec.tsx @@ -1,8 +1,12 @@ +import {ThemeFixture} from 'sentry-fixture/theme'; + import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; import {getEmotionRules} from 'sentry-test/utils'; import {InfoText} from '@sentry/scraps/info'; +const theme = ThemeFixture(); + describe('InfoText', () => { function mockOverflow(width: number, containerWidth: number) { Object.defineProperty(HTMLElement.prototype, 'scrollWidth', { @@ -69,6 +73,23 @@ describe('InfoText', () => { expect(screen.getByText('Text content')).toHaveAttribute('tabindex', '0'); }); + it('forwards padding to the tooltip', async () => { + // InfoText forwards an explicit list of tooltip props, so a tooltip built + // out of Tooltip.Header/Body/Row can only opt out of the shared content + // padding if `padding` is on that list. + render( + + Text content + + ); + + await userEvent.hover(screen.getByText('Text content')); + + const content = screen.getByText('Tooltip content'); + expect(content).toHaveAttribute('data-tooltip'); + expect(getEmotionRules(content).join('')).toContain(`padding: ${theme.space['0']};`); + }); + it('supports ellipsis with the regular always-on tooltip', () => { render( diff --git a/static/app/components/core/tooltip/tooltip.spec.tsx b/static/app/components/core/tooltip/tooltip.spec.tsx index 14d5daaf8cdc..e2e60c99dad5 100644 --- a/static/app/components/core/tooltip/tooltip.spec.tsx +++ b/static/app/components/core/tooltip/tooltip.spec.tsx @@ -1,8 +1,13 @@ +import {ThemeFixture} from 'sentry-fixture/theme'; + import {act, render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; +import {getEmotionRules} from 'sentry-test/utils'; import {Container} from '@sentry/scraps/layout'; import {Tooltip, type TooltipProps} from '@sentry/scraps/tooltip'; +const theme = ThemeFixture(); + describe('Tooltip', () => { let originalResizeObserver: typeof window.ResizeObserver; @@ -204,35 +209,32 @@ describe('Tooltip', () => { }); describe('content padding', () => { - // This suite stubs `getComputedStyle` so that it cannot see emotion rules - // (tests/js/setup.ts), which rules out asserting padding directly — and - // makes a negative style assertion pass vacuously. Emotion derives the - // class name from a hash of the serialized styles, so comparing classes - // between two renders is a real assertion about the CSS they produce. - async function paddingClassName(padding?: TooltipProps['padding']) { - const {unmount} = render( + // `getComputedStyle` is stubbed in tests/js/setup.ts and cannot see emotion + // rules, so read the generated CSS rather than using toHaveStyle, which + // would pass vacuously against an empty declaration. + async function contentRules(padding?: TooltipProps['padding']) { + render( ); await userEvent.hover(screen.getByText('My Button')); - const className = screen.getByText('test').closest('[data-tooltip]')?.className; - unmount(); + const content = screen.getByText('test'); + expect(content).toHaveAttribute('data-tooltip'); - return className; + return getEmotionRules(content).join(''); } it('pads the content by default', async () => { - const byDefault = await paddingClassName(); - - // Every tooltip that has not opted out depends on this default, so it is - // the regression guard for the existing call sites. - expect(byDefault).toBeTruthy(); - expect(byDefault).toBe(await paddingClassName('md lg')); + // Every tooltip that has not opted out depends on this, so it is the + // regression guard for the existing call sites. + expect(await contentRules()).toContain( + `padding: ${theme.space.md} ${theme.space.lg};` + ); }); it('drops the content padding when opted out', async () => { - expect(await paddingClassName('0')).not.toBe(await paddingClassName()); + expect(await contentRules('0')).toContain(`padding: ${theme.space['0']};`); }); }); @@ -301,6 +303,20 @@ describe('Tooltip', () => { expect(firstRow?.parentElement).toBe(secondRow?.parentElement); }); + it('stays described by its trigger', async () => { + // Sections are the tooltip's content, so they must not disturb the + // trigger/overlay association screen readers rely on. + render( + Last Seen}> + + + ); + + await userEvent.hover(screen.getByText('My Button')); + + expect(screen.getByText('My Button')).toHaveAttribute('aria-describedby'); + }); + it('renders a row as a layout-less wrapper so its cells join that grid', async () => { // A row that established its own layout box would align its columns only // against itself, so what it renders has to stay `display: contents`.