Skip to content

FIX: associate converter parameter controls with visible labels #2561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Sylvester Kaczmarek (sylvesterkaczmarek) wants to merge 5 commits into
base: main
Choose a base branch
from
+88 −10
Open

FIX: associate converter parameter controls with visible labels #2561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
791329c
FIX: associate converter parameters with visible labels
sylvesterkaczmarek Sep 3, 2026
bd3430b
TEST: cover converter parameter accessible names
sylvesterkaczmarek Sep 3, 2026
6c65892
TEST: use accessible description matcher
sylvesterkaczmarek Sep 3, 2026
fe699da
FIX: expose converter parameter required state
sylvesterkaczmarek Sep 3, 2026
b59e1be
TEST: verify required parameter semantics
sylvesterkaczmarek Sep 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions frontend/src/components/Chat/ConverterPanel/ConverterParams.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import React from 'react'
import { render, screen } from '@testing-library/react'
import { FluentProvider, webLightTheme } from '@fluentui/react-components'
import type { ConverterCatalogEntry } from '../../../types'
import ConverterParams from './ConverterParams'

const converter: ConverterCatalogEntry = {
converter_type: 'TestConverter',
supported_input_types: ['text'],
supported_output_types: ['text'],
is_llm_based: false,
parameters: [
{ name: 'key', type_name: 'str', required: true, default: null, description: 'Cipher key' },
{ name: 'append_description', type_name: 'bool', required: false, default: 'false' },
{ name: 'mode', type_name: 'str', required: false, default: 'safe', choices: ['safe', 'strict'] },
{ name: 'image_path', type_name: 'str', required: false, default: null, description: 'Image file path' },
],
}

function renderParams(showValidation = false) {
return render(
<FluentProvider theme={webLightTheme}>
<ConverterParams
converter={converter}
paramValues={{}}
paramsExpanded
showValidation={showValidation}
onParamChange={jest.fn()}
onFileBrowse={jest.fn()}
onToggleExpanded={jest.fn()}
/>
</FluentProvider>,
)
}

describe('ConverterParams accessibility', () => {
it('associates visible parameter names with every control variant', () => {
renderParams()

expect(screen.getByRole('textbox', { name: 'key' })).toBeInTheDocument()
expect(screen.getByRole('switch', { name: 'append_description' })).toBeInTheDocument()
expect(screen.getByRole('combobox', { name: 'mode' })).toBeInTheDocument()
expect(screen.getByRole('textbox', { name: 'image_path' })).toBeInTheDocument()
})

it('connects required validation text and state to the input', () => {
renderParams(true)

const keyInput = screen.getByRole('textbox', { name: 'key' })
expect(keyInput).toHaveAttribute('aria-required', 'true')
expect(keyInput).toHaveAttribute('aria-invalid', 'true')
expect(keyInput).toHaveAccessibleDescription(/Required/)
})
})
44 changes: 34 additions & 10 deletions frontend/src/components/Chat/ConverterPanel/ConverterParams.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,20 @@ interface ParamInputProps {
param: Parameter
value: string
isMissing: boolean
labelId: string
describedBy?: string
onChange: (name: string, value: string) => void
}

