Skip to content

Commit 4a7a96f

Browse files
ashley-o0oashley-o0o
committed
Feedback revisions
1 parent 807be24 commit 4a7a96f

File tree

7 files changed

+159
-151
lines changed

7 files changed

+159
-151
lines changed

‎packages/cypress/cypress/pages/modelServing.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1334,6 +1334,10 @@ class ModelServingWizard extends Wizard {
13341334
findYAMLCodeEditor() {
13351335
return cy.findByTestId('yaml-editor');
13361336
}
1337+
1338+
findYAMLEditorEmptyState() {
1339+
return cy.findByTestId('yaml-editor-empty-state');
1340+
}
13371341
}
13381342

13391343
export const modelServingGlobal = new ModelServingGlobal();

‎packages/cypress/cypress/tests/mocked/modelServing/modelServingDeploy.cy.ts‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1705,33 +1705,12 @@ describe('Model Serving Deploy Wizard', () => {
17051705

17061706
modelServingGlobal.visit('test-project');
17071707
modelServingGlobal.findDeployModelButton().click();
1708-
modelServingWizard.findModelTypeSelectOption(ModelTypeLabel.GENERATIVE).should('exist').click();
1709-
modelServingWizard.findModelLocationSelect().should('exist');
1710-
modelServingWizard
1711-
.findModelLocationSelectOption(ModelLocationSelectOption.EXISTING)
1712-
.should('exist')
1713-
.click();
1714-
modelServingWizard.findExistingConnectionSelect().should('exist').click();
1715-
modelServingWizard
1716-
.findExistingConnectionSelectOption('Test URI Secret')
1717-
.should('exist')
1718-
.click();
1719-
modelServingWizard.findNextButton().should('be.enabled').click();
1720-
modelServingWizard.findModelDeploymentNameInput().type('test-model');
1721-
modelServingWizard.findServingRuntimeTemplateSearchSelector().click();
1722-
modelServingWizard
1723-
.findGlobalScopedTemplateOption('Distributed inference with llm-d')
1724-
.should('exist')
1725-
.click();
17261708

17271709
// YAML Viewer
17281710
modelServingWizard.findYAMLViewerToggle('YAML').should('exist').click();
1729-
modelServingWizard.findYAMLCodeEditor().should('exist');
1711+
modelServingWizard.findYAMLEditorEmptyState().should('exist');
17301712

17311713
modelServingWizard.findYAMLViewerToggle('Form').should('exist').click();
1732-
modelServingWizard.findServingRuntimeTemplateSearchSelector().click();
1733-
modelServingWizard.findGlobalScopedTemplateOption('vLLM NVIDIA').should('exist').click();
1734-
modelServingWizard.findYAMLViewerToggle('YAML').should('not.exist');
17351714
});
17361715

