Skip to content

Commit 65332e0

Browse files
fix a11y from model catalog details (#2627)
* fix a11y from model details Signed-off-by: Philip Colares Carneiro <philip.colares@gmail.com> * add changes requested Signed-off-by: Philip Colares Carneiro <philip.colares@gmail.com> --------- Signed-off-by: Philip Colares Carneiro <philip.colares@gmail.com>
1 parent 6e18d5a commit 65332e0

3 files changed

Lines changed: 123 additions & 6 deletions

File tree

clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelCatalog.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,10 @@ class ModelCatalog {
168168
return cy.findByTestId('model-long-description');
169169
}
170170

171+
findModelCardMarkdown() {
172+
return cy.findByTestId('model-card-markdown');
173+
}
174+
171175
findModelArchitecture() {
172176
return cy.findByTestId('model-architecture');
173177
}

clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelCatalog/modelCatalogDetails.cy.ts

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ import {
77
interceptArtifactsList,
88
interceptPerformanceArtifactsList,
99
} from '~/__tests__/cypress/cypress/support/interceptHelpers/modelCatalog';
10-
import { mockCatalogModelArtifact } from '~/__mocks__';
10+
import { mockCatalogModelArtifact, mockCatalogModel } from '~/__mocks__';
1111
import { ModelRegistryMetadataType } from '~/app/types';
12+
import { MODEL_CATALOG_API_VERSION } from '~/__tests__/cypress/cypress/support/commands/api';
1213

1314
describe('Model Catalog Details Page', () => {
1415
beforeEach(() => {
@@ -201,3 +202,113 @@ describe('Model Catalog Details Page - Filter State Management', () => {
201202
modelCatalog.findPerformanceFiltersUpdatedAlert().should('be.visible');
202203
});
203204
});
205+
206+
describe('Model Catalog Details Page - Edge Cases', () => {
207+
beforeEach(() => {
208+
cy.intercept('GET', '/model-registry/api/v1/model_registry*', [
209+
mockModelRegistry({ name: 'modelregistry-sample' }),
210+
]).as('getModelRegistries');
211+
});
212+
213+
it('should show "No description" when model has no description', () => {
214+
const modelWithoutDescription = mockCatalogModel({
215+
name: 'no-description-model',
216+
description: undefined,
217+
});
218+
219+
setupModelCatalogIntercepts({ customNonValidatedModel: modelWithoutDescription });
220+
modelCatalog.visit();
221+
modelCatalog.findLoadingState().should('not.exist');
222+
modelCatalog.findModelCatalogDetailLink().first().click();
223+
modelCatalog.findBreadcrumb().should('exist');
224+
225+
modelCatalog.findDetailsDescription().should('contain.text', 'No description');
226+
});
227+
228+
it('should show model card markdown when readme exists', () => {
229+
setupModelCatalogIntercepts({});
230+
modelCatalog.visit();
231+
modelCatalog.findLoadingState().should('not.exist');
232+
modelCatalog.findModelCatalogDetailLink().first().click();
233+
modelCatalog.findBreadcrumb().should('exist');
234+
235+
modelCatalog.findModelCardMarkdown().should('exist');
236+
});
237+
238+
it('should show "No model card" when model has no readme', () => {
239+
const modelWithoutReadme = mockCatalogModel({
240+
name: 'no-readme-model',
241+
readme: undefined,
242+
});
243+
244+
setupModelCatalogIntercepts({ customNonValidatedModel: modelWithoutReadme });
245+
modelCatalog.visit();
246+
modelCatalog.findLoadingState().should('not.exist');
247+
modelCatalog.findModelCatalogDetailLink().first().click();
248+
modelCatalog.findBreadcrumb().should('exist');
249+
250+
cy.contains('No model card').should('be.visible');
251+
});
252+
253+
it('should show "N/A" for provider when provider is not set', () => {
254+
const modelWithoutProvider = mockCatalogModel({
255+
name: 'no-provider-model',
256+
provider: undefined,
257+
});
258+
259+
setupModelCatalogIntercepts({ customNonValidatedModel: modelWithoutProvider });
260+
modelCatalog.visit();
261+
modelCatalog.findLoadingState().should('not.exist');
262+
modelCatalog.findModelCatalogDetailLink().first().click();
263+
modelCatalog.findBreadcrumb().should('exist');
264+
265+
cy.findAllByText('N/A').should('have.length.at.least', 1);
266+
});
267+
268+
it('should show error alert when artifacts fail to load', () => {
269+
setupModelCatalogIntercepts({});
270+
271+
cy.intercept(
272+
{
273+
method: 'GET',
274+
url: new RegExp(
275+
`/model-registry/api/${MODEL_CATALOG_API_VERSION}/model_catalog/sources/.*/artifacts/.*`,
276+
),
277+
},
278+
{ statusCode: 500, body: { message: 'Failed to load artifacts' } },
279+
).as('getArtifactsError');
280+
281+
modelCatalog.visit();
282+
modelCatalog.findLoadingState().should('not.exist');
283+
modelCatalog.findModelCatalogDetailLink().first().click();
284+
modelCatalog.findBreadcrumb().should('exist');
285+
286+
cy.wait('@getArtifactsError');
287+
cy.get('.pf-v6-c-alert.pf-m-danger').should('be.visible');
288+
});
289+
290+
it('should show spinner while artifacts are loading', () => {
291+
setupModelCatalogIntercepts({});
292+
293+
cy.intercept(
294+
{
295+
method: 'GET',
296+
url: new RegExp(
297+
`/model-registry/api/${MODEL_CATALOG_API_VERSION}/model_catalog/sources/.*/artifacts/.*`,
298+
),
299+
},
300+
(req) => {
301+
req.on('response', (res) => {
302+
res.setDelay(10000);
303+
});
304+
},
305+
).as('getArtifactsSlow');
306+
307+
modelCatalog.visit();
308+
modelCatalog.findLoadingState().should('not.exist');
309+
modelCatalog.findModelCatalogDetailLink().first().click();
310+
modelCatalog.findBreadcrumb().should('exist');
311+
312+
cy.findByRole('progressbar').should('exist');
313+
});
314+
});

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,11 +166,13 @@ const ModelDetailsView: React.FC<ModelDetailsViewProps> = ({
166166
</DescriptionListGroup>
167167
<DescriptionListGroup>
168168
<DescriptionListTerm>License</DescriptionListTerm>
169-
<ExternalLink
170-
text="Agreement"
171-
to={model.licenseLink || ''}
172-
testId="model-license-link"
173-
/>
169+
<DescriptionListDescription>
170+
<ExternalLink
171+
text="Agreement"
172+
to={model.licenseLink || ''}
173+
testId="model-license-link"
174+
/>
175+
</DescriptionListDescription>
174176
</DescriptionListGroup>
175177
<DescriptionListGroup>
176178
<DescriptionListTerm>Provider</DescriptionListTerm>

0 commit comments

Comments
 (0)