function ConverterParameterChoiceViewer({ param, value, onChange }: ParamInputProps) {
function ConverterParameterChoiceViewer({ param, value, labelId, describedBy, onChange }: ParamInputProps) {
const stringDefault = typeof param.default === 'string' ? param.default : ''
return (
<Select
value={value ?? stringDefault}
onChange={(_, data) => onChange(param.name, data.value)}
aria-labelledby={labelId}
aria-describedby={describedBy}
aria-required={param.required || undefined}
data-testid={`param-${param.name}`}
>
{(param.choices ?? []).map((choice) => (
Expand All @@ -27,7 +32,7 @@ function ConverterParameterChoiceViewer({ param, value, onChange }: ParamInputPr
)
}

function ParameterFileViewer({ param, value, isMissing, onChange, onBrowse }: ParamInputProps & { onBrowse: (name: string) => void }) {
function ParameterFileViewer({ param, value, isMissing, labelId, describedBy, onChange, onBrowse }: ParamInputProps & { onBrowse: (name: string) => void }) {
const styles = useConverterPanelStyles()
const stringDefault = typeof param.default === 'string' ? param.default : 'Select a file...'

Expand All @@ -37,6 +42,10 @@ function ParameterFileViewer({ param, value, isMissing, onChange, onBrowse }: Pa
value={value ?? ''}
placeholder={stringDefault}
onChange={(_, data) => onChange(param.name, data.value)}
aria-labelledby={labelId}
aria-describedby={describedBy}
aria-required={param.required || undefined}
aria-invalid={isMissing || undefined}
className={isMissing ? styles.paramInputError : undefined}
data-testid={`param-${param.name}`}
/>
Expand All @@ -53,7 +62,7 @@ function ParameterFileViewer({ param, value, isMissing, onChange, onBrowse }: Pa
)
}

function ConverterParameterViewer({ param, value, isMissing, onChange }: ParamInputProps) {
function ConverterParameterViewer({ param, value, isMissing, labelId, describedBy, onChange }: ParamInputProps) {
const styles = useConverterPanelStyles()
const stringDefault = typeof param.default === 'string' ? param.default : undefined

Expand All @@ -62,6 +71,10 @@ function ConverterParameterViewer({ param, value, isMissing, onChange }: ParamIn
value={value ?? ''}
placeholder={stringDefault}
onChange={(_, data) => onChange(param.name, data.value)}
aria-labelledby={labelId}
aria-describedby={describedBy}
aria-required={param.required || undefined}
aria-invalid={isMissing || undefined}
className={isMissing ? styles.paramInputError : undefined}
data-testid={`param-${param.name}`}
/>
Expand Down Expand Up @@ -97,10 +110,17 @@ export default function ConverterParams({ converter, paramValues, paramsExpanded
</Button>
{paramsExpanded && (converter.parameters ?? []).map((param) => {
const isMissing = showValidation && param.required && !paramValues[param.name]?.trim()
const labelId = `converter-param-${param.name}-label`
const errorId = isMissing ? `converter-param-${param.name}-error` : undefined
const hintId = param.type_name !== 'bool' && !param.choices ? `converter-param-${param.name}-hint` : undefined
const describedBy = [errorId, hintId].filter(Boolean).join(' ') || undefined
return (
<div key={param.name} className={styles.paramBlock}>
<span className={styles.paramLabel}>
<Text size={200} weight="semibold">{param.name}{param.required ? ' *' : ''}</Text>
<span id={labelId} className={styles.paramLabel}>
<Text size={200} weight="semibold">
{param.name}
{param.required ? <span aria-hidden="true"> *</span> : null}
</Text>
{param.description && (
<Tooltip content={param.description} relationship="description">
<span className={styles.paramInfo}><InfoRegular fontSize={12} /></span>
Expand All @@ -112,20 +132,24 @@ export default function ConverterParams({ converter, paramValues, paramsExpanded
checked={(paramValues[param.name] ?? (typeof param.default === 'string' ? param.default : 'false')).toLowerCase() === 'true'}
onChange={(_, data) => onParamChange(param.name, data.checked ? 'true' : 'false')}
label={(paramValues[param.name] ?? (typeof param.default === 'string' ? param.default : 'false')).toLowerCase() === 'true' ? 'True' : 'False'}
aria-labelledby={labelId}
aria-describedby={describedBy}
aria-required={param.required || undefined}
aria-invalid={isMissing || undefined}
data-testid={`param-${param.name}`}
/>
) : param.choices ? (
<ConverterParameterChoiceViewer param={param} value={paramValues[param.name]} isMissing={isMissing} onChange={onParamChange} />
<ConverterParameterChoiceViewer param={param} value={paramValues[param.name]} isMissing={isMissing} labelId={labelId} describedBy={describedBy} onChange={onParamChange} />
) : /path|file/i.test(param.name) || /path|file/i.test(param.description ?? '') ? (
<ParameterFileViewer param={param} value={paramValues[param.name]} isMissing={isMissing} onChange={onParamChange} onBrowse={onFileBrowse} />
<ParameterFileViewer param={param} value={paramValues[param.name]} isMissing={isMissing} labelId={labelId} describedBy={describedBy} onChange={onParamChange} onBrowse={onFileBrowse} />
) : (
<ConverterParameterViewer param={param} value={paramValues[param.name]} isMissing={isMissing} onChange={onParamChange} />
<ConverterParameterViewer param={param} value={paramValues[param.name]} isMissing={isMissing} labelId={labelId} describedBy={describedBy} onChange={onParamChange} />
)}
{isMissing && (
<Text size={100} className={styles.paramErrorText}>Required</Text>
<Text id={errorId} size={100} className={styles.paramErrorText}>Required</Text>
)}
{param.type_name !== 'bool' && !param.choices && (
<Text size={100} className={styles.hintText}>{param.type_name}</Text>
<Text id={hintId} size={100} className={styles.hintText}>{param.type_name}</Text>
)}
</div>
)
Expand Down