diff --git a/web/packages/studio/src/components/CreateFilesetStart/index.test.tsx b/web/packages/studio/src/components/CreateFilesetStart/index.test.tsx index a094771be9..0dd2be46df 100644 --- a/web/packages/studio/src/components/CreateFilesetStart/index.test.tsx +++ b/web/packages/studio/src/components/CreateFilesetStart/index.test.tsx @@ -5,25 +5,30 @@ import { CreateFilesetStart } from '@studio/components/CreateFilesetStart'; import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +const renderStart = () => { + const onContinue = vi.fn(); + render(); + return { onContinue }; +}; + describe('CreateFilesetStart', () => { - it('renders all four start options', () => { - render(); + it('renders all start options', () => { + renderStart(); expect(screen.getByText('Describe with AI')).toBeInTheDocument(); expect(screen.getByText('Start from a template')).toBeInTheDocument(); expect(screen.getByText('Build from scratch')).toBeInTheDocument(); }); - it('shows no Continue footer until a selectable option is chosen', () => { - render(); + it('shows no Continue footer until an option is chosen', () => { + renderStart(); expect(screen.queryByRole('button', { name: /continue/i })).not.toBeInTheDocument(); }); it('does not select disabled options (they are no-ops)', async () => { const user = userEvent.setup(); - const onContinue = vi.fn(); - render(); + const { onContinue } = renderStart(); await user.click(screen.getByText('Describe with AI')); @@ -33,56 +38,57 @@ describe('CreateFilesetStart', () => { it('selecting Build from scratch reveals Continue and invokes onContinue with "scratch"', async () => { const user = userEvent.setup(); - const onContinue = vi.fn(); - render(); + const { onContinue } = renderStart(); await user.click(screen.getByText('Build from scratch')); const continueButton = screen.getByRole('button', { name: /continue/i }); - expect(continueButton).toBeInTheDocument(); + expect(continueButton).toBeEnabled(); await user.click(continueButton); expect(onContinue).toHaveBeenCalledTimes(1); - expect(onContinue).toHaveBeenCalledWith('scratch'); + expect(onContinue).toHaveBeenCalledWith({ optionId: 'scratch' }); }); - it('reveals template cards but no Continue until a template is chosen', async () => { + it('reveals template cards but keeps Continue disabled until a template is chosen', async () => { const user = userEvent.setup(); - render(); + renderStart(); await user.click(screen.getByText('Start from a template')); expect(screen.getByText('Instruction fine-tuning (SFT)')).toBeInTheDocument(); - expect(screen.queryByRole('button', { name: /continue/i })).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: /continue/i })).toBeDisabled(); + expect(screen.getByText('Pick a recipe to continue.')).toBeInTheDocument(); }); - it('choosing a template reveals Continue and invokes onContinue with the template id', async () => { + it('choosing a template enables Continue and invokes onContinue with the template id', async () => { const user = userEvent.setup(); - const onContinue = vi.fn(); - render(); + const { onContinue } = renderStart(); await user.click(screen.getByText('Start from a template')); await user.click(screen.getByText('Instruction fine-tuning (SFT)')); - const continueButton = screen.getByRole('button', { name: /continue/i }); - await user.click(continueButton); + await user.click(screen.getByRole('button', { name: /continue/i })); expect(onContinue).toHaveBeenCalledTimes(1); - expect(onContinue).toHaveBeenCalledWith('template', 'sft-instruction'); + expect(onContinue).toHaveBeenCalledWith({ + optionId: 'template', + templateId: 'sft-instruction', + }); }); it('switching options clears a prior template selection', async () => { const user = userEvent.setup(); - render(); + renderStart(); await user.click(screen.getByText('Start from a template')); await user.click(screen.getByText('Instruction fine-tuning (SFT)')); - expect(screen.getByRole('button', { name: /continue/i })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /continue/i })).toBeEnabled(); await user.click(screen.getByText('Build from scratch')); await user.click(screen.getByText('Start from a template')); - // Template selection was reset when the option changed, so Continue is gone again. - expect(screen.queryByRole('button', { name: /continue/i })).not.toBeInTheDocument(); + // Template selection was reset when the option changed, so Continue is blocked again. + expect(screen.getByRole('button', { name: /continue/i })).toBeDisabled(); }); }); diff --git a/web/packages/studio/src/components/CreateFilesetStart/index.tsx b/web/packages/studio/src/components/CreateFilesetStart/index.tsx index bd37b7c99e..a98b4a692e 100644 --- a/web/packages/studio/src/components/CreateFilesetStart/index.tsx +++ b/web/packages/studio/src/components/CreateFilesetStart/index.tsx @@ -21,6 +21,11 @@ import type { import { ArrowRight } from 'lucide-react'; import { useState, type FC } from 'react'; +/** Why Continue is unavailable, shown next to the disabled button. */ +const BLOCKED_HINT: Partial> = { + template: 'Pick a recipe to continue.', +}; + export const CreateFilesetStart: FC = ({ onContinue }) => { const [selectedId, setSelectedId] = useState(null); const [selectedTemplateId, setSelectedTemplateId] = useState(null); @@ -38,9 +43,9 @@ export const CreateFilesetStart: FC = ({ onContinue }) const handleContinue = () => { if (!selectedOption) return; if (selectedOption.id === 'template' && selectedTemplateId) { - onContinue(selectedOption.id, selectedTemplateId); - } else { - onContinue(selectedOption.id); + onContinue({ optionId: 'template', templateId: selectedTemplateId }); + } else if (selectedOption.id === 'scratch') { + onContinue({ optionId: 'scratch' }); } }; @@ -80,13 +85,18 @@ export const CreateFilesetStart: FC = ({ onContinue }) - {canContinue ? ( + {selectedOption ? ( - diff --git a/web/packages/studio/src/components/CreateFilesetStart/types.ts b/web/packages/studio/src/components/CreateFilesetStart/types.ts index 5eca3139d2..4b94880f61 100644 --- a/web/packages/studio/src/components/CreateFilesetStart/types.ts +++ b/web/packages/studio/src/components/CreateFilesetStart/types.ts @@ -89,10 +89,10 @@ export interface StartOptionDetailProps { onSelectTemplate: (templateId: string) => void; } +/** What the user confirmed via the Continue footer, carrying that option's payload. */ +export type StartSelection = { optionId: 'scratch' } | { optionId: 'template'; templateId: string }; + export interface CreateFilesetStartProps { - /** - * Fired when the user confirms a selected start option via the Continue footer. For - * the "template" option, the chosen template id is passed as the second argument. - */ - onContinue: (optionId: StartOptionId, templateId?: string) => void; + /** Fired when the user confirms a selected start option via the Continue footer. */ + onContinue: (selection: StartSelection) => void; } diff --git a/web/packages/studio/src/routes/NewDataDesignerJobRoute/index.tsx b/web/packages/studio/src/routes/NewDataDesignerJobRoute/index.tsx index f18bcc9bc8..c4d8385686 100644 --- a/web/packages/studio/src/routes/NewDataDesignerJobRoute/index.tsx +++ b/web/packages/studio/src/routes/NewDataDesignerJobRoute/index.tsx @@ -3,7 +3,7 @@ import { AccessibleTitle } from '@studio/components/AccessibleTitle'; import { CreateFilesetStart } from '@studio/components/CreateFilesetStart'; -import type { StartOptionId } from '@studio/components/CreateFilesetStart/types'; +import type { StartSelection } from '@studio/components/CreateFilesetStart/types'; import { useWorkspaceFromPath } from '@studio/hooks/useWorkspaceFromPath'; import { useBreadcrumbs } from '@studio/providers/breadcrumbs/useBreadcrumbs'; import { getDataDesignerJobBuildRoute, getDataDesignerJobListRoute } from '@studio/routes/utils'; @@ -21,11 +21,14 @@ export const NewDataDesignerJobRoute: FC = () => { ], }); - const handleContinue = (optionId: StartOptionId, templateId?: string) => { - if (optionId === 'scratch') { - navigate(getDataDesignerJobBuildRoute(workspace)); - } else if (optionId === 'template' && templateId) { - navigate(`${getDataDesignerJobBuildRoute(workspace)}?template=${templateId}`); + const handleContinue = (selection: StartSelection) => { + switch (selection.optionId) { + case 'scratch': + navigate(getDataDesignerJobBuildRoute(workspace)); + break; + case 'template': + navigate(`${getDataDesignerJobBuildRoute(workspace)}?template=${selection.templateId}`); + break; } };