From ac10d580775b44d02e15979d50737f80751ebbfc Mon Sep 17 00:00:00 2001 From: Jeff Puzzo Date: Tue, 13 Aug 2024 09:36:50 -0400 Subject: [PATCH] [RHOAIENG-7572] Registered Models - Empty View Redesign --- .../cypress/cypress/pages/modelRegistry.ts | 6 +- .../mocked/modelRegistry/modelRegistry.cy.ts | 27 ++++++--- frontend/src/concepts/design/utils.ts | 4 +- .../images/empty-state-model-registries.svg | 9 +++ .../modelRegistry/ModelRegistryCoreLoader.tsx | 59 +++++++++++++------ .../modelRegistry/screens/ModelRegistry.tsx | 6 +- .../ModelVersionDetails.tsx | 2 +- .../screens/ModelVersions/ModelVersions.tsx | 2 +- .../ModelVersionArchiveDetailsBreadcrumb.tsx | 2 +- .../ModelVersionsArchive.tsx | 2 +- ...egisteredModelArchiveDetailsBreadcrumb.tsx | 2 +- .../RegisteredModelsArchive.tsx | 2 +- .../components/EmptyModelRegistryState.tsx | 31 ++++++---- 13 files changed, 106 insertions(+), 48 deletions(-) create mode 100644 frontend/src/images/empty-state-model-registries.svg diff --git a/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts b/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts index 88688563d9..2bc472f72c 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts @@ -66,7 +66,7 @@ class ModelRegistry { private wait() { cy.findByTestId('app-page-title').should('exist'); - cy.findByTestId('app-page-title').contains('Registered models'); + cy.findByTestId('app-page-title').contains('Model registry'); cy.testA11y(); } @@ -79,6 +79,10 @@ class ModelRegistry { return this; } + findModelRegistryEmptyState() { + return cy.findByTestId('empty-model-registries-state'); + } + shouldregisteredModelsEmpty() { cy.findByTestId('empty-registered-models').should('exist'); } diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts index 0dbfe06f81..6f39297340 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts @@ -12,17 +12,19 @@ import { } from '~/__tests__/cypress/cypress/utils/models'; import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; import { mockModelVersion } from '~/__mocks__/mockModelVersion'; -import type { ModelVersion, RegisteredModel } from '~/concepts/modelRegistry/types'; import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; import { mockModelRegistryService } from '~/__mocks__/mockModelRegistryService'; import { asProjectEditUser } from '~/__tests__/cypress/cypress/utils/mockUsers'; import { mockSelfSubjectRulesReview } from '~/__mocks__/mockSelfSubjectRulesReview'; import { mockSelfSubjectAccessReview } from '~/__mocks__/mockSelfSubjectAccessReview'; +import type { ModelVersion, RegisteredModel } from '~/concepts/modelRegistry/types'; +import type { ServiceKind } from '~/k8sTypes'; const MODEL_REGISTRY_API_VERSION = 'v1alpha3'; type HandlersProps = { disableModelRegistryFeature?: boolean; + modelRegistries?: ServiceKind[]; registeredModels?: RegisteredModel[]; modelVersions?: ModelVersion[]; allowed?: boolean; @@ -30,6 +32,10 @@ type HandlersProps = { const initIntercepts = ({ disableModelRegistryFeature = false, + modelRegistries = [ + mockModelRegistryService({ name: 'modelregistry-sample' }), + mockModelRegistryService({ name: 'modelregistry-sample-2' }), + ], registeredModels = [ mockRegisteredModel({ name: 'Fraud detection model', @@ -76,13 +82,7 @@ const initIntercepts = ({ cy.interceptK8s('POST', SelfSubjectRulesReviewModel, mockSelfSubjectRulesReview()); - cy.interceptK8sList( - ServiceModel, - mockK8sResourceList([ - mockModelRegistryService({ name: 'modelregistry-sample' }), - mockModelRegistryService({ name: 'modelregistry-sample-2' }), - ]), - ); + cy.interceptK8sList(ServiceModel, mockK8sResourceList(modelRegistries)); cy.interceptK8s(ServiceModel, mockModelRegistryService({ name: 'modelregistry-sample' })); @@ -141,6 +141,17 @@ describe('Model Registry core', () => { modelRegistry.tabEnabled(); }); + it('Renders empty state with no model registries', () => { + initIntercepts({ + disableModelRegistryFeature: false, + modelRegistries: [], + }); + + modelRegistry.visit(); + modelRegistry.navigate(); + modelRegistry.findModelRegistryEmptyState().should('exist'); + }); + it('No registered models in the selected Model Registry', () => { initIntercepts({ disableModelRegistryFeature: false, diff --git a/frontend/src/concepts/design/utils.ts b/frontend/src/concepts/design/utils.ts index 5fcf9ed8d5..60c4f68aed 100644 --- a/frontend/src/concepts/design/utils.ts +++ b/frontend/src/concepts/design/utils.ts @@ -15,6 +15,7 @@ import pipelineEmptyStateImg from '~/images/empty-state-pipelines.svg'; import clusterStorageEmptyStateImg from '~/images/empty-state-cluster-storage.svg'; import modelServerEmptyStateImg from '~/images/empty-state-model-serving.svg'; import dataConnectionEmptyStateImg from '~/images/empty-state-data-connections.svg'; +import modelRegistryEmptyStateImg from '~/images/empty-state-model-registries.svg'; import './vars.scss'; @@ -114,8 +115,9 @@ export const typedEmptyImage = (objectType: ProjectObjectType): string => { case ProjectObjectType.clusterStorage: return clusterStorageEmptyStateImg; case ProjectObjectType.modelServer: - case ProjectObjectType.registeredModels: return modelServerEmptyStateImg; + case ProjectObjectType.registeredModels: + return modelRegistryEmptyStateImg; case ProjectObjectType.dataConnection: return dataConnectionEmptyStateImg; default: diff --git a/frontend/src/images/empty-state-model-registries.svg b/frontend/src/images/empty-state-model-registries.svg new file mode 100644 index 0000000000..87b06da4eb --- /dev/null +++ b/frontend/src/images/empty-state-model-registries.svg @@ -0,0 +1,9 @@ + + + + + + + + + \ No newline at end of file diff --git a/frontend/src/pages/modelRegistry/ModelRegistryCoreLoader.tsx b/frontend/src/pages/modelRegistry/ModelRegistryCoreLoader.tsx index 4a8c3a2828..53a8acbae7 100644 --- a/frontend/src/pages/modelRegistry/ModelRegistryCoreLoader.tsx +++ b/frontend/src/pages/modelRegistry/ModelRegistryCoreLoader.tsx @@ -1,25 +1,29 @@ import * as React from 'react'; import { Navigate, Outlet, useParams } from 'react-router'; -import { Bullseye, Alert } from '@patternfly/react-core'; + +import { Bullseye, Alert, Popover, List, ListItem, Button } from '@patternfly/react-core'; +import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; + import { conditionalArea, SupportedArea } from '~/concepts/areas'; import { ModelRegistryContextProvider } from '~/concepts/modelRegistry/context/ModelRegistryContext'; import ApplicationsPage from '~/pages/ApplicationsPage'; import TitleWithIcon from '~/concepts/design/TitleWithIcon'; -import { ProjectObjectType } from '~/concepts/design/utils'; - +import { ProjectObjectType, typedEmptyImage } from '~/concepts/design/utils'; import { ModelRegistrySelectorContext } from '~/concepts/modelRegistry/context/ModelRegistrySelectorContext'; import InvalidModelRegistry from './screens/InvalidModelRegistry'; import EmptyModelRegistryState from './screens/components/EmptyModelRegistryState'; import ModelRegistrySelectorNavigator from './screens/ModelRegistrySelectorNavigator'; type ApplicationPageProps = React.ComponentProps; -type EmptyStateProps = 'emptyStatePage' | 'empty'; type ModelRegistryCoreLoaderProps = { getInvalidRedirectPath: (modelRegistry: string) => string; }; -type ApplicationPageRenderState = Pick; +type ApplicationPageRenderState = Pick< + ApplicationPageProps, + 'emptyStatePage' | 'empty' | 'headerContent' +>; const ModelRegistryCoreLoader: React.FC = conditionalArea( @@ -52,16 +56,37 @@ const ModelRegistryCoreLoader: React.FC = renderStateProps = { empty: true, emptyStatePage: ( - // TODO: Replace this with a component for empty registries once we have the designs { - // TODO: Add primary action - }} + testid="empty-model-registries-state" + title="Request access to model registries" + description="To request a new model registry, or to request permission to access an existing model registry, contact your administrator." + headerIcon={() => ( + + )} + customAction={ + + + The person who gave you your username, or who helped you to log in for the + first time + + Someone in your IT department or help desk + A project manager or developer + + } + > + + + } /> ), + headerContent: null, }; } else if (modelRegistry) { const foundModelRegistry = modelRegistryServices.find( @@ -90,18 +115,16 @@ const ModelRegistryCoreLoader: React.FC = return ( + } - {...renderStateProps} - loaded + description="View and manage all of your registered models. Registering models to model registry allows you to manage their content, metadata, versions, and user access settings." headerContent={ `/modelRegistry/${modelRegistryName}`} /> } + {...renderStateProps} + loaded provideChildrenPadding /> ); diff --git a/frontend/src/pages/modelRegistry/screens/ModelRegistry.tsx b/frontend/src/pages/modelRegistry/screens/ModelRegistry.tsx index 4bc6619153..016f834de5 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelRegistry.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelRegistry.tsx @@ -24,10 +24,8 @@ const ModelRegistry: React.FC = ({ ...pageProps }) => { return ( - } - description="View and manage your registered models." + title={} + description="View and manage all of your registered models. Registering models to model registry allows you to manage their content, metadata, versions, and user access settings." headerContent={ `/modelRegistry/${modelRegistryName}`} diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetails.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetails.tsx index a36eceff50..82b24e9239 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetails.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetails.tsx @@ -36,7 +36,7 @@ const ModelVersionsDetails: React.FC = ({ tab, ...page ( - Registered models - {preferredModelRegistry?.metadata.name} + Model registry - {preferredModelRegistry?.metadata.name} )} /> diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersions.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersions.tsx index 0bf008a6ec..d09ac348cb 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersions.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersions.tsx @@ -34,7 +34,7 @@ const ModelVersions: React.FC = ({ tab, ...pageProps }) => { ( - Registered models - {preferredModelRegistry?.metadata.name} + Model registry - {preferredModelRegistry?.metadata.name} )} /> diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionArchiveDetailsBreadcrumb.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionArchiveDetailsBreadcrumb.tsx index 3a70dd7b56..93569ff3fc 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionArchiveDetailsBreadcrumb.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionArchiveDetailsBreadcrumb.tsx @@ -20,7 +20,7 @@ const ModelVersionArchiveDetailsBreadcrumb: React.FC ( Registered models - {preferredModelRegistry}} + render={() => Model registry - {preferredModelRegistry}} /> ( diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchive.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchive.tsx index a61c63fcbf..05341f6a88 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchive.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchive.tsx @@ -30,7 +30,7 @@ const ModelVersionsArchive: React.FC = ({ ...pageProp ( - Registered models - {preferredModelRegistry?.metadata.name} + Model registry - {preferredModelRegistry?.metadata.name} )} /> diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelArchiveDetailsBreadcrumb.tsx b/frontend/src/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelArchiveDetailsBreadcrumb.tsx index fdc160a20c..cdece62842 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelArchiveDetailsBreadcrumb.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelArchiveDetailsBreadcrumb.tsx @@ -14,7 +14,7 @@ const RegisteredModelArchiveDetailsBreadcrumb: React.FC< > = ({ preferredModelRegistry, registeredModel }) => ( Registered models - {preferredModelRegistry}} + render={() => Model registry - {preferredModelRegistry}} /> ( diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchive.tsx b/frontend/src/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchive.tsx index c98930d376..ffabdc6c31 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchive.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchive.tsx @@ -24,7 +24,7 @@ const RegisteredModelsArchive: React.FC = ({ ...pa ( - Registered models - {preferredModelRegistry?.metadata.name} + Model registry - {preferredModelRegistry?.metadata.name} )} /> diff --git a/frontend/src/pages/modelRegistry/screens/components/EmptyModelRegistryState.tsx b/frontend/src/pages/modelRegistry/screens/components/EmptyModelRegistryState.tsx index 65d239019c..e223096ff5 100644 --- a/frontend/src/pages/modelRegistry/screens/components/EmptyModelRegistryState.tsx +++ b/frontend/src/pages/modelRegistry/screens/components/EmptyModelRegistryState.tsx @@ -1,3 +1,4 @@ +import React from 'react'; import { Button, ButtonVariant, @@ -7,10 +8,10 @@ import { EmptyStateFooter, EmptyStateHeader, EmptyStateIcon, + EmptyStateIconProps, EmptyStateVariant, } from '@patternfly/react-core'; import { PlusCircleIcon } from '@patternfly/react-icons'; -import * as React from 'react'; type EmptyModelRegistryStateType = { testid?: string; @@ -20,6 +21,8 @@ type EmptyModelRegistryStateType = { primaryActionOnClick?: () => void; secondaryActionText?: string; secondaryActionOnClick?: () => void; + headerIcon?: EmptyStateIconProps['icon']; + customAction?: React.ReactNode; }; const EmptyModelRegistryState: React.FC = ({ @@ -30,13 +33,18 @@ const EmptyModelRegistryState: React.FC = ({ secondaryActionText, primaryActionOnClick, secondaryActionOnClick, + headerIcon, + customAction, }) => ( - } /> + } + /> {description} - - {primaryActionText && ( + {primaryActionText && ( + - )} - - - {secondaryActionText && ( + + )} + + {secondaryActionText && ( + - )} - + + )} + + {customAction && {customAction}} );