Skip to content

Commit 79c36c8

Browse files
jaaydenhjaaydenh
committed
fix: improvements to dynamic parameter handling
1 parent 9c94e2a commit 79c36c8

File tree

2 files changed

+151
-119
lines changed

2 files changed

+151
-119
lines changed

‎site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx

+145-92
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,17 @@ import {
3232
TooltipProvider,
3333
TooltipTrigger,
3434
} from "components/Tooltip/Tooltip";
35+
import { useDebouncedValue } from "hooks/debounce";
36+
import { useEffectEvent } from "hooks/hookPolyfills";
3537
import { Info, LinkIcon, Settings, TriangleAlert } from "lucide-react";
36-
import { type FC, useId, useState } from "react";
38+
import { type FC, useEffect, useId, useRef, useState } from "react";
3739
import type { AutofillBuildParameter } from "utils/richParameters";
3840
import * as Yup from "yup";
3941

4042
export interface DynamicParameterProps {
4143
parameter: PreviewParameter;
4244
value?: string;
43-
onChange: (value: string) => void;
45+
onChange: (value: string) => Promise<void>;
4446
disabled?: boolean;
4547
isPreset?: boolean;
4648
autofill: boolean;
@@ -68,13 +70,24 @@ export const DynamicParameter: FC<DynamicParameterProps> = ({
6870
autofill={autofill}
6971
/>
7072
<div className="max-w-lg">
71-
<ParameterField
72-
id={id}
73-
parameter={parameter}
74-
value={value}
75-
onChange={onChange}
76-
disabled={disabled}
77-
/>
73+
{parameter.form_type === "input" ||
74+
parameter.form_type === "textarea" ? (
75+
<DebouncedParameterField
76+
id={id}
77+
parameter={parameter}
78+
value={value}
79+
onChange={onChange}
80+
disabled={disabled}
81+
/>
82+
) : (
83+
<ParameterField
84+
id={id}
85+
parameter={parameter}
86+
value={value}
87+
onChange={onChange}
88+
disabled={disabled}
89+
/>
90+
)}
7891
</div>
7992
{parameter.diagnostics.length > 0 && (
8093
<ParameterDiagnostics diagnostics={parameter.diagnostics} />
@@ -187,15 +200,15 @@ const ParameterLabel: FC<ParameterLabelProps> = ({
187200
);
188201
};
189202

190-
interface ParameterFieldProps {
203+
interface DebouncedParameterFieldProps {
191204
parameter: PreviewParameter;
192205
value?: string;
193-
onChange: (value: string) => void;
206+
onChange: (value: string) => Promise<void>;
194207
disabled?: boolean;
195208
id: string;
196209
}
197210

198-
const ParameterField: FC<ParameterFieldProps> = ({
211+
const DebouncedParameterField: FC<DebouncedParameterFieldProps> = ({
199212
parameter,
200213
value,
201214
onChange,
@@ -205,16 +218,96 @@ const ParameterField: FC<ParameterFieldProps> = ({
205218
const [localValue, setLocalValue] = useState(
206219
value !== undefined ? value : validValue(parameter.value),
207220
);
208-
if (value !== undefined && value !== localValue) {
209-
setLocalValue(value);
221+
const debouncedLocalValue = useDebouncedValue(localValue, 500);
222+
const onChangeEvent = useEffectEvent(onChange);
223+
// prevDebouncedValueRef is to prevent calling the onChangeEvent on the initial render
224+
const prevDebouncedValueRef = useRef<string | undefined>();
225+
226+
useEffect(() => {
227+
if (prevDebouncedValueRef.current !== undefined) {
228+
onChangeEvent(debouncedLocalValue);
229+
}
230+
231+
prevDebouncedValueRef.current = debouncedLocalValue;
232+
}, [debouncedLocalValue, onChangeEvent]);
233+
234+
switch (parameter.form_type) {
235+
case "textarea":
236+
return (
237+
<Textarea
238+
id={id}
239+
className="max-w-2xl"
240+
value={localValue}
241+
onChange={(e) => {
242+
setLocalValue(e.target.value);
243+
}}
244+
onInput={(e) => {
245+
const target = e.currentTarget;
246+
target.style.maxHeight = "700px";
247+
target.style.height = `${target.scrollHeight}px`;
248+
}}
249+
disabled={disabled}
250+
placeholder={parameter.styling?.placeholder}
251+
required={parameter.required}
252+
/>
253+
);
254+
255+
case "input": {
256+
const inputType = parameter.type === "number" ? "number" : "text";
257+
const inputProps: Record<string, unknown> = {};
258+
259+
if (parameter.type === "number") {
260+
const validations = parameter.validations[0] || {};
261+
const { validation_min, validation_max } = validations;
262+
263+
if (validation_min !== null) {
264+
inputProps.min = validation_min;
265+
}
266+
267+
if (validation_max !== null) {
268+
inputProps.max = validation_max;
269+
}
270+
}
271+
272+
return (
273+
<Input
274+
id={id}
275+
type={inputType}
276+
value={localValue}
277+
onChange={(e) => {
278+
setLocalValue(e.target.value);
279+
}}
280+
disabled={disabled}
281+
required={parameter.required}
282+
placeholder={parameter.styling?.placeholder}
283+
{...inputProps}
284+
/>
285+
);
286+
}
210287
}
288+
};
211289

290+
interface ParameterFieldProps {
291+
parameter: PreviewParameter;
292+
value?: string;
293+
onChange: (value: string) => Promise<void>;
294+
disabled?: boolean;
295+
id: string;
296+
}
297+
298+
const ParameterField: FC<ParameterFieldProps> = ({
299+
parameter,
300+
value,
301+
onChange,
302+
disabled,
303+
id,
304+
}) => {
212305
switch (parameter.form_type) {
213306
case "dropdown":
214307
return (
215308
<Select
216309
onValueChange={onChange}
217-
value={localValue}
310+
value={value}
218311
disabled={disabled}
219312
required={parameter.required}
220313
>
@@ -234,7 +327,15 @@ const ParameterField: FC<ParameterFieldProps> = ({
234327
);
235328

236329
case "multi-select": {
237-
const values = parseStringArrayValue(localValue ?? "");
330+
const parsedValues = parseStringArrayValue(value ?? "");
331+
332+
if (parsedValues.error) {
333+
return (
334+
<p className="text-sm text-content-destructive">
335+
{parsedValues.error}
336+
</p>
337+
);
338+
}
238339

239340
// Map parameter options to MultiSelectCombobox options format
240341
const options: Option[] = parameter.options.map((opt) => ({
@@ -247,7 +348,7 @@ const ParameterField: FC<ParameterFieldProps> = ({
247348
parameter.options.map((opt) => [opt.value.value, opt.name]),
248349
);
249350

250-
const selectedOptions: Option[] = values.map((val) => {
351+
const selectedOptions: Option[] = parsedValues.values.map((val) => {
251352
return {
252353
value: val,
253354
label: optionMap.get(val) || val,
@@ -279,13 +380,21 @@ const ParameterField: FC<ParameterFieldProps> = ({
279380
}
280381

281382
case "tag-select": {
282-
const values = parseStringArrayValue(localValue ?? "");
383+
const parsedValues = parseStringArrayValue(value ?? "");
384+
385+
if (parsedValues.error) {
386+
return (
387+
<p className="text-sm text-content-destructive">
388+
{parsedValues.error}
389+
</p>
390+
);
391+
}
283392

284393
return (
285394
<TagInput
286395
id={id}
287396
label={parameter.display_name || parameter.name}
288-
values={values}
397+
values={parsedValues.values}
289398
onChange={(values) => {
290399
onChange(JSON.stringify(values));
291400
}}
@@ -297,7 +406,7 @@ const ParameterField: FC<ParameterFieldProps> = ({
297406
return (
298407
<Switch
299408
id={id}
300-
checked={localValue === "true"}
409+
checked={value === "true"}
301410
onCheckedChange={(checked) => {
302411
onChange(checked ? "true" : "false");
303412
}}
@@ -307,11 +416,7 @@ const ParameterField: FC<ParameterFieldProps> = ({
307416

308417
case "radio":
309418
return (
310-
<RadioGroup
311-
onValueChange={onChange}
312-
disabled={disabled}
313-
value={localValue}
314-
>
419+
<RadioGroup onValueChange={onChange} disabled={disabled} value={value}>
315420
{parameter.options.map((option) => (
316421
<div
317422
key={option.value.value}
@@ -337,7 +442,7 @@ const ParameterField: FC<ParameterFieldProps> = ({
337442
<div className="flex items-center space-x-2">
338443
<Checkbox
339444
id={id}
340-
checked={localValue === "true"}
445+
checked={value === "true"}
341446
onCheckedChange={(checked) => {
342447
onChange(checked ? "true" : "false");
343448
}}
@@ -353,9 +458,8 @@ const ParameterField: FC<ParameterFieldProps> = ({
353458
<Slider
354459
id={id}
355460
className="mt-2"
356-
value={[Number(localValue)]}
461+
value={[Number(value)]}
357462
onValueChange={([value]) => {
358-
setLocalValue(value.toString());
359463
onChange(value.toString());
360464
}}
361465
min={parameter.validations[0]?.validation_min ?? 0}
@@ -365,79 +469,32 @@ const ParameterField: FC<ParameterFieldProps> = ({
365469
<span className="w-4 font-medium">{parameter.value.value}</span>
366470
</div>
367471
);
368-
369-
case "textarea":
370-
return (
371-
<Textarea
372-
id={id}
373-
className="max-w-2xl"
374-
value={localValue}
375-
onChange={(e) => {
376-
setLocalValue(e.target.value);
377-
onChange(e.target.value);
378-
}}
379-
onInput={(e) => {
380-
const target = e.currentTarget;
381-
target.style.maxHeight = "700px";
382-
target.style.height = `${target.scrollHeight}px`;
383-
}}
384-
disabled={disabled}
385-
placeholder={parameter.styling?.placeholder}
386-
required={parameter.required}
387-
/>
388-
);
389-
390-
case "input": {
391-
const inputType = parameter.type === "number" ? "number" : "text";
392-
const inputProps: Record<string, unknown> = {};
393-
394-
if (parameter.type === "number") {
395-
const validations = parameter.validations[0] || {};
396-
const { validation_min, validation_max } = validations;
397-
398-
if (validation_min !== null) {
399-
inputProps.min = validation_min;
400-
}
401-
402-
if (validation_max !== null) {
403-
inputProps.max = validation_max;
404-
}
405-
}
406-
407-
return (
408-
<Input
409-
id={id}
410-
type={inputType}
411-
value={localValue}
412-
onChange={(e) => {
413-
setLocalValue(e.target.value);
414-
onChange(e.target.value);
415-
}}
416-
disabled={disabled}
417-
required={parameter.required}
418-
placeholder={parameter.styling?.placeholder}
419-
{...inputProps}
420-
/>
421-
);
422-
}
423472
}
424473
};
425474

426-
const parseStringArrayValue = (value: string): string[] => {
427-
let values: string[] = [];
475+
type ParsedValues = {
476+
values: string[];
477+
error: string;
478+
};
479+
480+
const parseStringArrayValue = (value: string): ParsedValues => {
481+
const parsedValues: ParsedValues = {
482+
values: [],
483+
error: "",
484+
};
428485

429486
if (value) {
430487
try {
431488
const parsed = JSON.parse(value);
432489
if (Array.isArray(parsed)) {
433-
values = parsed;
490+
parsedValues.values = parsed;
434491
}
435492
} catch (e) {
436-
console.error("Error parsing parameter of type list(string)", e);
493+
parsedValues.error = `Error parsing parameter of type list(string), ${e}`;
437494
}
438495
}
439496

440-
return values;
497+
return parsedValues;
441498
};
442499

443500
interface OptionDisplayProps {
@@ -542,10 +599,6 @@ const isValidParameterOption = (
542599
values = parsed;
543600
}
544601
} catch (e) {
545-
console.error(
546-
"Error parsing parameter value with form_type multi-select",
547-
e,
548-
);
549602
return false;
550603
}
551604

0 commit comments

Comments
 (0)