17371716
describe('redirect from v2 to v3 route', () => {

‎packages/model-serving/src/components/deploymentWizard/DeploymentWizardYAMLView.tsx‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import React from 'react';
2+
import { Language, CodeEditor } from '@patternfly/react-code-editor';
3+
import {
4+
Stack,
5+
StackItem,
6+
Flex,
7+
FlexItem,
8+
Button,
9+
Bullseye,
10+
EmptyState,
11+
EmptyStateBody,
12+
} from '@patternfly/react-core';
13+
14+
export const DeploymentWizardYAMLView: React.FC = () => {
15+
const [code, setCode] = React.useState('');
16+
17+
return (
18+
<Stack hasGutter>
19+
<StackItem>
20+
<Flex justifyContent={{ default: 'justifyContentFlexEnd' }}>
21+
<FlexItem>
22+
<Button variant="primary" data-testid="manual-edit-mode-button">
23+
Enter Manual Edit Mode
24+
</Button>
25+
</FlexItem>
26+
</Flex>
27+
</StackItem>
28+
<StackItem>
29+
<CodeEditor
30+
emptyState={
31+
<Bullseye>
32+
<EmptyState
33+
headingLevel="h4"
34+
titleText="No YAML available"
35+
data-testid="yaml-editor-empty-state"
36+
>
37+
<EmptyStateBody>Continue in the form, or select Manual Edit Mode.</EmptyStateBody>
38+
</EmptyState>
39+
</Bullseye>
40+
}
41+
data-testid="yaml-editor"
42+
code={code}
43+
onCodeChange={setCode}
44+
language={Language.plaintext}
45+
isLanguageLabelVisible
46+
/>
47+
</StackItem>
48+
</Stack>
49+
);
50+
};

‎packages/model-serving/src/components/deploymentWizard/ModelDeploymentStepContentToggle.tsx‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import React from 'react';
2+
3+
type StepContentToggleProps = {
4+
contentView: React.ReactNode;
5+
yamlView: React.ReactNode;
6+
viewMode: 'form' | 'yaml-preview' | 'yaml-edit';
7+
};
8+
9+
export const StepContentToggle: React.FC<StepContentToggleProps> = ({
10+
contentView,
11+
yamlView,
12+
viewMode,
13+
}) => {
14+
return <>{viewMode === 'yaml-preview' || viewMode === 'yaml-edit' ? yamlView : contentView}</>;
15+
};

‎packages/model-serving/src/components/deploymentWizard/ModelDeploymentWizard.tsx‎

Lines changed: 89 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import React from 'react';
2-
import { Spinner, Wizard, WizardStep } from '@patternfly/react-core';
2+
import { Spinner, ToggleGroup, ToggleGroupItem, Wizard, WizardStep } from '@patternfly/react-core';
33
import ApplicationsPage from '@odh-dashboard/internal/pages/ApplicationsPage';
44
import { getServingRuntimeFromTemplate } from '@odh-dashboard/internal/pages/modelServing/customServingRuntimes/utils';
55
import { ProjectKind } from '@odh-dashboard/internal/k8sTypes';
66
import {
77
getGeneratedSecretName,
88
isGeneratedSecretName,
99
} from '@odh-dashboard/internal/api/k8s/secrets';
10+
import { SupportedArea, useIsAreaAvailable } from '@odh-dashboard/internal/concepts/areas';
1011
import { Deployment } from 'extension-points';
1112
import { deployModel } from './utils';
1213
import { ExternalDataLoader, type ExternalDataMap } from './ExternalDataLoader';
@@ -22,7 +23,8 @@ import { InitialWizardFormData, WizardStepTitle } from './types';
2223
import { ExitDeploymentModal } from './exitModal/ExitDeploymentModal';
2324
import { useRefreshWizardPage } from './useRefreshWizardPage';
2425
import { useExitDeploymentWizard } from './exitModal/useExitDeploymentWizard';
25-
import { DeploymentWizardYAMLView } from './useDeploymentWizardYAMLView';
26+
import { DeploymentWizardYAMLView } from './DeploymentWizardYAMLView';
27+
import { StepContentToggle } from './ModelDeploymentStepContentToggle';
2628
import { WizardFooterWithDisablingNext } from '../generic/WizardFooterWithDisablingNext';
2729

2830
type ModelDeploymentWizardProps = {
@@ -34,9 +36,6 @@ type ModelDeploymentWizardProps = {
3436
existingDeployment?: Deployment;
3537
returnRoute?: string;
3638
cancelReturnRoute?: string;
37-
headerAction?: React.ReactNode;
38-
viewMode?: 'form' | 'yaml-preview' | 'yaml-edit';
39-
onModelServerChange?: (modelServerName: string | undefined) => void;
4039
};
4140

4241
const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
@@ -48,10 +47,9 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
4847
existingDeployment,
4948
returnRoute,
5049
cancelReturnRoute,
51-
headerAction,
52-
viewMode = 'form',
53-
onModelServerChange,
5450
}) => {
51+
const [viewMode, setViewMode] = React.useState<'form' | 'yaml-preview' | 'yaml-edit'>('form');
52+
const isYAMLViewerEnabled = useIsAreaAvailable(SupportedArea.YAML_VIEWER).status;
5553
const onRefresh = useRefreshWizardPage(existingDeployment);
5654
const { isExitModalOpen, openExitModal, closeExitModal, handleExitConfirm, exitWizardOnSubmit } =
5755
useExitDeploymentWizard({ returnRoute, cancelReturnRoute });
@@ -63,12 +61,6 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
6361
const validation = useModelDeploymentWizardValidation(wizardState.state, wizardState.fields);
6462
const currentProjectName = wizardState.state.project.projectName ?? undefined;
6563

66-
React.useEffect(() => {
67-
if (onModelServerChange) {
68-
onModelServerChange(wizardState.state.modelServer.data?.name);
69-
}
70-
}, [wizardState.state.modelServer.data?.name, onModelServerChange]);
71-
7264
const { deployMethod, deployMethodLoaded } = useDeployMethod(wizardState.state);
7365
// TODO in same jira, replace deployMethod with applyFieldData for all other fields
7466
const { applyFieldData, applyExtensionsLoaded } = useWizardFieldApply(wizardState.state);
@@ -212,7 +204,26 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
212204
description={description}
213205
loaded
214206
empty={false}
215-
headerAction={headerAction}
207+
headerAction={
208+
isYAMLViewerEnabled ? (
209+
<ToggleGroup aria-label="Deployment view mode">
210+
<ToggleGroupItem
211+
data-testid="form-view"
212+
text="Form"
213+
buttonId="form-view"
214+
isSelected={viewMode === 'form'}
215+
onChange={() => setViewMode('form')}
216+
/>
217+
<ToggleGroupItem
218+
data-testid="yaml-view"
219+
text="YAML"
220+
buttonId="yaml-view"
221+
isSelected={viewMode === 'yaml-preview'}
222+
onChange={() => setViewMode('yaml-preview')}
223+
/>
224+
</ToggleGroup>
225+
) : undefined
226+
}
216227
>
217228
<ExternalDataLoader
218229
fields={wizardState.fields}
@@ -229,74 +240,88 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
229240
startIndex={wizardState.initialData?.wizardStartIndex ?? 1}
230241
>
231242
<WizardStep name={WizardStepTitle.MODEL_DETAILS} id="source-model-step">
232-
{viewMode === 'yaml-preview' ? (
233-
<DeploymentWizardYAMLView />
234-
) : wizardState.loaded.modelSourceLoaded ? (
235-
<ModelSourceStepContent
236-
wizardState={wizardState}
237-
validation={validation.modelSource}
238-
/>
239-
) : (
240-
<Spinner data-testid="spinner" />
241-
)}
243+
<StepContentToggle
244+
viewMode={viewMode}
245+
yamlView={<DeploymentWizardYAMLView />}
246+
contentView={
247+
wizardState.loaded.modelSourceLoaded ? (
248+
<ModelSourceStepContent
249+
wizardState={wizardState}
250+
validation={validation.modelSource}
251+
/>
252+
) : (
253+
<Spinner data-testid="spinner" />
254+
)
255+
}
256+
/>
242257
</WizardStep>
243258
<WizardStep
244259
name={WizardStepTitle.MODEL_DEPLOYMENT}
245260
id="model-deployment-step"
246-
isDisabled={viewMode === 'form' && !validation.isModelSourceStepValid}
261+
isDisabled={!validation.isModelSourceStepValid}
247262
>
248-
{viewMode === 'yaml-preview' ? (
249-
<DeploymentWizardYAMLView />
250-
) : wizardState.loaded.modelDeploymentLoaded ? (
251-
<ModelDeploymentStepContent
252-
projectName={currentProjectName}
253-
wizardState={wizardState}
254-
/>
255-
) : (
256-
<Spinner data-testid="spinner" />
257-
)}
263+
<StepContentToggle
264+
viewMode={viewMode}
265+
yamlView={<DeploymentWizardYAMLView />}
266+
contentView={
267+
wizardState.loaded.modelDeploymentLoaded ? (
268+
<ModelDeploymentStepContent
269+
projectName={currentProjectName}
270+
wizardState={wizardState}
271+
/>
272+
) : (
273+
<Spinner data-testid="spinner" />
274+
)
275+
}
276+
/>
258277
</WizardStep>
259278
<WizardStep
260279
name={WizardStepTitle.ADVANCED_SETTINGS}
261280
id="advanced-options-step"
262281
isDisabled={
263-
viewMode === 'form' &&
264-
(!validation.isModelSourceStepValid || !validation.isModelDeploymentStepValid)
282+
!validation.isModelSourceStepValid || !validation.isModelDeploymentStepValid
265283
}
266284
>
267-
{viewMode === 'yaml-preview' ? (
268-
<DeploymentWizardYAMLView />
269-
) : wizardState.loaded.advancedOptionsLoaded ? (
270-
<AdvancedSettingsStepContent
271-
wizardState={wizardState}
272-
projectName={currentProjectName}
273-
externalData={externalData}
274-
/>
275-
) : (
276-
<Spinner data-testid="spinner" />
277-
)}
285+
<StepContentToggle
286+
viewMode={viewMode}
287+
yamlView={<DeploymentWizardYAMLView />}
288+
contentView={
289+
wizardState.loaded.advancedOptionsLoaded ? (
290+
<AdvancedSettingsStepContent
291+
wizardState={wizardState}
292+
projectName={currentProjectName}
293+
externalData={externalData}
294+
/>
295+
) : (
296+
<Spinner data-testid="spinner" />
297+
)
298+
}
299+
/>
278300
</WizardStep>
279301
<WizardStep
280302
name={WizardStepTitle.REVIEW}
281303
id="summary-step"
282304
isDisabled={
283-
viewMode === 'form' &&
284-
(!validation.isModelSourceStepValid ||
285-
!validation.isModelDeploymentStepValid ||
286-
!validation.isAdvancedSettingsStepValid)
305+
!validation.isModelSourceStepValid ||
306+
!validation.isModelDeploymentStepValid ||
307+
!validation.isAdvancedSettingsStepValid
287308
}
288309
>
289-
{viewMode === 'yaml-preview' ? (
290-
<DeploymentWizardYAMLView />
291-
) : wizardState.loaded.summaryLoaded ? (
292-
<ReviewStepContent
293-
wizardState={wizardState}
294-
projectName={currentProjectName}
295-
externalData={externalData}
296-
/>
297-
) : (
298-
<Spinner data-testid="spinner" />
299-
)}
310+
<StepContentToggle
311+
viewMode={viewMode}
312+
yamlView={<DeploymentWizardYAMLView />}
313+
contentView={
314+
wizardState.loaded.summaryLoaded ? (
315+
<ReviewStepContent
316+
wizardState={wizardState}
317+
projectName={currentProjectName}
318+
externalData={externalData}
319+
/>
320+
) : (
321+
<Spinner data-testid="spinner" />
322+
)
323+
}
324+
/>
300325
</WizardStep>
301326
</Wizard>
302327
</ApplicationsPage>

‎packages/model-serving/src/components/deploymentWizard/ModelDeploymentWizardPage.tsx‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,8 @@ import {
99
EmptyStateBody,
1010
EmptyState,
1111
Spinner,
12-
ToggleGroup,
13-
ToggleGroupItem,
1412
} from '@patternfly/react-core';
1513
import { ExclamationCircleIcon } from '@patternfly/react-icons';
16-
import { SupportedArea, useIsAreaAvailable } from '@odh-dashboard/internal/concepts/areas';
1714
import ModelDeploymentWizard from './ModelDeploymentWizard';
1815
import { ModelDeploymentsProvider } from '../../concepts/ModelDeploymentsContext';
1916
import { useAvailableClusterPlatforms } from '../../concepts/useAvailableClusterPlatforms';
@@ -41,14 +38,8 @@ const ErrorContent: React.FC<{ error: Error }> = ({ error }) => {
4138
);
4239
};
4340

44-
const LLMD_SERVING_ID = 'llmd-serving';
45-
4641
export const ModelDeploymentWizardPage: React.FC = () => {
4742
const location = useLocation();
48-
const [viewMode, setViewMode] = React.useState<'form' | 'yaml-preview' | 'yaml-edit'>('form');
49-
const [selectedModelServer, setSelectedModelServer] = React.useState<string | undefined>();
50-
const isYAMLViewerEnabled = useIsAreaAvailable(SupportedArea.YAML_VIEWER).status;
51-
const isLLMdSelected = selectedModelServer === LLMD_SERVING_ID;
5243

5344
// Extract state from navigation
5445
const existingData = location.state?.initialData;
@@ -108,28 +99,6 @@ export const ModelDeploymentWizardPage: React.FC = () => {
10899
existingDeployment={existingDeployment}
109100
returnRoute={returnRoute}
110101
cancelReturnRoute={cancelReturnRoute}
111-
viewMode={viewMode}
112-
onModelServerChange={setSelectedModelServer}
113-
headerAction={
114-
isYAMLViewerEnabled && isLLMdSelected ? (
115-
<ToggleGroup aria-label="Deployment view mode">
116-
<ToggleGroupItem
117-
data-testid="form-view"
118-
text="Form"
119-
buttonId="form-view"
120-
isSelected={viewMode === 'form'}
121-
onChange={() => setViewMode('form')}
122-
/>
123-
<ToggleGroupItem
124-
data-testid="yaml-view"
125-
text="YAML"
126-
buttonId="yaml-view"
127-
isSelected={viewMode === 'yaml-preview'}
128-
onChange={() => setViewMode('yaml-preview')}
129-
/>
130-
</ToggleGroup>
131-
) : undefined
132-
}
133102
/>
134103
</ModelDeploymentsProvider>
135104
);

0 commit comments

Comments
 (0)