Skip to content

Commit

Permalink
feat(ui): Open accordions which include form validation errors
Browse files Browse the repository at this point in the history
Earlier on, when the ORT run creation form contained validation errors
in some fields, the `Create` button just did nothing, giving no hints
to the user as to what failed and where. As a first stage of improving
this behaviour, whenever there is a validation failure in any jobs,
open the corresponding accordion components so that the user can see
the errors.

Signed-off-by: Jyrki Keisala <jyrki.keisala@doubleopen.org>
  • Loading branch information
Etsija committed Nov 13, 2024
1 parent 2482ada commit a170c12
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,15 @@ import { CreateRunFormValues } from '../-create-run-utils';

type AdvisorFieldsProps = {
form: UseFormReturn<CreateRunFormValues>;
value: string;
onToggle: () => void;
};

export const AdvisorFields = ({ form }: AdvisorFieldsProps) => {
export const AdvisorFields = ({
form,
value,
onToggle,
}: AdvisorFieldsProps) => {
return (
<div className='flex flex-row align-middle'>
<FormField
Expand All @@ -56,8 +62,8 @@ export const AdvisorFields = ({ form }: AdvisorFieldsProps) => {
</FormControl>
)}
/>
<AccordionItem value='advisor' className='flex-1'>
<AccordionTrigger>Advisor</AccordionTrigger>
<AccordionItem value={value} className='flex-1'>
<AccordionTrigger onClick={onToggle}>Advisor</AccordionTrigger>
<AccordionContent>
<FormField
control={form.control}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,15 @@ import { PackageManagerField } from './package-manager-field';

type AnalyzerFieldsProps = {
form: UseFormReturn<CreateRunFormValues>;
value: string;
onToggle: () => void;
};

export const AnalyzerFields = ({ form }: AnalyzerFieldsProps) => {
export const AnalyzerFields = ({
form,
value,
onToggle,
}: AnalyzerFieldsProps) => {
return (
<div className='flex flex-row align-middle'>
<FormField
Expand All @@ -58,8 +64,8 @@ export const AnalyzerFields = ({ form }: AnalyzerFieldsProps) => {
</FormControl>
)}
/>
<AccordionItem value='analyzer' className='flex-1'>
<AccordionTrigger>Analyzer</AccordionTrigger>
<AccordionItem value={value} className='flex-1'>
<AccordionTrigger onClick={onToggle}>Analyzer</AccordionTrigger>
<AccordionContent className='flex flex-col gap-6'>
<FormField
control={form.control}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,15 @@ import { CreateRunFormValues } from '../-create-run-utils';

type EvaluatorFieldsProps = {
form: UseFormReturn<CreateRunFormValues>;
value: string;
onToggle: () => void;
};

export const EvaluatorFields = ({ form }: EvaluatorFieldsProps) => {
export const EvaluatorFields = ({
form,
value,
onToggle,
}: EvaluatorFieldsProps) => {
return (
<div className='flex flex-row align-middle'>
<FormField
Expand All @@ -56,8 +62,8 @@ export const EvaluatorFields = ({ form }: EvaluatorFieldsProps) => {
</FormControl>
)}
/>
<AccordionItem value='evaluator' className='flex-1'>
<AccordionTrigger>Evaluator</AccordionTrigger>
<AccordionItem value={value} className='flex-1'>
<AccordionTrigger onClick={onToggle}>Evaluator</AccordionTrigger>
<AccordionContent>
<div className='text-sm text-gray-500'>
In case any input field is left empty, the default path from the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,15 @@ import { CreateRunFormValues } from '../-create-run-utils';

type NotifierFieldsProps = {
form: UseFormReturn<CreateRunFormValues>;
value: string;
onToggle: () => void;
};

export const NotifierFields = ({ form }: NotifierFieldsProps) => {
export const NotifierFields = ({
form,
value,
onToggle,
}: NotifierFieldsProps) => {
const { fields, append, remove } = useFieldArray({
name: 'jobConfigs.notifier.mail.recipientAddresses',
control: form.control,
Expand All @@ -64,8 +70,8 @@ export const NotifierFields = ({ form }: NotifierFieldsProps) => {
</FormControl>
)}
/>
<AccordionItem value='notifier' className='flex-1'>
<AccordionTrigger>Notifier</AccordionTrigger>
<AccordionItem value={value} className='flex-1'>
<AccordionTrigger onClick={onToggle}>Notifier</AccordionTrigger>
<AccordionContent>
<FormField
control={form.control}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,15 @@ import { CreateRunFormValues } from '../-create-run-utils';

type ReporterFieldsProps = {
form: UseFormReturn<CreateRunFormValues>;
value: string;
onToggle: () => void;
};

export const ReporterFields = ({ form }: ReporterFieldsProps) => {
export const ReporterFields = ({
form,
value,
onToggle,
}: ReporterFieldsProps) => {
return (
<div className='flex flex-row align-middle'>
<FormField
Expand All @@ -50,8 +56,8 @@ export const ReporterFields = ({ form }: ReporterFieldsProps) => {
</FormControl>
)}
/>
<AccordionItem value='reporter' className='flex-1'>
<AccordionTrigger>Reporter</AccordionTrigger>
<AccordionItem value={value} className='flex-1'>
<AccordionTrigger onClick={onToggle}>Reporter</AccordionTrigger>
<AccordionContent>
<MultiSelectField
form={form}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,15 @@ import { CreateRunFormValues } from '../-create-run-utils';

type ScannerFieldsProps = {
form: UseFormReturn<CreateRunFormValues>;
value: string;
onToggle: () => void;
};

export const ScannerFields = ({ form }: ScannerFieldsProps) => {
export const ScannerFields = ({
form,
value,
onToggle,
}: ScannerFieldsProps) => {
return (
<div className='flex flex-row align-middle'>
<FormField
Expand All @@ -54,8 +60,8 @@ export const ScannerFields = ({ form }: ScannerFieldsProps) => {
</FormControl>
)}
/>
<AccordionItem value='scanner' className='flex-1'>
<AccordionTrigger>Scanner</AccordionTrigger>
<AccordionItem value={value} className='flex-1'>
<AccordionTrigger onClick={onToggle}>Scanner</AccordionTrigger>
<AccordionContent>
<FormField
control={form.control}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import { zodResolver } from '@hookform/resolvers/zod';
import { createFileRoute, useNavigate } from '@tanstack/react-router';
import { Loader2, PlusIcon, TrashIcon } from 'lucide-react';
import { useState } from 'react';
import { useFieldArray, useForm } from 'react-hook-form';
import { z } from 'zod';

Expand Down Expand Up @@ -64,6 +65,26 @@ const CreateRunPage = () => {
const params = Route.useParams();
const ortRun = Route.useLoaderData();

type AccordionSection =
| 'analyzer'
| 'advisor'
| 'scanner'
| 'evaluator'
| 'reporter'
| 'notifier';

const [openAccordions, setOpenAccordions] = useState<AccordionSection[]>([]);

// Manually toggle accordion open/close state
const toggleAccordionOpen = (value: AccordionSection) => {
setOpenAccordions(
(prev) =>
prev.includes(value)
? prev.filter((v) => v !== value) // Close accordion if open
: [...prev, value] // Open accordion if closed
);
};

const { mutateAsync, isPending } = useRepositoriesServicePostOrtRun({
onSuccess() {
toast.info('Create ORT Run', {
Expand Down Expand Up @@ -120,13 +141,43 @@ const CreateRunPage = () => {
});
}

const onValidationFailed = (errors: typeof form.formState.errors) => {
// Determine which accordions contain errors
const accordionsWithErrors: AccordionSection[] = [];

if (errors.jobConfigs?.analyzer) {
accordionsWithErrors.push('analyzer');
}
if (errors.jobConfigs?.advisor) {
accordionsWithErrors.push('advisor');
}
if (errors.jobConfigs?.scanner) {
accordionsWithErrors.push('scanner');
}
if (errors.jobConfigs?.evaluator) {
accordionsWithErrors.push('evaluator');
}
if (errors.jobConfigs?.reporter) {
accordionsWithErrors.push('reporter');
}
if (errors.jobConfigs?.notifier) {
accordionsWithErrors.push('notifier');
}

// Open the accordions with errors
setOpenAccordions(accordionsWithErrors);
};

return (
<Card>
<CardHeader>
<CardTitle>Create an ORT run</CardTitle>
</CardHeader>
<Form {...form}>
<form onSubmit={form.handleSubmit(onSubmit)} className='space-y-8'>
<form
onSubmit={form.handleSubmit(onSubmit, onValidationFailed)}
className='space-y-8'
>
<CardContent>
<FormField
control={form.control}
Expand Down Expand Up @@ -314,13 +365,43 @@ const CreateRunPage = () => {
NOTE: Currently, the Analyzer needs to always run as part of an
ORT Run.
</div>
<Accordion type='multiple'>
<AnalyzerFields form={form} />
<AdvisorFields form={form} />
<ScannerFields form={form} />
<EvaluatorFields form={form} />
<ReporterFields form={form} />
<NotifierFields form={form} />
<Accordion
type='multiple'
value={openAccordions}
onValueChange={(value) =>
setOpenAccordions(value as AccordionSection[])
}
>
<AnalyzerFields
form={form}
value='analyzer'
onToggle={() => toggleAccordionOpen('analyzer')}
/>
<AdvisorFields
form={form}
value='advisor'
onToggle={() => toggleAccordionOpen('advisor')}
/>
<ScannerFields
form={form}
value='scanner'
onToggle={() => toggleAccordionOpen('scanner')}
/>
<EvaluatorFields
form={form}
value='evaluator'
onToggle={() => toggleAccordionOpen('evaluator')}
/>
<ReporterFields
form={form}
value='reporter'
onToggle={() => toggleAccordionOpen('reporter')}
/>
<NotifierFields
form={form}
value='notifier'
onToggle={() => toggleAccordionOpen('notifier')}
/>
</Accordion>
</CardContent>
<CardFooter>
Expand Down

0 comments on commit a170c12

Please sign in to comment.