Skip to content

Commit f95bad1

Browse files
manaswinidasmanaswinidas
authored
feat(mr): Fix model registry/catalog UX inconsistencies (kubeflow#1716)
* feat(mr): Fix model registry/catalog UX inconsistencies Signed-off-by: manaswinidas <[email protected]> * Sorting by last modified and addressing review comments Signed-off-by: manaswinidas <[email protected]> * Fix Cypress tests related to Last modified column Signed-off-by: manaswinidas <[email protected]> * Add pf-override component for proper spacing for formsection description Signed-off-by: manaswinidas <[email protected]> * Fix lint error Signed-off-by: manaswinidas <[email protected]> * Clean up Signed-off-by: manaswinidas <[email protected]> --------- Signed-off-by: manaswinidas <[email protected]>
1 parent 1393ec7 commit f95bad1

File tree

19 files changed

+174
-75
lines changed

19 files changed

+174
-75
lines changed

‎clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,12 +273,12 @@ describe('Model Registry core', () => {
273273
});
274274

275275
it('Sort by Last modified', () => {
276-
modelRegistry.findRegisteredModelTableHeaderButton('Last modified').click();
277-
modelRegistry.findRegisteredModelTableHeaderButton('Last modified').should(be.sortAscending);
278276
modelRegistry.findRegisteredModelTableHeaderButton('Last modified').click();
279277
modelRegistry.findRegisteredModelTableHeaderButton('Last modified').should(be.sortDescending);
280278
modelRegistry.findRegisteredModelTableHeaderButton('Last modified').click();
281279
modelRegistry.findRegisteredModelTableHeaderButton('Last modified').should(be.sortAscending);
280+
modelRegistry.findRegisteredModelTableHeaderButton('Last modified').click();
281+
modelRegistry.findRegisteredModelTableHeaderButton('Last modified').should(be.sortDescending);
282282
});
283283

284284
it('Filter by keyword then both', () => {

‎clients/ui/frontend/src/app/pages/modelCatalog/components/ModelCatalogLabels.tsx‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,8 @@ const ModelCatalogLabels: React.FC<ModelCatalogLabelsProps> = ({
2828
{label}
2929
</Label>
3030
))}
31-
{license && (
32-
<Label color="purple" isCompact>
33-
{license}
34-
</Label>
35-
)}
31+
{license && <Label color="purple">{license}</Label>}
32+
{provider && <Label>{provider}</Label>}
3633
</LabelGroup>
3734
);
3835

‎clients/ui/frontend/src/app/pages/modelCatalog/screens/ModelCatalogGalleryView.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ const ModelCatalogGalleryView: React.FC<ModelCatalogPageProps> = ({
108108
isModelValidated(model) ? mockPerformanceMetricsArtifacts : undefined
109109
}
110110
accuracyMetrics={isModelValidated(model) ? mockAccuracyMetricsArtifacts : undefined}
111+
truncate
111112
/>
112113
))}
113114
</Gallery>

