Skip to content

Commit ed337aa

Browse files
jvorcakjvorcak
committed
Addressed comments from the PR
1 parent 4932bce commit ed337aa

8 files changed

Lines changed: 192 additions & 246 deletions

File tree

‎frontend/src/components/pages/topics/Tab.Acl/acl-list.tsx‎

Lines changed: 8 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,10 @@ import {
1515
getCoreRowModel,
1616
getPaginationRowModel,
1717
getSortedRowModel,
18-
type PaginationState,
19-
type SortingState,
20-
type Updater,
2118
useReactTable,
2219
} from '@tanstack/react-table';
23-
import { parseAsBoolean, parseAsInteger, parseAsString, useQueryState } from 'nuqs';
2420

25-
import { useQueryStateWithCallback } from '../../../../hooks/use-query-state-with-callback';
21+
import { useUrlTableState } from '../../../../hooks/use-url-table-state';
2622
import type {
2723
AclRule,
2824
AclStrOperation,
@@ -33,7 +29,6 @@ import type {
3329
} from '../../../../state/rest-interfaces';
3430
import { uiSettings } from '../../../../state/ui';
3531
import { toJson } from '../../../../utils/json-utils';
36-
import { DEFAULT_TABLE_PAGE_SIZE } from '../../../constants';
3732
import { Alert, AlertDescription } from '../../../redpanda-ui/components/alert';
3833
import { DataTableColumnHeader, DataTablePagination } from '../../../redpanda-ui/components/data-table';
3934
import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from '../../../redpanda-ui/components/table';
@@ -95,68 +90,18 @@ const columns: ColumnDef<AclFlatResource>[] = [
9590
const AclList = ({ acl }: { acl: Acls }) => {
9691
const resources = flatResourceList(acl);
9792

98-
const [pageIndex, setPageIndex] = useQueryState('aclPage', parseAsInteger.withDefault(0));
99-
100-
const [pageSize, setPageSize] = useQueryStateWithCallback<number>(
101-
{
102-
onUpdate: (val) => {
103-
uiSettings.topicAclList.pageSize = val;
104-
},
105-
getDefaultValue: () => uiSettings.topicAclList.pageSize,
106-
},
107-
'aclPageSize',
108-
parseAsInteger.withDefault(DEFAULT_TABLE_PAGE_SIZE)
109-
);
110-
111-
const [sortId, setSortId] = useQueryStateWithCallback<string>(
112-
{
113-
onUpdate: (val) => {
114-
uiSettings.topicAclList.sortId = val;
115-
},
116-
getDefaultValue: () => uiSettings.topicAclList.sortId,
117-
},
118-
'aclSortId',
119-
parseAsString.withDefault('')
120-
);
121-
122-
const [sortDesc, setSortDesc] = useQueryStateWithCallback<boolean>(
123-
{
124-
onUpdate: (val) => {
125-
uiSettings.topicAclList.sortDesc = val;
126-
},
127-
getDefaultValue: () => uiSettings.topicAclList.sortDesc,
128-
},
129-
'aclSortDesc',
130-
parseAsBoolean.withDefault(false)
131-
);
132-
133-
const sorting: SortingState = sortId ? [{ id: sortId, desc: sortDesc }] : [];
134-
const pagination: PaginationState = { pageIndex, pageSize };
135-
136-
const handleSortingChange = (updater: Updater<SortingState>) => {
137-
const next = typeof updater === 'function' ? updater(sorting) : updater;
138-
if (next.length > 0) {
139-
setSortId(next[0].id);
140-
setSortDesc(next[0].desc);
141-
} else {
142-
setSortId('');
143-
setSortDesc(false);
144-
}
145-
void setPageIndex(0);
146-
};
147-
148-
const handlePaginationChange = (updater: Updater<PaginationState>) => {
149-
const next = typeof updater === 'function' ? updater(pagination) : updater;
150-
void setPageIndex(next.pageIndex);
151-
setPageSize(next.pageSize);
152-
};
93+
const { sorting, pagination, onSortingChange, onPaginationChange } = useUrlTableState({
94+
keyPrefix: 'acl',
95+
settings: uiSettings.topicAclList,
96+
rowCount: resources.length,
97+
});
15398

15499
const table = useReactTable({
155100
data: resources,
156101
columns,
157102
state: { sorting, pagination },
158-
onSortingChange: handleSortingChange,
159-
onPaginationChange: handlePaginationChange,
103+
onSortingChange,
104+
onPaginationChange,
160105
getCoreRowModel: getCoreRowModel(),
161106
getSortedRowModel: getSortedRowModel(),
162107
getPaginationRowModel: getPaginationRowModel(),

‎frontend/src/components/pages/topics/tab-consumers.tsx‎

Lines changed: 9 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,15 @@ import {
1616
getCoreRowModel,
1717
getPaginationRowModel,
1818
getSortedRowModel,
19-
type PaginationState,
20-
type SortingState,
21-
type Updater,
2219
useReactTable,
2320
} from '@tanstack/react-table';
24-
import { parseAsBoolean, parseAsInteger, parseAsString, useQueryState } from 'nuqs';
2521
import { type FC, useEffect } from 'react';
2622

27-
import { useQueryStateWithCallback } from '../../../hooks/use-query-state-with-callback';
23+
import { useUrlTableState } from '../../../hooks/use-url-table-state';
2824
import { api, useApiStoreHook } from '../../../state/backend-api';
2925
import type { Topic, TopicConsumer } from '../../../state/rest-interfaces';
3026
import { uiSettings } from '../../../state/ui';
3127
import { DefaultSkeleton } from '../../../utils/tsx-utils';
32-
import { DEFAULT_TABLE_PAGE_SIZE } from '../../constants';
3328
import { DataTableColumnHeader, DataTablePagination } from '../../redpanda-ui/components/data-table';
3429
import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from '../../redpanda-ui/components/table';
3530

@@ -44,61 +39,12 @@ export const TopicConsumers: FC<TopicConsumersProps> = ({ topic }) => {
4439
const isLoading = rawConsumers === undefined;
4540
const consumers = rawConsumers ?? [];
4641

47-
const [pageIndex, setPageIndex] = useQueryState('consumerPage', parseAsInteger.withDefault(0));
48-
49-
const [pageSize, setPageSize] = useQueryStateWithCallback<number>(
50-
{
51-
onUpdate: (val) => {
52-
uiSettings.topicConsumersList.pageSize = val;
53-
},
54-
getDefaultValue: () => uiSettings.topicConsumersList.pageSize,
55-
},
56-
'consumerPageSize',
57-
parseAsInteger.withDefault(DEFAULT_TABLE_PAGE_SIZE)
58-
);
59-
60-
const [sortId, setSortId] = useQueryStateWithCallback<string>(
61-
{
62-
onUpdate: (val) => {
63-
uiSettings.topicConsumersList.sortId = val;
64-
},
65-
getDefaultValue: () => uiSettings.topicConsumersList.sortId,
66-
},
67-
'consumerSortId',
68-
parseAsString.withDefault('')
69-
);
70-
71-
const [sortDesc, setSortDesc] = useQueryStateWithCallback<boolean>(
72-
{
73-
onUpdate: (val) => {
74-
uiSettings.topicConsumersList.sortDesc = val;
75-
},
76-
getDefaultValue: () => uiSettings.topicConsumersList.sortDesc,
77-
},
78-
'consumerSortDesc',
79-
parseAsBoolean.withDefault(false)
80-
);
81-
82-
const sorting: SortingState = sortId ? [{ id: sortId, desc: sortDesc }] : [];
83-
const pagination: PaginationState = { pageIndex, pageSize };
84-
85-
const handleSortingChange = (updater: Updater<SortingState>) => {
86-
const next = typeof updater === 'function' ? updater(sorting) : updater;
87-
if (next.length > 0) {
88-
setSortId(next[0].id);
89-
setSortDesc(next[0].desc);
90-
} else {
91-
setSortId('');
92-
setSortDesc(false);
93-
}
94-
void setPageIndex(0);
95-
};
96-
97-
const handlePaginationChange = (updater: Updater<PaginationState>) => {
98-
const next = typeof updater === 'function' ? updater(pagination) : updater;
99-
void setPageIndex(next.pageIndex);
100-
setPageSize(next.pageSize);
101-
};
42+
const { sorting, pagination, onSortingChange, onPaginationChange } = useUrlTableState({
43+
keyPrefix: 'consumer',
44+
settings: uiSettings.topicConsumersList,
45+
rowCount: consumers.length,
46+
enabled: !isLoading,
47+
});
10248

10349
const columns: ColumnDef<TopicConsumer>[] = [
10450
{
@@ -125,8 +71,8 @@ export const TopicConsumers: FC<TopicConsumersProps> = ({ topic }) => {
12571
data: consumers,
12672
columns,
12773
state: { sorting, pagination },
128-
onSortingChange: handleSortingChange,
129-
onPaginationChange: handlePaginationChange,
74+
onSortingChange,
75+
onPaginationChange,
13076
getCoreRowModel: getCoreRowModel(),
13177
getSortedRowModel: getSortedRowModel(),
13278
getPaginationRowModel: getPaginationRowModel(),

‎frontend/src/components/pages/topics/tab-partitions.tsx‎

Lines changed: 11 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -15,23 +15,18 @@ import {
1515
getCoreRowModel,
1616
getPaginationRowModel,
1717
getSortedRowModel,
18-
type PaginationState,
19-
type SortingState,
20-
type Updater,
2118
useReactTable,
2219
} from '@tanstack/react-table';
2320
import { AlertTriangle } from 'lucide-react';
24-
import { parseAsBoolean, parseAsInteger, parseAsString, useQueryState } from 'nuqs';
2521
import type { FC } from 'react';
2622

2723
import '../../../utils/array-extensions';
2824

29-
import { useQueryStateWithCallback } from '../../../hooks/use-query-state-with-callback';
25+
import { useUrlTableState } from '../../../hooks/use-url-table-state';
3026
import { useApiStoreHook } from '../../../state/backend-api';
3127
import type { Partition, Topic } from '../../../state/rest-interfaces';
3228
import { uiSettings } from '../../../state/ui';
3329
import { DefaultSkeleton, numberToThousandsString } from '../../../utils/tsx-utils';
34-
import { DEFAULT_TABLE_PAGE_SIZE } from '../../constants';
3530
import { BrokerList } from '../../misc/broker-list';
3631
import { Alert, AlertDescription } from '../../redpanda-ui/components/alert';
3732
import { Badge } from '../../redpanda-ui/components/badge';
@@ -45,40 +40,13 @@ export const TopicPartitions: FC<TopicPartitionsProps> = ({ topic }) => {
4540
const partitions = useApiStoreHook((s) => s.topicPartitions.get(topic.topicName));
4641
const clusterHealth = useApiStoreHook((s) => s.clusterHealth);
4742

48-
const [pageIndex, setPageIndex] = useQueryState('partitionPage', parseAsInteger.withDefault(0));
49-
50-
const [pageSize, setPageSize] = useQueryStateWithCallback<number>(
51-
{
52-
onUpdate: (val) => {
53-
uiSettings.topicPartitionsList.pageSize = val;
54-
},
55-
getDefaultValue: () => uiSettings.topicPartitionsList.pageSize,
56-
},
57-
'partitionPageSize',
58-
parseAsInteger.withDefault(DEFAULT_TABLE_PAGE_SIZE)
59-
);
60-
61-
const [sortId, setSortId] = useQueryStateWithCallback<string>(
62-
{
63-
onUpdate: (val) => {
64-
uiSettings.topicPartitionsList.sortId = val;
65-
},
66-
getDefaultValue: () => uiSettings.topicPartitionsList.sortId,
67-
},
68-
'partitionSortId',
69-
parseAsString.withDefault('')
70-
);
71-
72-
const [sortDesc, setSortDesc] = useQueryStateWithCallback<boolean>(
73-
{
74-
onUpdate: (val) => {
75-
uiSettings.topicPartitionsList.sortDesc = val;
76-
},
77-
getDefaultValue: () => uiSettings.topicPartitionsList.sortDesc,
78-
},
79-
'partitionSortDesc',
80-
parseAsBoolean.withDefault(false)
81-
);
43+
// Kept above the early returns so the hook order stays stable; clamping no-ops until partitions load.
44+
const { sorting, pagination, onSortingChange, onPaginationChange } = useUrlTableState({
45+
keyPrefix: 'partition',
46+
settings: uiSettings.topicPartitionsList,
47+
rowCount: Array.isArray(partitions) ? partitions.length : 0,
48+
enabled: Array.isArray(partitions),
49+
});
8250

8351
if (partitions === undefined) {
8452
return DefaultSkeleton;
@@ -95,27 +63,6 @@ export const TopicPartitions: FC<TopicPartitionsProps> = ({ topic }) => {
9563
({ topicName }) => topicName === topic.topicName
9664
)?.partitionIds;
9765

98-
const sorting: SortingState = sortId ? [{ id: sortId, desc: sortDesc }] : [];
99-
const pagination: PaginationState = { pageIndex, pageSize };
100-
101-
const handleSortingChange = (updater: Updater<SortingState>) => {
102-
const next = typeof updater === 'function' ? updater(sorting) : updater;
103-
if (next.length > 0) {
104-
setSortId(next[0].id);
105-
setSortDesc(next[0].desc);
106-
} else {
107-
setSortId('');
108-
setSortDesc(false);
109-
}
110-
void setPageIndex(0);
111-
};
112-
113-
const handlePaginationChange = (updater: Updater<PaginationState>) => {
114-
const next = typeof updater === 'function' ? updater(pagination) : updater;
115-
void setPageIndex(next.pageIndex);
116-
setPageSize(next.pageSize);
117-
};
118-
11966
const columns: ColumnDef<Partition>[] = [
12067
{
12168
accessorKey: 'id',
@@ -160,8 +107,8 @@ export const TopicPartitions: FC<TopicPartitionsProps> = ({ topic }) => {
160107
data: partitions,
161108
columns,
162109
state: { sorting, pagination },
163-
onSortingChange: handleSortingChange,
164-
onPaginationChange: handlePaginationChange,
110+
onSortingChange,
111+
onPaginationChange,
165112
getCoreRowModel: getCoreRowModel(),
166113
getSortedRowModel: getSortedRowModel(),
167114
getPaginationRowModel: getPaginationRowModel(),
@@ -218,7 +165,7 @@ const PartitionError: FC<{ partition: Partition }> = ({ partition }) => {
218165
return (
219166
<Popover>
220167
<PopoverTrigger asChild>
221-
<button className="inline-flex" type="button">
168+
<button aria-label="Show partition error details" className="inline-flex" type="button">
222169
<AlertTriangle className="h-5 w-5 text-orange-500" />
223170
</button>
224171
</PopoverTrigger>

‎frontend/src/components/pages/topics/topic-configuration.test.tsx‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,12 @@ describe('TopicConfiguration', () => {
9090
describe('grouped layout (enableNewTopicPage on)', () => {
9191
beforeEach(() => mockIsFeatureFlagEnabled.mockReturnValue(true));
9292

93-
test('renders a sidebar and titled sections, collapsing unmapped categories into Other', () => {
93+
test('renders a sidebar and titled sections, preserving backend categories and collapsing only unmapped ones into Other', () => {
9494
const entries: ConfigEntryExtended[] = [
9595
makeEntry({ name: 'retention.ms', category: 'Retention', isExplicitlySet: true }),
9696
makeEntry({ name: 'cleanup.policy', category: 'Compaction' }),
9797
makeEntry({ name: 'redpanda.iceberg.mode', category: 'Iceberg' }),
98+
makeEntry({ name: 'some.unknown.option', category: 'Totally Unknown' }),
9899
];
99100

100101
render(
@@ -107,12 +108,13 @@ describe('TopicConfiguration', () => {
107108
/>
108109
);
109110

110-
// Sidebar lists each visible category; unmapped 'Iceberg' collapses into 'Other'.
111+
// Sidebar lists each visible category; known backend categories like 'Iceberg' are
112+
// preserved, and only genuinely unmapped categories collapse into 'Other'.
111113
const nav = screen.getByRole('navigation', { name: 'Configuration categories' });
112114
expect(within(nav).getByText('Retention')).toBeVisible();
113115
expect(within(nav).getByText('Compaction')).toBeVisible();
116+
expect(within(nav).getByText('Iceberg')).toBeVisible();
114117
expect(within(nav).getByText('Other')).toBeVisible();
115-
expect(within(nav).queryByText('Iceberg')).not.toBeInTheDocument();
116118

117119
// Each visible category renders as a titled section.
118120
const retentionSection = screen.getByRole('heading', { name: 'Retention' }).closest('section') as HTMLElement;

0 commit comments

Comments
 (0)