Skip to content

Commit 25fcc52

Browse files
lokanandaprabhulokanandaprabhu
authored
feat: improve title consistency and pagination options (#1364)
1 parent cdf9bc0 commit 25fcc52

16 files changed

Lines changed: 250 additions & 33 deletions

File tree

‎workspaces/adoption-insights/.changeset/fix-dynamic-top-x-pagination-options.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@red-hat-developer-hub/backstage-plugin-adoption-insights': patch
3+
---
4+
5+
Pagination and title consistency improvements:
6+
7+
- Fixed dynamic "Top X" pagination options to adapt to actual item count, resolving misleading filter options when dataset is smaller than defaults
8+
- Added "All" option for datasets smaller than maxDefaultOption (20 items)
9+
- Improved component title consistency across the UI

‎workspaces/adoption-insights/packages/app/e2e-tests/insights.test.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,10 @@ test('Total number of users panel shows 1 visitor of 100', async () => {
9191
});
9292

9393
test('Top plugins shows catalog', async () => {
94-
const panel = page.locator('.v5-MuiPaper-root', { hasText: 'Top 3 plugins' });
94+
await navigate('Adoption Insights');
95+
const panel = page.locator('.v5-MuiPaper-root', { hasText: 'All plugins' });
9596
await expect(panel).toMatchAriaSnapshot(`
96-
- heading "Top 3 plugins" [level=5]
97+
- heading "All plugins" [level=5]
9798
- table:
9899
- rowgroup:
99100
- row :
@@ -165,7 +166,7 @@ test.describe(() => {
165166

166167
test('Visited component shows up in top catalog entities', async () => {
167168
const panel = page.locator('.v5-MuiPaper-root', {
168-
hasText: 'Top catalog entities',
169+
hasText: 'All catalog entities',
169170
});
170171
const entries = panel.locator('tbody').locator('tr');
171172
await expect(entries).toHaveCount(1);
@@ -174,7 +175,7 @@ test.describe(() => {
174175

175176
test('Visited TechDoc shows up in top TechDocs', async () => {
176177
const panel = page.locator('.v5-MuiPaper-root', {
177-
hasText: 'Top 3 TechDocs',
178+
hasText: 'All TechDocs',
178179
});
179180
const entries = panel.locator('tbody').locator('tr');
180181
await expect(entries).toHaveCount(1);
@@ -191,7 +192,7 @@ test.describe(() => {
191192

192193
test('New data shows in top templates', async () => {
193194
const panel = page.locator('.v5-MuiPaper-root', {
194-
hasText: 'Top 3 templates',
195+
hasText: 'All templates',
195196
});
196197
await panel.scrollIntoViewIfNeeded();
197198
const entries = panel.locator('tbody').locator('tr');

‎workspaces/adoption-insights/plugins/adoption-insights/report.api.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export const adoptionInsightsTranslationRef: TranslationRef<
5151
readonly 'filter.selectKind': string;
5252
readonly 'plugins.title': string;
5353
readonly 'plugins.topNTitle': string;
54+
readonly 'plugins.allTitle': string;
5455
readonly 'page.title': string;
5556
readonly 'activeUsers.legend.newUsers': string;
5657
readonly 'activeUsers.legend.returningUsers': string;
@@ -62,9 +63,13 @@ export const adoptionInsightsTranslationRef: TranslationRef<
6263
readonly 'activeUsers.averageSuffix': string;
6364
readonly 'templates.title': string;
6465
readonly 'templates.topNTitle': string;
66+
readonly 'templates.allTitle': string;
6567
readonly 'catalogEntities.title': string;
68+
readonly 'catalogEntities.topNTitle': string;
69+
readonly 'catalogEntities.allTitle': string;
6670
readonly 'techDocs.title': string;
6771
readonly 'techDocs.topNTitle': string;
72+
readonly 'techDocs.allTitle': string;
6873
readonly 'searches.title': string;
6974
readonly 'searches.hour': string;
7075
readonly 'searches.day': string;

‎workspaces/adoption-insights/plugins/adoption-insights/src/components/CardFooter/TableFooterPagination.tsx‎

Lines changed: 44 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,44 @@ interface TableFooterPaginationProps {
3333
) => void;
3434
}
3535

36+
const generateRowsPerPageOptions = (totalCount: number, t: any) => {
37+
const defaultOptions = [3, 5, 10, 20];
38+
const maxDefaultOption = Math.max(...defaultOptions);
39+
40+
if (defaultOptions.includes(totalCount)) {
41+
const validOptions = defaultOptions.filter(option => option <= totalCount);
42+
return validOptions.map(value => ({
43+
label: t('table.pagination.topN' as any, { count: value.toString() }),
44+
value,
45+
}));
46+
}
47+
48+
const validDefaults = defaultOptions.filter(option => option < totalCount);
49+
50+
if (validDefaults.length > 0 && totalCount <= maxDefaultOption) {
51+
const options = validDefaults.map(value => ({
52+
label: t('table.pagination.topN' as any, { count: value.toString() }),
53+
value,
54+
}));
55+
56+
options.push({
57+
label: t('filter.all' as any, {}),
58+
value: totalCount,
59+
});
60+
61+
return options;
62+
}
63+
64+
if (validDefaults.length > 0) {
65+
return validDefaults.map(value => ({
66+
label: t('table.pagination.topN' as any, { count: value.toString() }),
67+
value,
68+
}));
69+
}
70+
71+
return [];
72+
};
73+
3674
const TableFooterPagination: FC<TableFooterPaginationProps> = ({
3775
count,
3876
rowsPerPage,
@@ -41,6 +79,11 @@ const TableFooterPagination: FC<TableFooterPaginationProps> = ({
4179
handleChangeRowsPerPage,
4280
}) => {
4381
const { t } = useTranslation();
82+
const rowsPerPageOptions = generateRowsPerPageOptions(count, t);
83+
84+
if (rowsPerPageOptions.length <= 1) {
85+
return null;
86+
}
4487

4588
return (
4689
<Box
@@ -57,24 +100,7 @@ const TableFooterPagination: FC<TableFooterPaginationProps> = ({
57100
backgroundColor: 'transparent',
58101
},
59102
}}
60-
rowsPerPageOptions={[
61-
{
62-
label: t('table.pagination.topN' as any, { count: '3' }),
63-
value: 3,
64-
},
65-
{
66-
label: t('table.pagination.topN' as any, { count: '5' }),
67-
value: 5,
68-
},
69-
{
70-
label: t('table.pagination.topN' as any, { count: '10' }),
71-
value: 10,
72-
},
73-
{
74-
label: t('table.pagination.topN' as any, { count: '20' }),
75-
value: 20,
76-
},
77-
]}
103+
rowsPerPageOptions={rowsPerPageOptions}
78104
component="div"
79105
count={count}
80106
rowsPerPage={rowsPerPage}

‎workspaces/adoption-insights/plugins/adoption-insights/src/components/CardFooter/__tests__/TableFooterPagination.test.tsx‎

Lines changed: 133 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import { ThemeProvider, createTheme } from '@mui/material/styles';
3636
describe('TableFooterPagination', () => {
3737
const defaultProps = {
3838
count: 100,
39-
rowsPerPage: 10,
39+
rowsPerPage: 5,
4040
page: 0,
4141
handleChangePage: jest.fn(),
4242
handleChangeRowsPerPage: jest.fn(),
@@ -66,7 +66,8 @@ describe('TableFooterPagination', () => {
6666
const select = screen.getByRole('combobox');
6767

6868
await userEvent.click(select);
69-
await userEvent.click(screen.getByText('Top 5'));
69+
const options = screen.getAllByRole('option');
70+
await userEvent.click(options[0]);
7071

7172
expect(defaultProps.handleChangeRowsPerPage).toHaveBeenCalled();
7273
});
@@ -106,4 +107,134 @@ describe('TableFooterPagination', () => {
106107
borderRadius: '8px',
107108
});
108109
});
110+
111+
describe('Dynamic "Top X" options logic', () => {
112+
it('should not render when count is 1 (single option)', () => {
113+
const { container } = renderComponent({ ...defaultProps, count: 1 });
114+
expect(container.firstChild).toBeNull();
115+
});
116+
117+
it('should not render when count is 2 (single option)', () => {
118+
const { container } = renderComponent({ ...defaultProps, count: 2 });
119+
expect(container.firstChild).toBeNull();
120+
});
121+
122+
it('should show only "Top 3" option for 3 items (exact match, single option)', () => {
123+
const { container } = renderComponent({ ...defaultProps, count: 3 });
124+
expect(container.firstChild).toBeNull(); // Still hidden because only one option
125+
});
126+
127+
it('should show "Top 3, All" options for 4 items', async () => {
128+
renderComponent({ ...defaultProps, count: 4, rowsPerPage: 3 });
129+
const select = screen.getByRole('combobox');
130+
131+
await userEvent.click(select);
132+
133+
const options = screen.getAllByRole('option');
134+
expect(options).toHaveLength(2);
135+
expect(options[0]).toHaveTextContent('Top 3');
136+
expect(options[1]).toHaveTextContent('All');
137+
expect(screen.queryByText('Top 4')).not.toBeInTheDocument();
138+
expect(screen.queryByText('Top 5')).not.toBeInTheDocument();
139+
});
140+
141+
it('should show "Top 3, Top 5" options for 5 items (exact match)', async () => {
142+
renderComponent({ ...defaultProps, count: 5, rowsPerPage: 3 });
143+
const select = screen.getByRole('combobox');
144+
145+
await userEvent.click(select);
146+
147+
const options = screen.getAllByRole('option');
148+
expect(options).toHaveLength(2);
149+
expect(options[0]).toHaveTextContent('Top 3');
150+
expect(options[1]).toHaveTextContent('Top 5');
151+
expect(screen.queryByText('Top 4')).not.toBeInTheDocument();
152+
});
153+
154+
it('should show "Top 3, Top 5, All" options for 6 items', async () => {
155+
renderComponent({ ...defaultProps, count: 6, rowsPerPage: 3 });
156+
const select = screen.getByRole('combobox');
157+
158+
await userEvent.click(select);
159+
160+
const options = screen.getAllByRole('option');
161+
expect(options).toHaveLength(3);
162+
expect(options[0]).toHaveTextContent('Top 3');
163+
expect(options[1]).toHaveTextContent('Top 5');
164+
expect(options[2]).toHaveTextContent('All');
165+
expect(screen.queryByText('Top 6')).not.toBeInTheDocument();
166+
});
167+
168+
it('should show "Top 3, Top 5, All" options for 7 items', async () => {
169+
renderComponent({ ...defaultProps, count: 7, rowsPerPage: 3 });
170+
const select = screen.getByRole('combobox');
171+
172+
await userEvent.click(select);
173+
174+
const options = screen.getAllByRole('option');
175+
expect(options).toHaveLength(3);
176+
expect(options[0]).toHaveTextContent('Top 3');
177+
expect(options[1]).toHaveTextContent('Top 5');
178+
expect(options[2]).toHaveTextContent('All');
179+
expect(screen.queryByText('Top 7')).not.toBeInTheDocument();
180+
});
181+
182+
it('should show "Top 3, Top 5, Top 10" options for 10 items (exact match)', async () => {
183+
renderComponent({ ...defaultProps, count: 10, rowsPerPage: 5 });
184+
const select = screen.getByRole('combobox');
185+
186+
await userEvent.click(select);
187+
188+
const options = screen.getAllByRole('option');
189+
expect(options).toHaveLength(3);
190+
expect(options[0]).toHaveTextContent('Top 3');
191+
expect(options[1]).toHaveTextContent('Top 5');
192+
expect(options[2]).toHaveTextContent('Top 10');
193+
});
194+
195+
it('should show "Top 3, Top 5, Top 10, All" options for 11 items', async () => {
196+
renderComponent({ ...defaultProps, count: 11, rowsPerPage: 3 });
197+
const select = screen.getByRole('combobox');
198+
199+
await userEvent.click(select);
200+
201+
const options = screen.getAllByRole('option');
202+
expect(options).toHaveLength(4);
203+
expect(options[0]).toHaveTextContent('Top 3');
204+
expect(options[1]).toHaveTextContent('Top 5');
205+
expect(options[2]).toHaveTextContent('Top 10');
206+
expect(options[3]).toHaveTextContent('All');
207+
expect(screen.queryByText('Top 11')).not.toBeInTheDocument();
208+
});
209+
210+
it('should show "Top 3, Top 5, Top 10, Top 20" options for 20 items (exact match)', async () => {
211+
renderComponent({ ...defaultProps, count: 20, rowsPerPage: 5 });
212+
const select = screen.getByRole('combobox');
213+
214+
await userEvent.click(select);
215+
216+
const options = screen.getAllByRole('option');
217+
expect(options).toHaveLength(4);
218+
expect(options[0]).toHaveTextContent('Top 3');
219+
expect(options[1]).toHaveTextContent('Top 5');
220+
expect(options[2]).toHaveTextContent('Top 10');
221+
expect(options[3]).toHaveTextContent('Top 20');
222+
});
223+
224+
it('should show "Top 3, Top 5, Top 10, Top 20" options for 25 items (no All - exceeds max)', async () => {
225+
renderComponent({ ...defaultProps, count: 25, rowsPerPage: 5 });
226+
const select = screen.getByRole('combobox');
227+
228+
await userEvent.click(select);
229+
230+
const options = screen.getAllByRole('option');
231+
expect(options).toHaveLength(4);
232+
expect(options[0]).toHaveTextContent('Top 3');
233+
expect(options[1]).toHaveTextContent('Top 5');
234+
expect(options[2]).toHaveTextContent('Top 10');
235+
expect(options[3]).toHaveTextContent('Top 20');
236+
expect(screen.queryByText('All')).not.toBeInTheDocument();
237+
expect(screen.queryByText('Top 25')).not.toBeInTheDocument();
238+
});
239+
});
109240
});

‎workspaces/adoption-insights/plugins/adoption-insights/src/components/CatalogEntities/CatalogEntities.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,13 @@ const CatalogEntities = () => {
124124

125125
return (
126126
<CardWrapper
127-
title={t('catalogEntities.title')}
127+
title={
128+
rowsPerPage >= (catalogEntities.data?.length ?? 0)
129+
? t('catalogEntities.allTitle' as any, {})
130+
: t('catalogEntities.topNTitle' as any, {
131+
count: rowsPerPage.toString(),
132+
})
133+
}
128134
filter={
129135
<FilterDropdown
130136
selectedOption={selectedOption}

‎workspaces/adoption-insights/plugins/adoption-insights/src/components/CatalogEntities/__tests__/CatalogEntities.test.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ describe('CatalogEntities', () => {
134134

135135
it('should render the component with initial data', () => {
136136
renderComponent();
137-
expect(screen.getByText('Top catalog entities')).toBeInTheDocument();
137+
expect(screen.getByText('Top 3 catalog entities')).toBeInTheDocument();
138138
expect(screen.getAllByRole('row')).toHaveLength(5);
139139
});
140140

@@ -173,7 +173,7 @@ describe('CatalogEntities', () => {
173173
await user.click(select);
174174
await user.click(screen.getByText('Top 5'));
175175

176-
expect(screen.getByText('Top catalog entities')).toBeInTheDocument();
176+
expect(screen.getByText('Top 5 catalog entities')).toBeInTheDocument();
177177
});
178178

179179
it('should create correct entity links', () => {

‎workspaces/adoption-insights/plugins/adoption-insights/src/components/Plugins/Plugins.tsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import Box from '@mui/material/Box';
3131

3232
import CardWrapper from '../CardWrapper';
3333
import { PLUGINS_TABLE_HEADERS } from '../../utils/constants';
34+
3435
import { usePlugins } from '../../hooks/usePlugins';
3536
import TableFooterPagination from '../CardFooter';
3637
import { Line, LineChart, ResponsiveContainer } from 'recharts';
@@ -89,7 +90,11 @@ const Plugins = () => {
8990

9091
return (
9192
<CardWrapper
92-
title={t('plugins.topNTitle' as any, { count: rowsPerPage.toString() })}
93+
title={
94+
rowsPerPage >= (plugins.data?.length ?? 0)
95+
? t('plugins.allTitle' as any, {})
96+
: t('plugins.topNTitle' as any, { count: rowsPerPage.toString() })
97+
}
9398
>
9499
<Table aria-labelledby="Plugins" sx={{ width: '100%' }}>
95100
<TableHead>

‎workspaces/adoption-insights/plugins/adoption-insights/src/components/Techdocs/Techdocs.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,11 @@ const Techdocs = () => {
8787

8888
return (
8989
<CardWrapper
90-
title={t('techDocs.topNTitle' as any, { count: rowsPerPage.toString() })}
90+
title={
91+
rowsPerPage >= (techdocs.data?.length ?? 0)
92+
? t('techDocs.allTitle' as any, {})
93+
: t('techDocs.topNTitle' as any, { count: rowsPerPage.toString() })
94+
}
9195
>
9296
<Table aria-labelledby="TechDocs" sx={{ width: '100%' }}>
9397
<TableHead>

‎workspaces/adoption-insights/plugins/adoption-insights/src/components/Techdocs/__tests__/Techdocs.test.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,9 @@ describe('Techdocs', () => {
133133
const select = screen.getByRole('combobox');
134134

135135
await user.click(select);
136-
await user.click(screen.getByText('Top 5'));
136+
await user.click(screen.getByText('All'));
137137

138-
expect(screen.getByText('Top 5 TechDocs')).toBeInTheDocument();
138+
expect(screen.getByText('All TechDocs')).toBeInTheDocument();
139139
expect(screen.getAllByRole('row')).toHaveLength(6);
140140
});
141141

0 commit comments

Comments
 (0)