From ec0e7c156a954d9c48f8feb47d50b6b3a54c4d4f Mon Sep 17 00:00:00 2001 From: Roman Lutz Date: Tue, 12 May 2026 21:21:07 -0700 Subject: [PATCH 1/3] FIX Redesign chat ribbon for clarity and narrow viewports The chat ribbon had several issues: - Redundant "PyRIT Attack ->" prefix added chrome with no information. - The target badge showed only type+model with a plain-text tooltip containing the registry name; none of the rich metadata (endpoint, capabilities, modalities, parameters) was reachable without leaving the chat view. - Below ~700 px the right-side actions (panel toggle and "+ New Attack") got pushed off-screen entirely. - Below ~500 px label chips were silently clipped without any indicator, and the inline "+ Add" button became unreachable. This change: 1. Drops the "PyRIT Attack" text and arrow. The target stands on its own as the leftmost element. 2. Introduces a TargetBadge component that shows the type + model in the badge and exposes the rest of the metadata via a rich tooltip: registry name, endpoint, capabilities (as outlined badges), input/output modalities, and any target_specific_params. Fluent's default Tooltip surface is hard-capped at max-width: 240 px, which truncated the endpoint and parameter lines; we override the surface via the Tooltip `content` slot's className with `width: max-content; min-width: 420 px; max-width: min(800px, calc(100vw - 64px))` so the surface grows to fit the longest line and only wraps if a deeply-nested deployment URL exceeds the cap. 3. Reworks LabelsBar into a single unified layout that scales gracefully: - A labels icon (TagRegular) with a count badge of the *total* number of labels is always present at the leftmost position, regardless of how many chips happen to fit. The bar's root reserves min-width: 60 px so the icon button stays visible at any ribbon width (previously the bar shrank to zero behind the target badge and clipped the icon at <500 px). Hovering shows a full tooltip ("N labels - click to view or add"); clicking opens a popover that contains the complete label list and the add form, so every action is always reachable. - As many full chips as fit are rendered inline in declaration order. The fit calculation uses an off-screen "measure" row so each chip's natural width is known before layout decisions are made (the previous getBoundingClientRect approach interacted poorly with overflow-hidden ancestors). - The inline "+ Add" button is shown only when every chip already fits (otherwise the popover already covers the same flow), and it sits next to the last chip rather than being right-aligned. 4. Hardens the right-side ribbon actions: flexShrink: 0 on the actions group so neither the panel toggle nor "+ New Attack" can ever be pushed off-screen, and a media query hides the "New Attack" text below 600 px - the button collapses to icon-only, still labelled by aria-label and tooltip. Tests: - New TargetBadge.test.tsx: 4 tests covering visible badge text, accessible name, no-model variant, and minimal-target rendering. - LabelsBar.test.tsx: 3 new tests asserting that the labels icon is always present with the total count, that its popover surfaces every label and the add form, and that the icon stays visible even when chips have to be hidden because they don't fit. The previous compact-mode and "+N more" tests are removed since both layouts have been replaced by the unified design. - ChatWindow.test.tsx: existing assertions updated to reflect that "PyRIT Attack" is gone and the target is queried via the new data-testid="target-badge". - All 568 frontend tests pass; lint and typecheck are clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/components/Chat/ChatWindow.styles.ts | 18 +- .../src/components/Chat/ChatWindow.test.tsx | 21 +- frontend/src/components/Chat/ChatWindow.tsx | 36 +- .../src/components/Chat/TargetBadge.styles.ts | 83 ++++ .../src/components/Chat/TargetBadge.test.tsx | 76 ++++ frontend/src/components/Chat/TargetBadge.tsx | 121 ++++++ .../src/components/Labels/LabelsBar.styles.ts | 44 ++- .../src/components/Labels/LabelsBar.test.tsx | 107 ++++- frontend/src/components/Labels/LabelsBar.tsx | 368 +++++++++++------- 9 files changed, 690 insertions(+), 184 deletions(-) create mode 100644 frontend/src/components/Chat/TargetBadge.styles.ts create mode 100644 frontend/src/components/Chat/TargetBadge.test.tsx create mode 100644 frontend/src/components/Chat/TargetBadge.tsx 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 a2666fc18c..16312a3ebc 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 43b3b438e0..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 @@ -541,17 +532,22 @@ export default function ChatWindow({ onClick={() => setIsPanelOpen(!isPanelOpen)} disabled={!attackResultId} data-testid="toggle-panel-btn" + 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 && ( + +
) } From 7136ca27672220690b62214ca46fedb0412bf002 Mon Sep 17 00:00:00 2001 From: romanlutz Date: Fri, 15 May 2026 16:12:15 -0700 Subject: [PATCH 2/3] TEST: Update Playwright specs to wait on new-attack-btn instead of removed "PyRIT Attack" header The chat ribbon redesign in this branch dropped the literal "PyRIT Attack" prefix text in favour of the target badge as the leftmost element. Twelve Playwright tests across seven spec files were using page.getByText("PyRIT Attack") as a "chat is loaded" sentinel; they now wait on page.getByTestId("new-attack-btn"), which is the equivalent always-present ribbon element under the new layout. The "should display PyRIT header" test in chat.spec.ts is renamed to "should display chat ribbon" to match what it actually asserts. The bounding-box anchor in accessibility.spec.ts switches to the same testid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- frontend/e2e/accessibility.spec.ts | 8 ++++---- frontend/e2e/api.spec.ts | 2 +- frontend/e2e/chat.spec.ts | 8 ++++---- frontend/e2e/config.spec.ts | 2 +- frontend/e2e/converters.spec.ts | 2 +- frontend/e2e/errors.spec.ts | 6 +++--- frontend/e2e/flows.spec.ts | 2 +- 7 files changed, 15 insertions(+), 15 deletions(-) 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..4de497d6fb 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"); diff --git a/frontend/e2e/config.spec.ts b/frontend/e2e/config.spec.ts index cf2ee36ce8..2fb0d151ea 100644 --- a/frontend/e2e/config.spec.ts +++ b/frontend/e2e/config.spec.ts @@ -232,7 +232,7 @@ test.describe("Target Config ↔ Chat Navigation", () => { // Navigate back to chat await page.getByTitle("Chat").click(); - await expect(page.getByText("PyRIT Attack")).toBeVisible(); + await expect(page.getByTestId("new-attack-btn")).toBeVisible(); // Chat should show the active target type await expect(page.getByText("OpenAIChatTarget")).toBeVisible(); 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. */ From 76c84fc4a46b68d7cf2901c93df7660b5e28f198 Mon Sep 17 00:00:00 2001 From: romanlutz Date: Sat, 16 May 2026 21:04:09 -0700 Subject: [PATCH 3/3] TEST: Scope target-text assertions to the badge to avoid tooltip duplicates Three Playwright tests asserted that strings like `getByText(\"OpenAIChatTarget\")` were visible in the chat ribbon. With the new `TargetBadge`, Fluent's `Tooltip` eagerly renders its content (containing the same `displayName`) into the DOM, which causes Playwright strict-mode locators to resolve to two elements and fail. Scope the assertions to `page.getByTestId(\"target-badge\").toContainText(...)` so they only match the visible badge, not the (hidden) tooltip copy. Affected tests: - chat.spec.ts::Chat Functionality / should display target info after activation - chat.spec.ts::Target type scenarios / should activate image target and show it in chat ribbon - config.spec.ts::Target Config <-> Chat Navigation / should display active target info in chat after setting it Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- frontend/e2e/chat.spec.ts | 16 ++++++++++++---- frontend/e2e/config.spec.ts | 9 ++++++--- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/frontend/e2e/chat.spec.ts b/frontend/e2e/chat.spec.ts index 4de497d6fb..54eef5291b 100644 --- a/frontend/e2e/chat.spec.ts +++ b/frontend/e2e/chat.spec.ts @@ -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 2fb0d151ea..c62edd1825 100644 --- a/frontend/e2e/config.spec.ts +++ b/frontend/e2e/config.spec.ts @@ -234,9 +234,12 @@ test.describe("Target Config ↔ Chat Navigation", () => { await page.getByTitle("Chat").click(); await expect(page.getByTestId("new-attack-btn")).toBeVisible(); - // Chat should show the active target type - await expect(page.getByText("OpenAIChatTarget")).toBeVisible(); - await expect(page.getByText(/gpt-4o/)).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 }) => {