Skip to content

Commit f869683

Browse files
alireza-aminzadehalireza-aminzadeh
committed
FIX associate converter parameter labels with their controls
1 parent 48384c0 commit f869683

2 files changed

Lines changed: 106 additions & 23 deletions

File tree

‎frontend/src/components/Chat/ConverterPanel/ConverterParams.test.tsx‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import { render, screen } from '@testing-library/react'
2+
import { FluentProvider, webLightTheme } from '@fluentui/react-components'
3+
import ConverterParams from './ConverterParams'
4+
import type { ConverterCatalogEntry } from '../../../types'
5+
6+
const TestWrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => (
7+
<FluentProvider theme={webLightTheme}>{children}</FluentProvider>
8+
)
9+
10+
const converter: ConverterCatalogEntry = {
11+
converter_type: 'VigenereConverter',
12+
supported_input_types: ['text'],
13+
supported_output_types: ['text'],
14+
is_llm_based: false,
15+
description: 'Vigenere cipher.',
16+
parameters: [
17+
{ name: 'key', type_name: 'str', required: true, default: null, choices: null, description: 'Cipher key.' },
18+
{ name: 'append_description', type_name: 'bool', required: false, default: 'false', choices: null, description: 'Append description.' },
19+
{ name: 'mode', type_name: 'str', required: false, default: 'fast', choices: ['fast', 'slow'], description: 'Speed mode.' },
20+
{ name: 'template_file_path', type_name: 'str', required: false, default: null, choices: null, description: 'Path to template.' },
21+
],
22+
}
23+
24+
function renderParams(overrides: Partial<React.ComponentProps<typeof ConverterParams>> = {}) {
25+
return render(
26+
<TestWrapper>
27+
<ConverterParams
28+
converter={converter}
29+
paramValues={{}}
30+
paramsExpanded
31+
showValidation={false}
32+
onParamChange={jest.fn()}
33+
onFileBrowse={jest.fn()}
34+
onToggleExpanded={jest.fn()}
35+
{...overrides}
36+
/>
37+
</TestWrapper>,
38+
)
39+
}
40+
41+
describe('ConverterParams accessible names', () => {
42+
it('exposes text, boolean, choice, and file controls by their visible parameter names', () => {
43+
renderParams()
44+
45+
expect(screen.getByRole('textbox', { name: 'key' })).toBeInTheDocument()
46+
expect(screen.getByRole('switch', { name: 'append_description' })).toBeInTheDocument()
47+
expect(screen.getByRole('combobox', { name: 'mode' })).toBeInTheDocument()
48+
expect(screen.getByRole('textbox', { name: 'template_file_path' })).toBeInTheDocument()
49+
})
50+
51+
it('keeps required, error, and type hint associated with the text control', () => {
52+
renderParams({ showValidation: true })
53+
54+
const key = screen.getByRole('textbox', { name: 'key' })
55+
expect(key).toBeRequired()
56+
expect(key).toBeInvalid()
57+
expect(key).toHaveAccessibleDescription(/Required/)
58+
expect(key).toHaveAccessibleDescription(/str/)
59+
})
60+
61+
it('keeps boolean state visible without using it as the accessible name', () => {
62+
renderParams({ paramValues: { append_description: 'true' } })
63+
64+
expect(screen.getByRole('switch', { name: 'append_description' })).toBeChecked()
65+
expect(screen.getByText('True')).toBeInTheDocument()
66+
})
67+
})

‎frontend/src/components/Chat/ConverterPanel/ConverterParams.tsx‎

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Button, Input, mergeClasses, Select, Switch, Text, Tooltip } from '@fluentui/react-components'
1+
import { Button, Field, Input, mergeClasses, Select, Switch, Text, Tooltip } from '@fluentui/react-components'
22
import { ChevronDownRegular, ChevronRightRegular, InfoRegular } from '@fluentui/react-icons'
33
import type { ConverterCatalogEntry, Parameter } from '../../../types'
44
import { useConverterPanelStyles } from './ConverterPanel.styles'
@@ -68,6 +68,23 @@ function ConverterParameterViewer({ param, value, isMissing, onChange }: ParamIn
6868
)
6969
}
7070

