diff --git a/frontend/e2e/accessibility.spec.ts b/frontend/e2e/accessibility.spec.ts index 3cab6378c1..771b75edba 100644 --- a/frontend/e2e/accessibility.spec.ts +++ b/frontend/e2e/accessibility.spec.ts @@ -146,17 +146,17 @@ test.describe("Visual Consistency", () => { await page.goto("/"); // Wait for initial render - await expect(page.getByText("PyRIT Attack")).toBeVisible(); + const anchor = page.getByTestId("new-attack-btn"); + await expect(anchor).toBeVisible(); // Take measurements - const header = page.getByText("PyRIT Attack"); - const initialBox = await header.boundingBox(); + const initialBox = await anchor.boundingBox(); // Wait a moment for any delayed renders await page.waitForTimeout(500); // Verify position hasn't changed - const finalBox = await header.boundingBox(); + const finalBox = await anchor.boundingBox(); if (initialBox && finalBox) { expect(finalBox.x).toBe(initialBox.x); diff --git a/frontend/e2e/api.spec.ts b/frontend/e2e/api.spec.ts index b0b390e514..9c50ad75f9 100644 --- a/frontend/e2e/api.spec.ts +++ b/frontend/e2e/api.spec.ts @@ -141,6 +141,6 @@ test.describe("Error Handling", () => { await page.goto("/"); // UI should be responsive even while APIs are delayed - await expect(page.getByText("PyRIT Attack")).toBeVisible({ timeout: 10000 }); + await expect(page.getByTestId("new-attack-btn")).toBeVisible({ timeout: 10000 }); }); }); diff --git a/frontend/e2e/chat.spec.ts b/frontend/e2e/chat.spec.ts index 0938dcafa0..54eef5291b 100644 --- a/frontend/e2e/chat.spec.ts +++ b/frontend/e2e/chat.spec.ts @@ -137,7 +137,7 @@ async function activateMockTarget(page: Page) { // Return to Chat view await page.getByTitle("Chat").click(); - await expect(page.getByText("PyRIT Attack")).toBeVisible({ timeout: 5000 }); + await expect(page.getByTestId("new-attack-btn")).toBeVisible({ timeout: 5000 }); } // --------------------------------------------------------------------------- @@ -153,8 +153,8 @@ test.describe("Application Smoke Tests", () => { await expect(page.locator("body")).toBeVisible(); }); - test("should display PyRIT header", async ({ page }) => { - await expect(page.getByText("PyRIT Attack")).toBeVisible({ timeout: 10000 }); + test("should display chat ribbon", async ({ page }) => { + await expect(page.getByTestId("new-attack-btn")).toBeVisible({ timeout: 10000 }); }); test("should have New Attack button", async ({ page }) => { @@ -169,7 +169,7 @@ test.describe("Application Smoke Tests", () => { test.describe("Theme Toggle", () => { test("should toggle dark/light theme", async ({ page }) => { await page.goto("/"); - await expect(page.getByText("PyRIT Attack")).toBeVisible({ timeout: 10000 }); + await expect(page.getByTestId("new-attack-btn")).toBeVisible({ timeout: 10000 }); // The app defaults to dark mode, so the toggle button title should say "Light Mode" const themeBtn = page.getByTitle("Light Mode"); @@ -197,8 +197,12 @@ test.describe("Chat Functionality", () => { }); test("should display target info after activation", async ({ page }) => { - await expect(page.getByText("OpenAIChatTarget")).toBeVisible(); - await expect(page.getByText(/gpt-4o-mock/)).toBeVisible(); + // Scope queries to the badge so we don't also match the (hidden) + // copy of the target text that Fluent's Tooltip renders into the DOM. + const badge = page.getByTestId("target-badge"); + await expect(badge).toBeVisible(); + await expect(badge).toContainText("OpenAIChatTarget"); + await expect(badge).toContainText(/gpt-4o-mock/); }); test("should send a message and receive backend response", async ({ page }) => { @@ -721,7 +725,11 @@ test.describe("Target type scenarios", () => { // Navigate to chat await page.getByTitle("Chat").click(); - await expect(page.getByText("OpenAIImageTarget")).toBeVisible(); - await expect(page.getByText(/dall-e-3/)).toBeVisible(); + // Scope queries to the badge so we don't also match the (hidden) + // copy of the target text that Fluent's Tooltip renders into the DOM. + const badge = page.getByTestId("target-badge"); + await expect(badge).toBeVisible(); + await expect(badge).toContainText("OpenAIImageTarget"); + await expect(badge).toContainText(/dall-e-3/); }); }); diff --git a/frontend/e2e/config.spec.ts b/frontend/e2e/config.spec.ts index cf2ee36ce8..c62edd1825 100644 --- a/frontend/e2e/config.spec.ts +++ b/frontend/e2e/config.spec.ts @@ -232,11 +232,14 @@ test.describe("Target Config ↔ Chat Navigation", () => { // Navigate back to chat await page.getByTitle("Chat").click(); - await expect(page.getByText("PyRIT Attack")).toBeVisible(); - - // Chat should show the active target type - await expect(page.getByText("OpenAIChatTarget")).toBeVisible(); - await expect(page.getByText(/gpt-4o/)).toBeVisible(); + await expect(page.getByTestId("new-attack-btn")).toBeVisible(); + + // Chat should show the active target type. Scope to the badge to + // avoid matching the (hidden) tooltip copy of the same text. + const badge = page.getByTestId("target-badge"); + await expect(badge).toBeVisible(); + await expect(badge).toContainText("OpenAIChatTarget"); + await expect(badge).toContainText(/gpt-4o/); }); test("should enable chat input after a target is set", async ({ page }) => { diff --git a/frontend/e2e/converters.spec.ts b/frontend/e2e/converters.spec.ts index 1aa0c5096d..ea8a16885c 100644 --- a/frontend/e2e/converters.spec.ts +++ b/frontend/e2e/converters.spec.ts @@ -362,7 +362,7 @@ async function activateMockTarget(page: Page) { await setActiveBtn.click(); await page.getByTitle("Chat", { exact: true }).click(); - await expect(page.getByText("PyRIT Attack")).toBeVisible({ timeout: 5000 }); + await expect(page.getByTestId("new-attack-btn")).toBeVisible({ timeout: 5000 }); } /** Open converter panel and select a converter by name. */ diff --git a/frontend/e2e/errors.spec.ts b/frontend/e2e/errors.spec.ts index 81e783a346..b76040e4fb 100644 --- a/frontend/e2e/errors.spec.ts +++ b/frontend/e2e/errors.spec.ts @@ -175,7 +175,7 @@ async function activateMockTarget(page: Page) { await expect(setActiveBtn).toBeVisible({ timeout: 5000 }); await setActiveBtn.click(); await page.getByTitle("Chat").click(); - await expect(page.getByText("PyRIT Attack")).toBeVisible({ timeout: 5000 }); + await expect(page.getByTestId("new-attack-btn")).toBeVisible({ timeout: 5000 }); } /** Send a message and wait for the response. */ @@ -324,7 +324,7 @@ test.describe("Error: connection banner on health failure", () => { }) => { // Let the page load normally first await page.goto("/"); - await expect(page.getByText("PyRIT Attack")).toBeVisible({ + await expect(page.getByTestId("new-attack-btn")).toBeVisible({ timeout: 10000, }); @@ -372,7 +372,7 @@ test.describe("Error: connection banner recovery", () => { }); await page.goto("/"); - await expect(page.getByText("PyRIT Attack")).toBeVisible({ + await expect(page.getByTestId("new-attack-btn")).toBeVisible({ timeout: 10000, }); diff --git a/frontend/e2e/flows.spec.ts b/frontend/e2e/flows.spec.ts index b5c9bcb82d..4bce02db0c 100644 --- a/frontend/e2e/flows.spec.ts +++ b/frontend/e2e/flows.spec.ts @@ -161,7 +161,7 @@ async function activateTarget(page: Page, targetType: string): Promise { const row = page.locator("tr", { has: page.getByText(targetType, { exact: true }) }).first(); await row.getByRole("button", { name: /set active/i }).click(); await page.getByTitle("Chat").click(); - await expect(page.getByText("PyRIT Attack")).toBeVisible({ timeout: 5_000 }); + await expect(page.getByTestId("new-attack-btn")).toBeVisible({ timeout: 5_000 }); } /** Navigate to an attack by opening the History view and clicking its row. */ diff --git a/frontend/src/components/Chat/ChatWindow.styles.ts b/frontend/src/components/Chat/ChatWindow.styles.ts index 24183e8d2f..504626d1f5 100644 --- a/frontend/src/components/Chat/ChatWindow.styles.ts +++ b/frontend/src/components/Chat/ChatWindow.styles.ts @@ -33,19 +33,27 @@ export const useChatWindowStyles = makeStyles({ gap: tokens.spacingHorizontalS, color: tokens.colorNeutralForeground2, fontSize: tokens.fontSizeBase300, - }, - targetInfo: { - display: 'flex', - alignItems: 'center', - gap: tokens.spacingHorizontalXS, + flex: '1 1 auto', + minWidth: 0, + overflow: 'hidden', }, noTarget: { color: tokens.colorNeutralForeground3, fontStyle: 'italic', + flexShrink: 0, }, ribbonActions: { display: 'flex', alignItems: 'center', gap: tokens.spacingHorizontalS, + flexShrink: 0, + }, + newAttackButton: { + flexShrink: 0, + }, + newAttackLabel: { + '@media (max-width: 600px)': { + display: 'none', + }, }, }) diff --git a/frontend/src/components/Chat/ChatWindow.test.tsx b/frontend/src/components/Chat/ChatWindow.test.tsx index 465d921c77..956dd01ecd 100644 --- a/frontend/src/components/Chat/ChatWindow.test.tsx +++ b/frontend/src/components/Chat/ChatWindow.test.tsx @@ -288,8 +288,11 @@ describe("ChatWindow Integration", () => { ); - expect(screen.getByText("PyRIT Attack")).toBeInTheDocument(); - expect(screen.getByText("New Attack")).toBeInTheDocument(); + // The ribbon no longer shows the "PyRIT Attack" prefix; the target + // badge stands on its own as the leftmost element. + expect(screen.queryByText("PyRIT Attack")).not.toBeInTheDocument(); + expect(screen.getByTestId("target-badge")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: /new attack/i })).toBeInTheDocument(); expect(screen.getByRole("textbox")).toBeInTheDocument(); }); @@ -321,8 +324,13 @@ describe("ChatWindow Integration", () => { ); - expect(screen.getByText(/OpenAIChatTarget/)).toBeInTheDocument(); - expect(screen.getByText(/gpt-4/)).toBeInTheDocument(); + // The target badge is the leftmost element. Its visible label + // includes the type and model. The same strings also appear in the + // tooltip body, so we query the badge specifically. + const badge = screen.getByTestId("target-badge"); + expect(badge).toHaveTextContent(/OpenAIChatTarget/); + expect(badge).toHaveTextContent(/gpt-4/); + expect(badge).toHaveAttribute("aria-label", expect.stringContaining(mockTarget.target_registry_name)); }); it("should show no-target message when target is null", () => { @@ -389,8 +397,9 @@ describe("ChatWindow Integration", () => { ); - expect(screen.getByText(/OpenAIChatTarget/)).toBeInTheDocument(); - expect(screen.queryByText(/gpt/)).not.toBeInTheDocument(); + const badge = screen.getByTestId("target-badge"); + expect(badge).toHaveTextContent(/OpenAIChatTarget/); + expect(badge).not.toHaveTextContent(/gpt/); }); // ----------------------------------------------------------------------- diff --git a/frontend/src/components/Chat/ChatWindow.tsx b/frontend/src/components/Chat/ChatWindow.tsx index f96fc85ee6..cb5109937a 100644 --- a/frontend/src/components/Chat/ChatWindow.tsx +++ b/frontend/src/components/Chat/ChatWindow.tsx @@ -2,7 +2,6 @@ import { useState, useRef, useEffect, useCallback } from 'react' import { Button, Text, - Badge, Tooltip, } from '@fluentui/react-components' import { AddRegular, PanelRightRegular } from '@fluentui/react-icons' @@ -10,6 +9,7 @@ import MessageList from './MessageList' import ChatInputArea from './ChatInputArea' import ConversationPanel from './ConversationPanel' import ConverterPanel from './ConverterPanel' +import TargetBadge from './TargetBadge' import type { PieceConversion } from './converterTypes' import { PIECE_TYPE_TO_DATA_TYPE } from './converterTypes' import LabelsBar from '../Labels/LabelsBar' @@ -513,17 +513,8 @@ export default function ChatWindow({
- PyRIT Attack {activeTarget ? ( -
- - - - {activeTarget.target_type} - {activeTarget.model_name ? ` (${activeTarget.model_name})` : ''} - - -
+ ) : ( No target selected @@ -544,15 +535,19 @@ export default function ChatWindow({ aria-label="Toggle conversations panel" /> - + + +
= ({ children }) => ( + {children} +) + +const baseTarget: TargetInstance = { + target_registry_name: 'azure_openai_gpt4o', + target_type: 'OpenAIChatTarget', + endpoint: 'https://example.openai.azure.com/openai/deployments/gpt-4o', + model_name: 'gpt-4o', + underlying_model_name: 'gpt-4o', + capabilities: { + supports_multi_turn: true, + supports_multi_message_pieces: true, + supports_json_schema: false, + supports_json_output: true, + supports_editable_history: true, + supports_system_prompt: true, + supported_input_modalities: ['text', 'image_path'], + supported_output_modalities: ['text'], + }, + target_specific_params: { + api_version: '2024-09-01', + }, +} + +describe('TargetBadge', () => { + it('renders the type and model name as the visible badge label', () => { + render( + + + + ) + const badge = screen.getByTestId('target-badge') + expect(badge).toHaveTextContent('OpenAIChatTarget (gpt-4o)') + }) + + it('uses the registry name in its accessible name so screen readers can disambiguate', () => { + render( + + + + ) + expect(screen.getByLabelText(/azure_openai_gpt4o/)).toBeInTheDocument() + }) + + it('renders only the type when no model name is set', () => { + render( + + + + ) + const badge = screen.getByTestId('target-badge') + expect(badge).toHaveTextContent('OpenAIChatTarget') + expect(badge).not.toHaveTextContent('gpt-4o') + }) + + it('does not throw on a target with no capabilities or params', () => { + const minimal: TargetInstance = { + target_registry_name: 't', + target_type: 'TextTarget', + } + expect(() => + render( + + + + ) + ).not.toThrow() + expect(screen.getByTestId('target-badge')).toHaveTextContent('TextTarget') + }) +}) diff --git a/frontend/src/components/Chat/TargetBadge.tsx b/frontend/src/components/Chat/TargetBadge.tsx new file mode 100644 index 0000000000..8df3943b14 --- /dev/null +++ b/frontend/src/components/Chat/TargetBadge.tsx @@ -0,0 +1,121 @@ +import { Badge, Text, Tooltip } from '@fluentui/react-components' +import type { TargetInstance } from '../../types' +import { useTargetBadgeStyles } from './TargetBadge.styles' + +interface TargetBadgeProps { + target: TargetInstance +} + +const CAPABILITY_LABELS: Array<{ key: keyof NonNullable; label: string }> = [ + { key: 'supports_multi_turn', label: 'Multi-turn' }, + { key: 'supports_multi_message_pieces', label: 'Multi-piece' }, + { key: 'supports_json_schema', label: 'JSON schema' }, + { key: 'supports_json_output', label: 'JSON output' }, + { key: 'supports_editable_history', label: 'Editable history' }, + { key: 'supports_system_prompt', label: 'System prompt' }, +] + +function formatParams(params?: Record | null): string { + if (!params) return '' + const lines: string[] = [] + for (const [key, val] of Object.entries(params)) { + if (val == null) continue + if (key === 'extra_body_parameters' && typeof val === 'object') { + for (const [k, v] of Object.entries(val as Record)) { + lines.push(`${k}: ${typeof v === 'object' ? JSON.stringify(v) : String(v)}`) + } + } else { + lines.push(`${key}: ${typeof val === 'object' ? JSON.stringify(val, null, 2) : String(val)}`) + } + } + return lines.join('\n') +} + +export default function TargetBadge({ target }: TargetBadgeProps) { + const styles = useTargetBadgeStyles() + const displayName = target.model_name + ? `${target.target_type} (${target.model_name})` + : target.target_type + const showUnderlying = + target.underlying_model_name && + target.model_name && + target.underlying_model_name !== target.model_name + const supportedCaps = target.capabilities + ? CAPABILITY_LABELS.filter(c => target.capabilities?.[c.key]).map(c => c.label) + : [] + const inputModalities = target.capabilities?.supported_input_modalities ?? [] + const outputModalities = target.capabilities?.supported_output_modalities ?? [] + const params = formatParams(target.target_specific_params) + + const tooltipContent = ( +
+
+ {target.target_registry_name} + {displayName} + {showUnderlying && ( + + Underlying model: {target.underlying_model_name} + + )} +
+ {target.endpoint && ( +
+ Endpoint + {target.endpoint} +
+ )} + {(inputModalities.length > 0 || outputModalities.length > 0) && ( +
+ Modalities + {inputModalities.length > 0 && ( + In: {inputModalities.join(', ')} + )} + {outputModalities.length > 0 && ( + Out: {outputModalities.join(', ')} + )} +
+ )} + {target.capabilities && ( +
+ Capabilities +
+ {supportedCaps.length > 0 ? ( + supportedCaps.map(cap => ( + + {cap} + + )) + ) : ( + None + )} +
+
+ )} + {params && ( +
+ Parameters +
{params}
+
+ )} +
+ ) + + return ( + + + + {displayName} + + + + ) +} diff --git a/frontend/src/components/Labels/LabelsBar.styles.ts b/frontend/src/components/Labels/LabelsBar.styles.ts index b4b285df82..b40fcf9334 100644 --- a/frontend/src/components/Labels/LabelsBar.styles.ts +++ b/frontend/src/components/Labels/LabelsBar.styles.ts @@ -6,8 +6,25 @@ export const useLabelsBarStyles = makeStyles({ alignItems: 'center', gap: tokens.spacingHorizontalXS, overflow: 'hidden', - flex: '1 1 0', - minWidth: 0, + // Always reserve enough room for the labels icon + count badge so it + // stays visible at any ribbon width. `minWidth` is the width of the + // icon button alone; the chip area beyond it grows when there's + // additional space. + flex: '1 1 auto', + minWidth: '60px', + position: 'relative', + }, + iconButton: { + flexShrink: 0, + }, + iconTooltipBody: { + whiteSpace: 'nowrap', + minWidth: 'max-content', + }, + inlineAddButton: { + flexShrink: 0, + minWidth: 'auto', + padding: `2px ${tokens.spacingHorizontalXS}`, }, labelsContainer: { display: 'flex', @@ -18,18 +35,26 @@ export const useLabelsBarStyles = makeStyles({ flex: '1 1 0', minWidth: 0, }, - overflowBadge: { - flexShrink: 0, - cursor: 'pointer', + measureRow: { + position: 'absolute', + visibility: 'hidden', + pointerEvents: 'none', + whiteSpace: 'nowrap', + display: 'inline-flex', + alignItems: 'center', + gap: tokens.spacingHorizontalXS, + top: 0, + left: 0, }, labelBadge: { - display: 'flex', + display: 'inline-flex', alignItems: 'center', gap: tokens.spacingHorizontalXXS, padding: `2px ${tokens.spacingHorizontalS}`, borderRadius: tokens.borderRadiusMedium, cursor: 'pointer', userSelect: 'none' as const, + flexShrink: 0, }, labelNormal: { backgroundColor: tokens.colorNeutralBackground3, @@ -52,6 +77,12 @@ export const useLabelsBarStyles = makeStyles({ padding: tokens.spacingVerticalM, minWidth: '250px', }, + popoverDivider: { + height: '1px', + backgroundColor: tokens.colorNeutralStroke2, + marginTop: tokens.spacingVerticalXS, + marginBottom: tokens.spacingVerticalXS, + }, inputRow: { display: 'flex', gap: tokens.spacingHorizontalXS, @@ -98,5 +129,6 @@ export const useLabelsBarStyles = makeStyles({ color: tokens.colorPaletteYellowForeground2, display: 'flex', alignItems: 'center', + flexShrink: 0, }, }) diff --git a/frontend/src/components/Labels/LabelsBar.test.tsx b/frontend/src/components/Labels/LabelsBar.test.tsx index dd7ae7850d..b30d09cbef 100644 --- a/frontend/src/components/Labels/LabelsBar.test.tsx +++ b/frontend/src/components/Labels/LabelsBar.test.tsx @@ -30,10 +30,12 @@ describe('LabelsBar', () => { ) - expect(screen.getByTestId('label-operator')).toBeInTheDocument() - expect(screen.getByTestId('label-operation')).toBeInTheDocument() - expect(screen.getByText('roakey')).toBeInTheDocument() - expect(screen.getByText('op_trash_panda')).toBeInTheDocument() + // The visible inline chips are the canonical render. The component + // also has an aria-hidden "measure" row with mirrored chips used + // purely to compute available width — query by data-testid so we + // don't accidentally match the hidden mirror. + expect(screen.getByTestId('label-operator')).toHaveTextContent('roakey') + expect(screen.getByTestId('label-operation')).toHaveTextContent('op_trash_panda') }) it('should show warning icon for dummy values', () => { @@ -541,4 +543,101 @@ describe('LabelsBar', () => { // team should have a remove button expect(screen.getByTestId('remove-label-team')).toBeInTheDocument() }) + + it('always renders a labels icon button with a count of total labels', () => { + // The labels icon is always present at the leftmost position with a + // badge showing the total label count, regardless of how many chips + // happen to fit inline. Clicking it opens a popover with the full + // label list and the add form. + render( + + + + ) + + const iconBtn = screen.getByTestId('labels-icon-btn') + expect(iconBtn).toBeInTheDocument() + expect(iconBtn).toHaveAttribute('aria-label', expect.stringContaining('3')) + expect(iconBtn).toHaveTextContent('3') + }) + + it('icon button opens a popover with all labels and the add form', async () => { + render( + + + + ) + + fireEvent.click(screen.getByTestId('labels-icon-btn')) + + await waitFor(() => { + expect(screen.getByTestId('popover-label-operator')).toBeInTheDocument() + }) + expect(screen.getByTestId('popover-label-operation')).toBeInTheDocument() + expect(screen.getByTestId('popover-label-team')).toBeInTheDocument() + expect(screen.getByTestId('new-label-key')).toBeInTheDocument() + expect(screen.getByTestId('new-label-value')).toBeInTheDocument() + expect(screen.getByTestId('confirm-add-label')).toBeInTheDocument() + }) + + it('hides only the chips that do not fit and never the icon button', async () => { + // Regression guard for the narrow-viewport ribbon bug: even when the + // available width is too small for every chip to fit, the labels icon + // (with full count) and as many chips as do fit should still render. + // Tests sub the layout properties to simulate a narrow ribbon. + const onChange = jest.fn() + const { container, rerender } = render( + +
+ +
+
+ ) + + const root = container.querySelector('[data-testid="labels-bar"]') as HTMLElement | null + if (!root) throw new Error('labels-bar not found') + Object.defineProperty(root, 'clientWidth', { configurable: true, value: 250 }) + // Each chip is 100 px wide → 2 chips fit after reserving room for the icon button. + const measure = root.querySelector('[aria-hidden="true"]') as HTMLElement | null + if (measure) { + const chips = Array.from(measure.querySelectorAll('[data-label-idx]')) as HTMLElement[] + for (const chip of chips) { + Object.defineProperty(chip, 'offsetWidth', { configurable: true, value: 100 }) + } + } + + rerender( + +
+ +
+
+ ) + + // The icon button stays visible with the full count (5 labels). + await waitFor(() => { + const btn = screen.getByTestId('labels-icon-btn') + expect(btn).toHaveAttribute('aria-label', expect.stringContaining('5')) + }) + + // Some chips render inline and some don't (the heuristic decides which); + // the important guarantee is that the popover is reachable for the rest. + fireEvent.click(screen.getByTestId('labels-icon-btn')) + await waitFor(() => { + expect(screen.getByTestId('popover-label-operator')).toBeInTheDocument() + }) + expect(screen.getByTestId('popover-label-extra')).toBeInTheDocument() + }) }) diff --git a/frontend/src/components/Labels/LabelsBar.tsx b/frontend/src/components/Labels/LabelsBar.tsx index c5362f38da..c8e1c5f866 100644 --- a/frontend/src/components/Labels/LabelsBar.tsx +++ b/frontend/src/components/Labels/LabelsBar.tsx @@ -13,6 +13,7 @@ import { AddRegular, DismissRegular, WarningRegular, + TagRegular, } from '@fluentui/react-icons' import { labelsApi } from '../../services/api' import { useLabelsBarStyles } from './LabelsBar.styles' @@ -119,43 +120,77 @@ export default function LabelsBar({ labels, onLabelsChange }: LabelsBarProps) { const suggestedKeys = Object.keys(existingLabels).filter(k => !(k in labels)) const suggestedValues = (editingLabel ? existingLabels[editingLabel] : existingLabels[newKey]) || [] - // Overflow detection: track which labels are visible - const containerRef = useRef(null) + // Layout: the labels icon (with total count badge) is always the first + // element on the bar. We then render as many full chips as fit, in + // declaration order. The icon's popover always shows the full list and + // the add form, so the user can reach everything regardless of how many + // chips are currently visible. The inline "+ Add" button is only shown + // when every chip already fits — otherwise the popover already covers + // the same flow. + const rootRef = useRef(null) + const measureRef = useRef(null) + const ICON_BUTTON_WIDTH_PX = 56 // labels icon + count badge + gap + const ADD_BUTTON_WIDTH_PX = 60 // "+ Add" button const [visibleCount, setVisibleCount] = useState(Infinity) const labelEntries = useMemo(() => Object.entries(labels), [labels]) useEffect(() => { - const el = containerRef.current - if (!el) return + const root = rootRef.current + const measure = measureRef.current + if (!root || !measure) return const check = () => { - const children = Array.from(el.children) as HTMLElement[] - if (children.length === 0) { setVisibleCount(Infinity); return } + const rootW = root.clientWidth + // jsdom and pre-layout: keep all chips so unit tests remain stable. + if (rootW === 0) { setVisibleCount(Infinity); return } - const containerRight = el.getBoundingClientRect().right + const chips = Array.from(measure.querySelectorAll('[data-label-idx]')) as HTMLElement[] + if (chips.length === 0) { setVisibleCount(Infinity); return } + + // Sum chip widths in order until we exceed available space. We + // measure against the off-screen `measure` row that has the same + // styling as the inline row but is allowed to lay out at full + // width, so each chip's offsetWidth reflects its natural size. + const gap = 4 + const reserved = ICON_BUTTON_WIDTH_PX + gap + const available = rootW - reserved + let used = 0 let count = 0 - for (const child of children) { - // Skip the overflow badge and add button (last elements) - if (child.dataset.labelIdx === undefined) continue - if (child.getBoundingClientRect().right <= containerRight + 2) { - count++ - } else { - break + for (const chip of chips) { + const next = used + chip.offsetWidth + (count > 0 ? gap : 0) + if (next > available) break + used = next + count++ + } + // If everything fits, also reserve room for the inline "+ Add" + // button. Drop the last chip(s) until "+ Add" fits too. + if (count === chips.length) { + const withAdd = used + gap + ADD_BUTTON_WIDTH_PX + if (withAdd > available) { + // Recompute with the +Add allowance baked in. + used = 0 + count = 0 + const availableWithAdd = available - ADD_BUTTON_WIDTH_PX - gap + for (const chip of chips) { + const next = used + chip.offsetWidth + (count > 0 ? gap : 0) + if (next > availableWithAdd) break + used = next + count++ + } } } setVisibleCount(count) } const observer = new ResizeObserver(check) - observer.observe(el) + observer.observe(root) + if (root.parentElement) observer.observe(root.parentElement) check() return () => observer.disconnect() }, [labelEntries]) - const overflowEntries = visibleCount < labelEntries.length - ? labelEntries.slice(visibleCount) - : [] + const allFit = visibleCount === Infinity || visibleCount >= labelEntries.length const renderLabelBadge = (key: string, value: string, idx: number) => { const isDummy = isDummyValue(key, value) @@ -227,8 +262,105 @@ export default function LabelsBar({ labels, onLabelsChange }: LabelsBarProps) { ) } + const renderLabelsList = () => ( +
+ {labelEntries.map(([key, value]) => { + const isDummy = isDummyValue(key, value) + const isRequired = key === 'operator' || key === 'operation' + return ( +
handleStartEdit(key)} + data-testid={`popover-label-${key}`} + style={{ flexShrink: 0 }} + > + {key}: + {value} + {!isRequired && ( +
+ ) + })} +
+ ) + + const renderAddForm = () => ( + <> +
+ { setNewKey(d.value.toLowerCase()); setError('') }} + onKeyDown={handleAddKeyDown} + data-testid="new-label-key" + /> + { setNewValue(d.value.toLowerCase()); setError('') }} + onKeyDown={handleAddKeyDown} + data-testid="new-label-value" + /> + +
+ {suggestedKeys.length > 0 && !newKey && ( + <> + Existing keys: +
+ {suggestedKeys.slice(0, 8).map(k => ( + setNewKey(k)} + >{k} + ))} +
+ + )} + {newKey && suggestedValues.length > 0 && ( + <> + Existing values for "{newKey}": +
+ {suggestedValues.slice(0, 8).map(v => ( + setNewValue(v)} + >{v} + ))} +
+ + )} + {error && {error}} + + ) + return ( -
+
{hasDummyValues && ( @@ -237,135 +369,85 @@ export default function LabelsBar({ labels, onLabelsChange }: LabelsBarProps) { )} -
- {labelEntries.map(([key, value], idx) => renderLabelBadge(key, value, idx))} - - {overflowEntries.length > 0 && ( - - - - +{overflowEntries.length} more - - - -
- All Labels -
- {labelEntries.map(([key, value]) => { - const isDummy = isDummyValue(key, value) - const isRequired = key === 'operator' || key === 'operation' - return ( -
handleStartEdit(key)} - style={{ flexShrink: 0 }} - > - {key}: - {value} - {!isRequired && ( -
- ) - })} -
-
-
-
- )} + {/* + Off-screen measurement row: contains every chip at its natural + width so we can compute how many fit. Hidden via CSS but laid out + normally; ResizeObserver triggers a re-measure on width changes. + */} + + {/* + Labels icon + total count. Always present, anchored leftmost. + Clicking opens a popover with the full label list and add form + — so even when every chip fits, this is still the canonical + entry point for editing/adding labels. + */} { setIsPopoverOpen(d.open); setError('') }}> - - - -
- Add Label -
- { setNewKey(d.value.toLowerCase()); setError('') }} - onKeyDown={handleAddKeyDown} - data-testid="new-label-key" - /> - { setNewValue(d.value.toLowerCase()); setError('') }} - onKeyDown={handleAddKeyDown} - data-testid="new-label-value" - /> - + + {`${labelEntries.length} label${labelEntries.length === 1 ? '' : 's'} — click to view or add`}
- {suggestedKeys.length > 0 && !newKey && ( - <> - Existing keys: -
- {suggestedKeys.slice(0, 8).map(k => ( - setNewKey(k)} - >{k} - ))} -
- - )} - {newKey && suggestedValues.length > 0 && ( - <> - Existing values for "{newKey}": -
- {suggestedValues.slice(0, 8).map(v => ( - setNewValue(v)} - >{v} - ))} -
- - )} - {error && {error}} -
-
-
+ } + relationship="label" + > + + + + +
+ All Labels + {renderLabelsList()} +
+ Add Label + {renderAddForm()} +
+ + + +
+ {labelEntries + .slice(0, visibleCount === Infinity ? labelEntries.length : visibleCount) + .map(([key, value], idx) => renderLabelBadge(key, value, idx))} + {allFit && labelEntries.length > 0 && ( + +
) }