Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug fixes in model registry #3284

Merged
merged 4 commits into from
Oct 11, 2024
Merged

Bug fixes in model registry #3284

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6655087
UX/usability bug fixes - Model registry
manaswinidas Oct 1, 2024
4afea72
Remove Settings breadcrumb item from internal MR and SR admin setting…
manaswinidas Oct 1, 2024
93f57d0
Updated empty state icon in archived versions page and source model f…
manaswinidas Oct 7, 2024
8e81504
Add DescriptionList wherever required
manaswinidas Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion frontend/src/pages/modelRegistry/screens/ModelRegistrySelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ const ModelRegistrySelector: React.FC<ModelRegistrySelectorProps> = ({
};

const options = [
<SelectGroup label="Select a model registry" key="all">
<SelectGroup label="All model registries" key="all">
<SelectList>
{modelRegistryServices.map((mr) => (
<SelectOption
Expand Down
12 changes: 11 additions & 1 deletion frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,23 @@ const ModelVersionDetailsView: React.FC<ModelVersionDetailsViewProps> = ({
</DashboardDescriptionListGroup>
</>
)}
<Title style={{ marginTop: '1em' }} headingLevel={TextVariants.h3}>
Source model format
</Title>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manaswinidas this is a real nitpick, but can you follow the same pattern above with the "Model location" title where we close the </DescriptionList> before the Title and then open a new <DescriptionList isFillColumns>? And then can we add some additional margin between the two source model format fields and the Author field, since it's not part of Source model format? Maybe putting those last few fields in their own DescriptionList would also do that. Right now you can't tell where the "Source model format" section ends.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added DescriptionList with margin.

Introducing aDescriptionList between version and author without margin:
Screenshot 2024-10-11 at 1 29 04 PM

Introducing aDescriptionList between version and author with 2em margin(current):
Screenshot 2024-10-11 at 1 37 11 PM

WithoutDescriptionList between version and author:
Screenshot 2024-10-11 at 1 25 29 PM

<DashboardDescriptionListGroup
title="Source model format"
title="Name"
isEmpty={modelArtifact.size === 0 || !modelArtifact.items[0].modelFormatName}
contentWhenEmpty="No source model format"
>
{modelArtifact.items[0]?.modelFormatName}
</DashboardDescriptionListGroup>
<DashboardDescriptionListGroup
title="Version"
isEmpty={modelArtifact.size === 0 || !modelArtifact.items[0].modelFormatVersion}
contentWhenEmpty="No source model format version"
>
{modelArtifact.items[0]?.modelFormatVersion}
</DashboardDescriptionListGroup>
<DashboardDescriptionListGroup
title="Author"
tooltip={
Expand Down
3 changes: 2 additions & 1 deletion ...end/src/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchiveListView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
ToolbarItem,
ToolbarToggleGroup,
} from '@patternfly/react-core';
import { FilterIcon } from '@patternfly/react-icons';
import { FilterIcon, SearchIcon } from '@patternfly/react-icons';
import { SearchType } from '~/concepts/dashboard/DashboardSearchField';
import { ModelVersion } from '~/concepts/modelRegistry/types';
import SimpleSelect from '~/components/SimpleSelect';
Expand Down Expand Up @@ -35,6 +35,7 @@ const ModelVersionsArchiveListView: React.FC<ModelVersionsArchiveListViewProps>
if (unfilteredmodelVersions.length === 0) {
return (
<EmptyModelRegistryState
headerIcon={SearchIcon}
testid="empty-archive-state"
title="No archived versions"
description="You can archive the active versions that you no longer use. You can restore an archived versions to make it active."
Expand Down
1 change: 1 addition & 0 deletions frontend/src/pages/modelRegistry/screens/RegisterModel/RegisteredModelSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const RegisteredModelSelector: React.FC<RegisteredModelSelectorProps> = ({
return (
<TypeaheadSelect
id="model-name"
onClearSelection={() => setRegisteredModelId('')}
selectOptions={options}
placeholder="Select a registered model"
noOptionsFoundMessage={(filter) => `No results found for "${filter}"`}
Expand Down
3 changes: 2 additions & 1 deletion ...c/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchiveListView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
ToolbarItem,
ToolbarToggleGroup,
} from '@patternfly/react-core';
import { FilterIcon } from '@patternfly/react-icons';
import { FilterIcon, SearchIcon } from '@patternfly/react-icons';
import { SearchType } from '~/concepts/dashboard/DashboardSearchField';
import { RegisteredModel } from '~/concepts/modelRegistry/types';
import SimpleSelect from '~/components/SimpleSelect';
Expand Down Expand Up @@ -39,6 +39,7 @@ const RegisteredModelsArchiveListView: React.FC<RegisteredModelsArchiveListViewP
if (unfilteredRegisteredModels.length === 0) {
return (
<EmptyModelRegistryState
headerIcon={SearchIcon}
manaswinidas marked this conversation as resolved.
Show resolved Hide resolved
testid="empty-archive-model-state"
title="No archived models"
description="You can archive the active models that you no longer use. You can restore an archived
Expand Down
1 change: 0 additions & 1 deletion frontend/src/pages/modelRegistrySettings/ModelRegistriesPermissions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ const ModelRegistriesManagePermissions: React.FC = () => {
description="Manage access to this model registry for individual users and user groups, and for service accounts in a project."
breadcrumb={
<Breadcrumb>
<BreadcrumbItem>Settings</BreadcrumbItem>
<BreadcrumbItem
render={() => <Link to="/modelRegistrySettings">Model registry settings</Link>}
/>
Expand Down
1 change: 0 additions & 1 deletion frontend/src/pages/modelServing/customServingRuntimes/CustomServingRuntimeAddTemplate.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ const CustomServingRuntimeAddTemplate: React.FC<CustomServingRuntimeAddTemplateP
}
breadcrumb={
<Breadcrumb>
<BreadcrumbItem>Settings</BreadcrumbItem>
<BreadcrumbItem render={() => <Link to="/servingRuntimes">Serving runtimes</Link>} />
{existingTemplate && (
<BreadcrumbItem>
Expand Down