71+
function ParameterNameLabel({ param }: { param: Parameter }) {
72+
const styles = useConverterPanelStyles()
73+
74+
return (
75+
<span className={styles.paramLabel}>
76+
{param.name}
77+
{param.description && (
78+
<Tooltip content={param.description} relationship="description">
79+
<span className={styles.paramInfo} onClick={(e) => e.preventDefault()}>
80+
<InfoRegular fontSize={12} />
81+
</span>
82+
</Tooltip>
83+
)}
84+
</span>
85+
)
86+
}
87+
7188
export interface ConverterParamsProps {
7289
converter: ConverterCatalogEntry
7390
paramValues: Record<string, string>
@@ -97,37 +114,36 @@ export default function ConverterParams({ converter, paramValues, paramsExpanded
97114
</Button>
98115
{paramsExpanded && (converter.parameters ?? []).map((param) => {
99116
const isMissing = showValidation && param.required && !paramValues[param.name]?.trim()
117+
const isChecked = (paramValues[param.name] ?? (typeof param.default === 'string' ? param.default : 'false')).toLowerCase() === 'true'
118+
const typeHint = param.type_name !== 'bool' && !param.choices ? param.type_name : undefined
119+
100120
return (
101-
<div key={param.name} className={styles.paramBlock}>
102-
<span className={styles.paramLabel}>
103-
<Text size={200} weight="semibold">{param.name}{param.required ? ' *' : ''}</Text>
104-
{param.description && (
105-
<Tooltip content={param.description} relationship="description">
106-
<span className={styles.paramInfo}><InfoRegular fontSize={12} /></span>
107-
</Tooltip>
108-
)}
109-
</span>
121+
<Field
122+
key={param.name}
123+
className={styles.paramBlock}
124+
label={<ParameterNameLabel param={param} />}
125+
required={param.required}
126+
validationMessage={isMissing ? 'Required' : undefined}
127+
validationState={isMissing ? 'error' : undefined}
128+
hint={typeHint}
129+
>
110130
{param.type_name === 'bool' ? (
111-
<Switch
112-
checked={(paramValues[param.name] ?? (typeof param.default === 'string' ? param.default : 'false')).toLowerCase() === 'true'}
113-
onChange={(_, data) => onParamChange(param.name, data.checked ? 'true' : 'false')}
114-
label={(paramValues[param.name] ?? (typeof param.default === 'string' ? param.default : 'false')).toLowerCase() === 'true' ? 'True' : 'False'}
115-
data-testid={`param-${param.name}`}
116-
/>
131+
<div className={styles.filePickerRow}>
132+
<Switch
133+
checked={isChecked}
134+
onChange={(_, data) => onParamChange(param.name, data.checked ? 'true' : 'false')}
135+
data-testid={`param-${param.name}`}
136+
/>
137+
<Text size={200} aria-hidden="true">{isChecked ? 'True' : 'False'}</Text>
138+
</div>
117139
) : param.choices ? (
118140
<ConverterParameterChoiceViewer param={param} value={paramValues[param.name]} isMissing={isMissing} onChange={onParamChange} />
119141
) : /path|file/i.test(param.name) || /path|file/i.test(param.description ?? '') ? (
120142
<ParameterFileViewer param={param} value={paramValues[param.name]} isMissing={isMissing} onChange={onParamChange} onBrowse={onFileBrowse} />
121143
) : (
122144
<ConverterParameterViewer param={param} value={paramValues[param.name]} isMissing={isMissing} onChange={onParamChange} />
123145
)}
124-
{isMissing && (
125-
<Text size={100} className={styles.paramErrorText}>Required</Text>
126-
)}
127-
{param.type_name !== 'bool' && !param.choices && (
128-
<Text size={100} className={styles.hintText}>{param.type_name}</Text>
129-
)}
130-
</div>
146+
</Field>
131147
)
132148
})}
133149
</div>

0 commit comments

Comments
 (0)