‎clients/ui/frontend/src/app/pages/modelCatalog/screens/ModelDetailsPage.tsx‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,7 @@ const ModelDetailsPage: React.FC<ModelDetailsPageProps> = ({ tab }) => {
146146
</StackItem>
147147
</Stack>
148148
</Flex>
149-
) : (
150-
'Model details'
151-
)
149+
) : null
152150
}
153151
empty={!model}
154152
emptyStatePage={

‎clients/ui/frontend/src/app/pages/modelCatalog/screens/ModelDetailsView.tsx‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ const ModelDetailsView: React.FC<ModelDetailsViewProps> = ({
5353
return (
5454
<PageSection hasBodyWrapper={false} isFilled>
5555
<Sidebar hasBorder hasGutter isPanelRight>
56-
<SidebarContent>
57-
<Content>
56+
<SidebarContent style={{ minWidth: 0, overflow: 'hidden' }}>
57+
<Content style={{ wordBreak: 'break-word' }}>
5858
<h2>Description</h2>
5959
<p data-testid="model-long-description">{model.description || 'No description'}</p>
6060
<h2>Model card</h2>
@@ -68,8 +68,8 @@ const ModelDetailsView: React.FC<ModelDetailsViewProps> = ({
6868
/>
6969
)}
7070
</SidebarContent>
71-
<SidebarPanel>
72-
<DescriptionList isFillColumns>
71+
<SidebarPanel width={{ default: 'width_33' }}>
72+
<DescriptionList>
7373
<DescriptionListGroup>
7474
<DescriptionListTerm>Labels</DescriptionListTerm>
7575
<DescriptionListDescription>
@@ -116,10 +116,12 @@ const ModelDetailsView: React.FC<ModelDetailsViewProps> = ({
116116
) : !artifactLoaded ? (
117117
<Spinner size="sm" />
118118
) : artifacts.items.length > 0 && hasModelArtifacts(artifacts.items) ? (
119-
<InlineTruncatedClipboardCopy
120-
testId="source-image-location"
121-
textToCopy={getModelArtifactUri(artifacts.items) || ''}
122-
/>
119+
<DescriptionListDescription>
120+
<InlineTruncatedClipboardCopy
121+
testId="source-image-location"
122+
textToCopy={getModelArtifactUri(artifacts.items) || ''}
123+
/>
124+
</DescriptionListDescription>
123125
) : (
124126
<p className={text.textColorDisabled}>No artifacts available</p>
125127
)}

‎clients/ui/frontend/src/app/pages/modelRegistry/components/pf-overrides/FormSection.scss‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
.odh-form-section {
2+
&__desc {
3+
margin-top: var(--pf-t--global--spacer--sm);
4+
font-size: var(--pf-t--global--font--size--sm);
5+
color: var(--pf-t--global--text--color--subtle);
6+
font-weight: initial;
7+
}
8+
}

‎clients/ui/frontend/src/app/pages/modelRegistry/components/pf-overrides/FormSection.tsx‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import * as React from 'react';
2+
import { FormSection as PFFormSection, FormSectionProps, Content } from '@patternfly/react-core';
3+
4+
import './FormSection.scss';
5+
6+
type Props = FormSectionProps & {
7+
description?: React.ReactNode;
8+
};
9+
10+
// Remove once https://github.com/patternfly/patternfly/issues/6663 is fixed
11+
const FormSection: React.FC<Props> = ({
12+
description,
13+
title,
14+
titleElement: TitleElement = 'div',
15+
...props
16+
}) => (
17+
<PFFormSection
18+
{...props}
19+
titleElement={description ? 'div' : TitleElement}
20+
title={
21+
description ? (
22+
<>
23+
<TitleElement className="pf-v6-c-form__section-title">{title}</TitleElement>
24+
<Content component="p" className="odh-form-section__desc">
25+
{description}
26+
</Content>
27+
</>
28+
) : (
29+
title
30+
)
31+
}
32+
/>
33+
);
34+
35+
export default FormSection;

‎clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsView.tsx‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ import * as React from 'react';
22
import {
33
DescriptionList,
44
Divider,
5-
Flex,
6-
FlexItem,
75
ContentVariants,
86
Title,
97
Bullseye,
@@ -14,6 +12,9 @@ import {
1412
Card,
1513
CardHeader,
1614
CardBody,
15+
Sidebar,
16+
SidebarPanel,
17+
SidebarContent,
1718
} from '@patternfly/react-core';
1819
import {
1920
EditableLabelsDescriptionListGroup,
@@ -104,20 +105,16 @@ const ModelVersionDetailsView: React.FC<ModelVersionDetailsViewProps> = ({
104105
<Title headingLevel="h2">Version details</Title>
105106
</CardHeader>
106107
<CardBody>
107-
<Flex
108-
direction={{ default: 'column', md: 'row' }}
109-
columnGap={{ default: 'columnGap4xl' }}
110-
rowGap={{ default: 'rowGapLg' }}
111-
>
112-
<FlexItem flex={{ default: 'flex_1' }}>
113-
<DescriptionList isFillColumns>
108+
<Sidebar hasBorder hasGutter isPanelRight>
109+
<SidebarContent>
110+
<DescriptionList>
114111
<EditableLabelsDescriptionListGroup
115112
labels={getLabels(mv.customProperties)}
116113
isArchive={isArchiveVersion}
117114
allExistingKeys={Object.keys(mv.customProperties)}
118115
title="Labels"
119116
contentWhenEmpty="No labels"
120-
labelProps={{ variant: 'outline' }}
117+
labelProps={{ variant: 'outline', color: 'grey' }}
121118
onLabelsChange={(editedLabels) =>
122119
handleVersionUpdate(
123120
apiState.api.patchModelVersion(
@@ -154,9 +151,8 @@ const ModelVersionDetailsView: React.FC<ModelVersionDetailsViewProps> = ({
154151
}
155152
/>
156153
</DescriptionList>
157-
</FlexItem>
158-
<Divider orientation={{ default: 'vertical' }} />
159-
<FlexItem flex={{ default: 'flex_1' }}>
154+
</SidebarContent>
155+
<SidebarPanel width={{ default: 'width_33' }}>
160156
{modelArtifact && (
161157
<ModelVersionRegisteredFromLink
162158
modelArtifact={modelArtifact}
@@ -301,8 +297,8 @@ const ModelVersionDetailsView: React.FC<ModelVersionDetailsViewProps> = ({
301297
<ModelTimestamp timeSinceEpoch={mv.createTimeSinceEpoch} />
302298
</DashboardDescriptionListGroup>
303299
</DescriptionList>
304-
</FlexItem>
305-
</Flex>
300+
</SidebarPanel>
301+
</Sidebar>
306302
</CardBody>
307303
</Card>
308304
</StackItem>

‎clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionSelector.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ const ModelVersionSelector: React.FC<ModelVersionSelectorProps> = ({
9898
<MenuList data-testid="model-version-selector-list">
9999
{menuListItems}
100100
<MenuItem>
101-
<ViewAllVersionsButton rmId={rmId} totalVersions={modelVersions.items.length} />
101+
<ViewAllVersionsButton rmId={rmId} totalVersions={liveModelVersions.length} />
102102
</MenuItem>
103103
</MenuList>
104104
</MenuContent>

‎clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelDetailsCard.tsx‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import {
55
StackItem,
66
Stack,
77
CardBody,
8-
CardTitle,
98
ClipboardCopy,
109
Content,
1110
CardHeader,
@@ -14,6 +13,7 @@ import {
1413
SidebarPanel,
1514
SidebarContent,
1615
Alert,
16+
Title,
1717
} from '@patternfly/react-core';
1818
import {
1919
EditableTextDescriptionListGroup,
@@ -63,6 +63,7 @@ const ModelDetailsCard: React.FC<ModelDetailsCardProps> = ({
6363

6464
const labelsSection = (
6565
<EditableLabelsDescriptionListGroup
66+
key={`labels-${rm.lastUpdateTimeSinceEpoch}`}
6667
labels={getLabels(rm.customProperties)}
6768
isArchive={isArchiveModel}
6869
allExistingKeys={Object.keys(rm.customProperties)}
@@ -80,13 +81,14 @@ const ModelDetailsCard: React.FC<ModelDetailsCardProps> = ({
8081
.then(refresh)
8182
}
8283
isCollapsible={false}
83-
labelProps={{ variant: 'outline' }}
84+
labelProps={{ variant: 'outline', color: 'grey' }}
8485
onEditingChange={isExpandable ? handleLabelsEditingChange : undefined}
8586
/>
8687
);
8788

8889
const descriptionSection = (
8990
<EditableTextDescriptionListGroup
91+
key={`description-${rm.lastUpdateTimeSinceEpoch}`}
9092
truncateMaxLines={3}
9193
editableVariant="TextArea"
9294
baseTestId="model-description"
@@ -148,6 +150,7 @@ const ModelDetailsCard: React.FC<ModelDetailsCardProps> = ({
148150

149151
const propertiesSection = (
150152
<ModelPropertiesExpandableSection
153+
key={`properties-${rm.lastUpdateTimeSinceEpoch}`}
151154
modelName={rm.name}
152155
isArchive={isArchiveModel}
153156
customProperties={rm.customProperties}
@@ -157,6 +160,7 @@ const ModelDetailsCard: React.FC<ModelDetailsCardProps> = ({
157160
.then(refresh)
158161
}
159162
onEditingChange={isExpandable ? handlePropertiesEditingChange : undefined}
163+
showInlineAlerts
160164
/>
161165
);
162166

@@ -226,15 +230,17 @@ const ModelDetailsCard: React.FC<ModelDetailsCardProps> = ({
226230
'aria-expanded': isExpanded,
227231
}}
228232
>
229-
<CardTitle>Model details</CardTitle>
233+
<Title headingLevel="h2">Model details</Title>
230234
</CardHeader>
231235
<CardExpandableContent data-testid="model-details-card-expandable-content">
232236
{cardBody}
233237
</CardExpandableContent>
234238
</>
235239
) : (
236240
<>
237-
<CardTitle>Model details</CardTitle>
241+
<CardHeader>
242+
<Title headingLevel="h2">Model details</Title>
243+
</CardHeader>
238244
{cardBody}
239245
</>
240246
)}

0 commit comments

Comments
 (0)