From 1a758ff0f6e6b8dd2ace956f92d95ce4a09ef447 Mon Sep 17 00:00:00 2001 From: Lion Chen Date: Fri, 21 Aug 2026 16:20:12 +0000 Subject: [PATCH] Compress UserDisplayName Into Two Layouts (#425) Export-Source-Commit: 6a8da4b292a0a8f5bb2fea6a01ebe72fdafba1e0 --- .../UserDisplayName/UserDisplayName.story.tsx | 53 +------- .../UserDisplayName/UserDisplayName.test.tsx | 121 +++++------------- .../UserDisplayName/UserDisplayName.tsx | 71 ++++------ 3 files changed, 57 insertions(+), 188 deletions(-) diff --git a/web/packages/shared/components/UserDisplayName/UserDisplayName.story.tsx b/web/packages/shared/components/UserDisplayName/UserDisplayName.story.tsx index 8373225584e..69224881d13 100644 --- a/web/packages/shared/components/UserDisplayName/UserDisplayName.story.tsx +++ b/web/packages/shared/components/UserDisplayName/UserDisplayName.story.tsx @@ -31,12 +31,12 @@ const meta = { username: 'alice@example.com', primaryText: 'Alice Jones', secondaryText: 'Engineering', - layout: 'tooltip', + layout: 'inline', }, argTypes: { layout: { control: { type: 'radio' }, - options: ['inline', 'stacked', 'tooltip'], + options: ['inline', 'stacked'], }, }, decorators: [ @@ -68,7 +68,7 @@ export const LayoutVariants: Story = { > - - - ), }; @@ -131,16 +120,6 @@ export const LayoutVariantsWithoutSecondary: Story = { layout="stacked" /> - - - ), }; @@ -155,7 +134,7 @@ export const LayoutVariantsWithoutPrimary: Story = { > - - - ), }; @@ -201,9 +170,6 @@ export const LayoutVariantsWithoutPrimaryOrSecondary: Story = { - - - ), }; @@ -235,17 +201,6 @@ export const LongValues: Story = { layout="stacked" /> - - - ), }; diff --git a/web/packages/shared/components/UserDisplayName/UserDisplayName.test.tsx b/web/packages/shared/components/UserDisplayName/UserDisplayName.test.tsx index c0f78c629cd..832d616d479 100644 --- a/web/packages/shared/components/UserDisplayName/UserDisplayName.test.tsx +++ b/web/packages/shared/components/UserDisplayName/UserDisplayName.test.tsx @@ -16,63 +16,57 @@ * along with this program. If not, see . */ -import { render, screen, userEvent, within } from 'design/utils/testing'; +import { render, screen, within } from 'design/utils/testing'; import { UserDisplayName, type UserDisplayNameLayout } from './UserDisplayName'; describe('UserDisplayName', () => { const username = 'alice@example.com'; - const layouts: UserDisplayNameLayout[] = ['inline', 'stacked', 'tooltip']; + const layouts: UserDisplayNameLayout[] = ['inline', 'stacked']; const valueScenarios: { name: string; primaryText?: string; secondaryText?: string; expectedPrimary: string; - expectedSecondary: string | null; - expectedVisibleUsernameCountByLayout: Record; + expectedVisibleSecondaryCountByLayout: Record< + UserDisplayNameLayout, + number + >; }[] = [ { name: 'primary, secondary, and username', primaryText: 'Alice Jones', secondaryText: 'Engineering', expectedPrimary: 'Alice Jones', - expectedSecondary: 'Engineering', - expectedVisibleUsernameCountByLayout: { - inline: 1, + expectedVisibleSecondaryCountByLayout: { + inline: 0, stacked: 1, - tooltip: 0, }, }, { name: 'only username', expectedPrimary: username, - expectedSecondary: null, - expectedVisibleUsernameCountByLayout: { - inline: 1, - stacked: 1, - tooltip: 1, + expectedVisibleSecondaryCountByLayout: { + inline: 0, + stacked: 0, }, }, { name: 'primary and username', primaryText: 'Alice Jones', expectedPrimary: 'Alice Jones', - expectedSecondary: null, - expectedVisibleUsernameCountByLayout: { - inline: 1, - stacked: 1, - tooltip: 0, + expectedVisibleSecondaryCountByLayout: { + inline: 0, + stacked: 0, }, }, { name: 'secondary and username', secondaryText: 'Engineering', expectedPrimary: username, - expectedSecondary: 'Engineering', - expectedVisibleUsernameCountByLayout: { - inline: 1, + expectedVisibleSecondaryCountByLayout: { + inline: 0, stacked: 1, - tooltip: 1, }, }, ]; @@ -91,29 +85,16 @@ describe('UserDisplayName', () => { expect(screen.getByText(scenario.expectedPrimary)).toBeInTheDocument(); expect(screen.queryAllByText('Engineering')).toHaveLength( - scenario.expectedSecondary ? 1 : 0 + scenario.expectedVisibleSecondaryCountByLayout[layout] ); - const expectedVisibleUsernameCount = - scenario.expectedVisibleUsernameCountByLayout[layout]; - expect(screen.queryAllByText(username)).toHaveLength( - expectedVisibleUsernameCount - ); - - const tooltipTriggerLabel = getTooltipAriaLabel( - scenario.expectedPrimary, - scenario.expectedSecondary, - username - ); - expect(screen.queryAllByLabelText(tooltipTriggerLabel)).toHaveLength( - layout === 'tooltip' && expectedVisibleUsernameCount === 0 ? 1 : 0 - ); + expect(screen.queryAllByText(username)).toHaveLength(1); }); } } - it('formats inline supporting values with delimiters', () => { - render( + it('ignores secondary text in inline layout', () => { + const { container, rerender } = render( { .parentElement as HTMLElement; const inlineUsername = within(primaryLine).getByText(username); const inlineSupportingValues = inlineUsername.parentElement as HTMLElement; - const inlineSecondary = within(inlineSupportingValues).getByText( - 'Engineering' - ); expect(primaryLine).toContainElement(inlineSupportingValues); expect(inlineSupportingValues).toHaveStyleRule('content', "'('", { @@ -137,9 +115,17 @@ describe('UserDisplayName', () => { expect(inlineSupportingValues).toHaveStyleRule('content', "')'", { modifier: '::after', }); - expect(inlineSecondary).toHaveStyleRule('content', "'•'", { - modifier: '::before', - }); + expect(screen.queryByText('Engineering')).not.toBeInTheDocument(); + + const withSecondaryText = container.innerHTML; + rerender( + + ); + expect(container.innerHTML).toBe(withSecondaryText); }); it('renders stacked supporting values together below the primary line', () => { @@ -186,49 +172,4 @@ describe('UserDisplayName', () => { ).not.toBeInTheDocument(); expect(screen.getByText('Engineering')).toBeInTheDocument(); }); - - it('defaults to tooltip layout', async () => { - const user = userEvent.setup(); - - render( - - ); - - expect(screen.queryByText(username)).not.toBeInTheDocument(); - - await user.hover(screen.getByText('Alice Jones')); - - const tooltip = await screen.findByRole('tooltip'); - expect(within(tooltip).getByText(username)).toBeInTheDocument(); - }); - - it('anchors tooltip layout to the primary text', () => { - render( - - ); - - const tooltipTrigger = screen.getByLabelText( - 'Alice Jones, Engineering, username alice@example.com' - ); - expect(tooltipTrigger).toBe(screen.getByText('Alice Jones')); - }); - - function getTooltipAriaLabel( - primary: string, - secondary: string | null | undefined, - username: string - ) { - return [primary, secondary, `username ${username}`] - .filter(Boolean) - .join(', '); - } }); diff --git a/web/packages/shared/components/UserDisplayName/UserDisplayName.tsx b/web/packages/shared/components/UserDisplayName/UserDisplayName.tsx index 9044f9df1dc..e2f986ff71f 100644 --- a/web/packages/shared/components/UserDisplayName/UserDisplayName.tsx +++ b/web/packages/shared/components/UserDisplayName/UserDisplayName.tsx @@ -20,15 +20,14 @@ import type { ComponentProps } from 'react'; import styled, { css } from 'styled-components'; import { Text } from 'design'; -import { HoverTooltip } from 'design/Tooltip'; -export type UserDisplayNameLayout = 'inline' | 'stacked' | 'tooltip'; +export type UserDisplayNameLayout = 'inline' | 'stacked'; export function UserDisplayName({ username, primaryText, secondaryText, - layout = 'tooltip', + layout, className, primaryTextProps, usernameTextProps, @@ -37,7 +36,7 @@ export function UserDisplayName({ username: string; primaryText?: string; secondaryText?: string; - layout?: UserDisplayNameLayout; + layout: UserDisplayNameLayout; className?: string; primaryTextProps?: ComponentProps; usernameTextProps?: ComponentProps; @@ -46,10 +45,9 @@ export function UserDisplayName({ const displayPrimary = normalizeText(primaryText); const displaySecondary = normalizeText(secondaryText); const primary = displayPrimary || username; - const tooltipLabel = getTooltipAriaLabel(primary, displaySecondary, username); - const primaryValue = (ariaLabel?: string) => ( - + const primaryValue = () => ( + {primary} ); @@ -66,35 +64,35 @@ export function UserDisplayName({ ); - const supportingValues = displayPrimary ? ( - <> - - {username} - - {separatedSecondaryValue} - - ) : ( - secondaryValue - ); - switch (layout) { case 'inline': return ( {primaryValue()} - {displayPrimary ? ( + {displayPrimary && ( - {supportingValues} + + {username} + - ) : ( - supportingValues )} ); - case 'stacked': + case 'stacked': { + const supportingValues = displayPrimary ? ( + <> + + {username} + + {separatedSecondaryValue} + + ) : ( + secondaryValue + ); + return ( {primaryValue()} @@ -105,22 +103,7 @@ export function UserDisplayName({ )} ); - - case 'tooltip': - return ( - - {displayPrimary ? ( - - - {primaryValue(tooltipLabel)} - - - ) : ( - {primaryValue()} - )} - {secondaryValue} - - ); + } default: layout satisfies never; @@ -133,16 +116,6 @@ function normalizeText(text?: string) { return trimmedText || undefined; } -function getTooltipAriaLabel( - primary: string, - secondary: string | undefined, - username: string -) { - return [primary, secondary, `username ${username}`] - .filter(Boolean) - .join(', '); -} - // `min-width: 0` lets a flex item shrink below its content size, which is what // allows the child text to actually trigger ellipsis instead of forcing the // container to grow.