Skip to content

Commit f52f5fc

Browse files
fix a11y from model details
Signed-off-by: Philip Colares Carneiro <philip.colares@gmail.com>
1 parent 6e18d5a commit f52f5fc

2 files changed

Lines changed: 187 additions & 5 deletions

File tree

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>
Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
import * as React from 'react';
2+
import { render, screen } from '@testing-library/react';
3+
import '@testing-library/jest-dom';
4+
import ModelDetailsView from '~/app/pages/modelCatalog/screens/ModelDetailsView';
5+
import { CatalogArtifactList, CatalogModel } from '~/app/modelCatalogTypes';
6+
7+
jest.mock('mod-arch-shared', () => ({
8+
InlineTruncatedClipboardCopy: ({
9+
textToCopy,
10+
testId,
11+
}: {
12+
textToCopy: string;
13+
testId: string;
14+
}) => <span data-testid={testId}>{textToCopy}</span>,
15+
relativeTime: () => '2 days ago',
16+
}));
17+
18+
jest.mock('~/app/shared/markdown/MarkdownComponent', () => ({
19+
__esModule: true,
20+
default: ({ data, dataTestId }: { data: string; dataTestId: string }) => (
21+
<div data-testid={dataTestId}>{data}</div>
22+
),
23+
}));
24+
25+
jest.mock('~/app/pages/modelCatalog/components/ModelCatalogLabels', () => ({
26+
__esModule: true,
27+
default: () => <span data-testid="model-catalog-labels">Labels</span>,
28+
}));
29+
30+
jest.mock('~/app/pages/modelRegistry/screens/components/ModelTimestamp', () => ({
31+
__esModule: true,
32+
default: ({ timeSinceEpoch }: { timeSinceEpoch?: string }) => (
33+
<span data-testid="model-timestamp">{timeSinceEpoch || '--'}</span>
34+
),
35+
}));
36+
37+
const mockModel: CatalogModel = {
38+
name: 'Test Model',
39+
description: 'A test model description',
40+
provider: 'Test Provider',
41+
readme: '# Test README',
42+
licenseLink: 'https://example.com/license',
43+
license: 'Apache-2.0',
44+
tasks: ['text-generation'],
45+
createTimeSinceEpoch: '1700000000000',
46+
lastUpdateTimeSinceEpoch: '1700100000000',
47+
customProperties: {},
48+
};
49+
50+
const mockArtifacts: CatalogArtifactList = {
51+
items: [],
52+
size: 0,
53+
pageSize: 0,
54+
nextPageToken: '',
55+
};
56+
57+
describe('ModelDetailsView', () => {
58+
it('renders model description', () => {
59+
render(
60+
<ModelDetailsView
61+
model={mockModel}
62+
artifacts={mockArtifacts}
63+
artifactLoaded
64+
artifactsLoadError={undefined}
65+
/>,
66+
);
67+
expect(screen.getByTestId('model-long-description')).toHaveTextContent(
68+
'A test model description',
69+
);
70+
});
71+
72+
it('renders license link inside a DescriptionListDescription (a11y compliance)', () => {
73+
const { container } = render(
74+
<ModelDetailsView
75+
model={mockModel}
76+
artifacts={mockArtifacts}
77+
artifactLoaded
78+
artifactsLoadError={undefined}
79+
/>,
80+
);
81+
82+
const licenseLink = screen.getByTestId('model-license-link');
83+
expect(licenseLink).toBeInTheDocument();
84+
85+
// Verify the ExternalLink is wrapped in a DescriptionListDescription (rendered as <dd>)
86+
const ddElement = licenseLink.closest('dd');
87+
expect(ddElement).not.toBeNull();
88+
89+
// Verify the dd is inside a DescriptionListGroup (rendered as a div within dl)
90+
const dlElement = container.querySelector('.pf-v6-c-description-list');
91+
expect(dlElement).toBeInTheDocument();
92+
});
93+
94+
it('renders provider information', () => {
95+
render(
96+
<ModelDetailsView
97+
model={mockModel}
98+
artifacts={mockArtifacts}
99+
artifactLoaded
100+
artifactsLoadError={undefined}
101+
/>,
102+
);
103+
expect(screen.getByText('Test Provider')).toBeInTheDocument();
104+
});
105+
106+
it('renders "No description" when description is empty', () => {
107+
render(
108+
<ModelDetailsView
109+
model={{ ...mockModel, description: undefined }}
110+
artifacts={mockArtifacts}
111+
artifactLoaded
112+
artifactsLoadError={undefined}
113+
/>,
114+
);
115+
expect(screen.getByTestId('model-long-description')).toHaveTextContent('No description');
116+
});
117+
118+
it('renders model card markdown when readme exists', () => {
119+
render(
120+
<ModelDetailsView
121+
model={mockModel}
122+
artifacts={mockArtifacts}
123+
artifactLoaded
124+
artifactsLoadError={undefined}
125+
/>,
126+
);
127+
expect(screen.getByTestId('model-card-markdown')).toBeInTheDocument();
128+
});
129+
130+
it('renders "No model card" when readme is absent', () => {
131+
render(
132+
<ModelDetailsView
133+
model={{ ...mockModel, readme: undefined }}
134+
artifacts={mockArtifacts}
135+
artifactLoaded
136+
artifactsLoadError={undefined}
137+
/>,
138+
);
139+
expect(screen.getByText('No model card')).toBeInTheDocument();
140+
});
141+
142+
it('renders N/A for provider when provider is not set', () => {
143+
render(
144+
<ModelDetailsView
145+
model={{ ...mockModel, provider: undefined }}
146+
artifacts={mockArtifacts}
147+
artifactLoaded
148+
artifactsLoadError={undefined}
149+
/>,
150+
);
151+
const providerValues = screen.getAllByText('N/A');
152+
expect(providerValues.length).toBeGreaterThan(0);
153+
});
154+
155+
it('renders artifact load error when present', () => {
156+
const error = new Error('Failed to load artifacts');
157+
error.name = 'LoadError';
158+
render(
159+
<ModelDetailsView
160+
model={mockModel}
161+
artifacts={mockArtifacts}
162+
artifactLoaded={false}
163+
artifactsLoadError={error}
164+
/>,
165+
);
166+
expect(screen.getByText('Failed to load artifacts')).toBeInTheDocument();
167+
});
168+
169+
it('renders spinner when artifacts are loading', () => {
170+
render(
171+
<ModelDetailsView
172+
model={mockModel}
173+
artifacts={mockArtifacts}
174+
artifactLoaded={false}
175+
artifactsLoadError={undefined}
176+
/>,
177+
);
178+
expect(screen.getByRole('progressbar')).toBeInTheDocument();
179+
});
180+
});

0 commit comments

Comments
 (0)