Skip to content

Commit c5f454e

Browse files
malinskibeniaminmalinskibeniamin
authored
Merge pull request #2171 from redpanda-data/tanstack-router-followups
frontend: apply tanstack router best practices
2 parents 042c578 + 3052711 commit c5f454e

File tree

19 files changed

+222
-77
lines changed

19 files changed

+222
-77
lines changed

‎frontend/src/components/layout/sidebar.tsx‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import {
2929
SidebarMenuButton,
3030
SidebarMenuItem,
3131
SidebarProvider,
32-
SidebarRail,
3332
useSidebar,
3433
} from 'components/redpanda-ui/components/sidebar';
3534
import { ChevronsLeft, ChevronsRight, ChevronUp, LogOut, Settings } from 'lucide-react';
@@ -268,8 +267,6 @@ export function AppSidebar() {
268267
</SidebarMenuItem>
269268
</SidebarMenu>
270269
</SidebarFooter>
271-
272-
<SidebarRail />
273270
</Sidebar>
274271
);
275272
}

‎frontend/src/components/misc/login-complete.tsx‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import type { ApiError, UserData } from '../../state/rest-interfaces';
1919
import { uiState } from '../../state/ui-state';
2020
import { getBasePath } from '../../utils/env';
2121
import fetchWithTimeout from '../../utils/fetch-with-timeout';
22-
import { queryToObj } from '../../utils/query-helper';
2322

2423
class LoginCompletePage extends Component<{ provider: string }> {
2524
componentDidMount() {
@@ -30,14 +29,16 @@ class LoginCompletePage extends Component<{ provider: string }> {
3029
async completeLogin(provider: string, location: Location) {
3130
const query = location.search;
3231

33-
const queryObj = queryToObj(query);
34-
if (queryObj.error || queryObj.error_description) {
32+
const searchParams = new URLSearchParams(query);
33+
const error = searchParams.get('error');
34+
const errorDescription = searchParams.get('error_description');
35+
if (error || errorDescription) {
3536
let errorString = '';
36-
if (queryObj.error) {
37-
errorString += `Error: ${queryObj.error}\n`;
37+
if (error) {
38+
errorString += `Error: ${error}\n`;
3839
}
39-
if (queryObj.error_description) {
40-
errorString += `Description: ${queryObj.error_description}\n`;
40+
if (errorDescription) {
41+
errorString += `Description: ${errorDescription}\n`;
4142
}
4243
uiState.loginError = errorString.trim();
4344
appGlobal.historyReplace(`${getBasePath()}/login`);

‎frontend/src/components/misc/login.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,8 @@ const authenticationApi = observable({
101101

102102
const AUTH_ELEMENTS: Partial<Record<AuthenticationMethod, React.FC>> = {
103103
[AuthenticationMethod.NONE]: observer(() => {
104-
const { search } = useLocation();
105-
const searchParams = new URLSearchParams(search);
104+
const { searchStr } = useLocation();
105+
const searchParams = new URLSearchParams(searchStr);
106106
// Don't auto-redirect if there was an auth error - user needs to see the login page
107107
const hasError = searchParams.has('error_code') || authenticationApi.methodsErrorResponse !== null;
108108

@@ -218,8 +218,8 @@ const AUTH_ELEMENTS: Partial<Record<AuthenticationMethod, React.FC>> = {
218218
};
219219

220220
const LoginPage = observer(() => {
221-
const { search } = useLocation();
222-
const searchParams = new URLSearchParams(search);
221+
const { searchStr } = useLocation();
222+
const searchParams = new URLSearchParams(searchStr);
223223

224224
useEffect(() => {
225225
authenticationApi.refreshAuthenticationMethods();

‎frontend/src/components/pages/connect/overview.tsx‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
import { create } from '@bufbuild/protobuf';
1313
import { Badge, Box, DataTable, Stack, Tooltip } from '@redpanda-data/ui';
14-
import { useLocation } from '@tanstack/react-router';
1514
import ErrorResult from 'components/misc/error-result';
1615
import { Link, Text } from 'components/redpanda-ui/components/typography';
1716
import { WaitingRedpanda } from 'components/redpanda-ui/components/waiting-redpanda';
@@ -78,21 +77,20 @@ const getDefaultView = (defaultView: string): { initialTab: ConnectView; redpand
7877
}
7978
};
8079

81-
const WrapKafkaConnectOverview: FunctionComponent<{ matchedPath: string }> = (props) => {
82-
const { search } = useLocation();
83-
const searchParams = new URLSearchParams(search);
84-
const defaultTab = searchParams.get('defaultTab') || '';
85-
80+
const WrapKafkaConnectOverview: FunctionComponent<{
81+
matchedPath: string;
82+
defaultTab?: ConnectView;
83+
}> = (props) => {
8684
const { data: kafkaConnectors, isLoading: isLoadingKafkaConnectors } = useKafkaConnectConnectorsQuery();
8785

8886
const isKafkaConnectEnabled = kafkaConnectors?.isConfigured === true;
8987

9088
return (
9189
<KafkaConnectOverview
92-
defaultView={defaultTab}
90+
defaultView={props.defaultTab ?? ''}
9391
isKafkaConnectEnabled={isKafkaConnectEnabled}
9492
isLoadingKafkaConnectors={isLoadingKafkaConnectors}
95-
{...props}
93+
matchedPath={props.matchedPath}
9694
/>
9795
);
9896
};

‎frontend/src/components/pages/consumers/group-details.tsx‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,18 @@ import { api } from '../../../state/backend-api';
4444
import type { GroupDescription, GroupMemberDescription } from '../../../state/rest-interfaces';
4545
import { Features } from '../../../state/supported-features';
4646
import { uiSettings } from '../../../state/ui';
47-
import { editQuery, queryToObj } from '../../../utils/query-helper';
4847
import { Button, DefaultSkeleton, IconButton, numberToThousandsString } from '../../../utils/tsx-utils';
4948
import PageContent from '../../misc/page-content';
5049
import { ShortNum } from '../../misc/short-num';
5150
import { Statistic } from '../../misc/statistic';
5251
import { PageComponent, type PageInitHelper, type PageProps } from '../page';
5352
import AclList from '../topics/Tab.Acl/acl-list';
5453

54+
type GroupSearchParams = {
55+
q?: string;
56+
withLag?: boolean;
57+
};
58+
5559
const DEFAULT_MATCH_ALL_REGEX = /.*/s;
5660
const QUICK_SEARCH_REGEX_CACHE = new Map<string, RegExp>();
5761

@@ -71,21 +75,29 @@ function getQuickSearchRegex(pattern: string): RegExp {
7175
return regExp;
7276
}
7377

78+
type GroupDetailsProps = {
79+
groupId: string;
80+
search: GroupSearchParams;
81+
onSearchChange: (updates: Partial<GroupSearchParams>) => void;
82+
};
83+
7484
@observer
75-
class GroupDetails extends PageComponent<{ groupId: string }> {
85+
class GroupDetails extends PageComponent<GroupDetailsProps> {
7686
@observable edittingOffsets: GroupOffset[] | null = null;
7787
@observable editedTopic: string | null = null;
7888
@observable editedPartition: number | null = null;
7989

8090
@observable deletingMode: GroupDeletingMode = 'group';
8191
@observable deletingOffsets: GroupOffset[] | null = null;
8292

83-
@observable quickSearch = queryToObj(window.location.search).q;
84-
@observable showWithLagOnly = Boolean(queryToObj(window.location.search).withLag);
93+
@observable quickSearch = '';
94+
@observable showWithLagOnly = false;
8595

86-
constructor(p: Readonly<PageProps<{ groupId: string }>>) {
96+
constructor(p: Readonly<PageProps<GroupDetailsProps>>) {
8797
super(p);
8898
makeObservable(this);
99+
this.quickSearch = p.search?.q ?? '';
100+
this.showWithLagOnly = p.search?.withLag ?? false;
89101
}
90102

91103
initPage(p: PageInitHelper): void {
@@ -118,21 +130,16 @@ class GroupDetails extends PageComponent<{ groupId: string }> {
118130
placeholderText="Filter by member"
119131
searchText={this.quickSearch}
120132
setSearchText={(filterText) => {
121-
editQuery((query) => {
122-
this.quickSearch = filterText;
123-
const q = String(filterText);
124-
query.q = q;
125-
});
133+
this.quickSearch = filterText;
134+
this.props.onSearchChange({ q: filterText });
126135
}}
127136
width={300}
128137
/>
129138
<Checkbox
130139
isChecked={this.showWithLagOnly}
131140
onChange={(e) => {
132-
editQuery((query) => {
133-
this.showWithLagOnly = e.target.checked;
134-
query.withLag = this.showWithLagOnly ? 'true' : null;
135-
});
141+
this.showWithLagOnly = e.target.checked;
142+
this.props.onSearchChange({ withLag: e.target.checked });
136143
}}
137144
>
138145
Only show topics with lag

‎frontend/src/components/redpanda-ui/components/motion-highlight.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ function MotionHighlight<T extends string>(props: MotionHighlightProps<T>) {
102102
defaultValue,
103103
onValueChange,
104104
className,
105-
transition = { type: 'spring', stiffness: 350, damping: 35 },
105+
transition = { type: 'tween', ease: 'easeOut', duration: 0.2 },
106106
hover = false,
107107
enabled = true,
108108
controlledItems,

‎frontend/src/components/redpanda-ui/components/sidebar.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ function Sidebar({
251251
{/* This is what handles the sidebar gap on desktop */}
252252
<div
253253
className={cn(
254-
'relative w-(--sidebar-width) bg-transparent transition-[width] duration-400 ease-[cubic-bezier(0.7,-0.15,0.25,1.15)]',
254+
'relative w-(--sidebar-width) bg-transparent transition-[width] duration-300 ease-out',
255255
'group-data-[collapsible=offcanvas]:w-0',
256256
'group-data-[side=right]:rotate-180',
257257
variant === 'floating' || variant === 'inset'
@@ -262,7 +262,7 @@ function Sidebar({
262262
/>
263263
<div
264264
className={cn(
265-
'fixed inset-y-0 z-10 flex max-md:hidden h-svh w-(--sidebar-width) transition-[left,right,width] duration-400 ease-[cubic-bezier(0.75,0,0.25,1)]',
265+
'fixed inset-y-0 z-10 flex max-md:hidden h-svh w-(--sidebar-width) transition-[left,right,width] duration-300 ease-out',
266266
side === 'left'
267267
? 'left-0 group-data-[collapsible=offcanvas]:left-[calc(var(--sidebar-width)*-1)]'
268268
: 'right-0 group-data-[collapsible=offcanvas]:right-[calc(var(--sidebar-width)*-1)]',
@@ -723,7 +723,7 @@ function SidebarMenuSub({ className, ...props }: SidebarMenuSubProps) {
723723
return (
724724
<ul
725725
className={cn(
726-
'mx-3.5 flex min-w-0 translate-x-px flex-col gap-1 border-l px-2.5 py-0.5',
726+
'mx-3.5 flex min-w-0 translate-x-px flex-col gap-1 border-l px-1 py-0.5',
727727
'group-data-[collapsible=icon]:hidden',
728728
className
729729
)}
@@ -764,7 +764,7 @@ const SidebarMenuSubButton = React.forwardRef<HTMLAnchorElement, SidebarMenuSubB
764764
<Comp
765765
aria-disabled={props['aria-disabled']}
766766
className={cn(
767-
'flex h-7 min-w-0 -translate-x-px items-center gap-2 overflow-hidden rounded-md px-0.5 text-sidebar-foreground outline-hidden ring-sidebar-ring focus-visible:ring-2 active:bg-sidebar-accent active:text-sidebar-accent-foreground disabled:pointer-events-none disabled:opacity-50 aria-disabled:pointer-events-none aria-disabled:cursor-not-allowed aria-disabled:opacity-50 [&:not([data-highlight])]:hover:bg-sidebar-accent [&:not([data-highlight])]:hover:text-sidebar-accent-foreground [&>span:last-child]:truncate [&>svg]:size-4 [&>svg]:shrink-0 [&>svg]:text-sidebar-accent-foreground',
767+
'flex h-7 min-w-0 -translate-x-px items-center gap-2 overflow-hidden rounded-md px-2 text-sidebar-foreground outline-hidden ring-sidebar-ring focus-visible:ring-2 active:bg-sidebar-accent active:text-sidebar-accent-foreground disabled:pointer-events-none disabled:opacity-50 aria-disabled:pointer-events-none aria-disabled:cursor-not-allowed aria-disabled:opacity-50 [&:not([data-highlight])]:hover:bg-sidebar-accent [&:not([data-highlight])]:hover:text-sidebar-accent-foreground [&>span:last-child]:truncate [&>svg]:size-4 [&>svg]:shrink-0 [&>svg]:text-sidebar-accent-foreground',
768768
'data-[active=true]:bg-sidebar-accent data-[active=true]:text-sidebar-accent-foreground',
769769
size === 'sm' && 'text-xs',
770770
size === 'md' && 'text-sm',

‎frontend/src/components/redpanda-ui/components/tooltip.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ function TooltipContent({
8181
className,
8282
side = 'top',
8383
sideOffset = 4,
84-
transition = { type: 'spring', stiffness: 300, damping: 25 },
84+
transition = { type: 'tween', ease: 'easeOut', duration: 0.15 },
8585
arrow = true,
8686
children,
8787
testId,
@@ -104,7 +104,7 @@ function TooltipContent({
104104
exit={{ opacity: 0, scale: 0, ...initialPosition }}
105105
transition={transition}
106106
className={cn(
107-
'relative bg-primary text-primary-foreground shadow-md w-fit origin-(--radix-tooltip-content-transform-origin) rounded-md px-3 py-1.5 text-sm text-balance',
107+
'relative bg-base-800 dark:bg-base-700 text-white shadow-md w-fit origin-(--radix-tooltip-content-transform-origin) rounded-md px-3 py-1.5 text-sm text-balance',
108108
className,
109109
)}
110110
>
@@ -113,7 +113,7 @@ function TooltipContent({
113113
{arrow && (
114114
<TooltipPrimitive.Arrow
115115
data-slot="tooltip-content-arrow"
116-
className="bg-primary fill-primary z-50 size-2.5 translate-y-[calc(-50%-2px)] rotate-45 rounded-[2px]"
116+
className="bg-base-800 fill-base-800 dark:bg-base-700 dark:fill-base-700 z-50 size-2.5 translate-y-[calc(-50%-2px)] rotate-45 rounded-[2px]"
117117
/>
118118
)}
119119
</motion.div>

‎frontend/src/components/redpanda-ui/style/theme.css‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@
7676
--color-sidebar-accent: oklch(1 0 0 / 8%);
7777
--color-sidebar-accent-foreground: var(--color-base-100);
7878
--color-sidebar-border: var(--color-base-600);
79-
--color-sidebar-ring: var(--color-primary-500);
79+
--color-sidebar-ring: var(--color-secondary-400);
8080
--color-selection: oklch(0.8462 0.027 262.33);
8181
--color-selection-foreground: var(--color-base-800);
8282
--color-sidebar-selection: oklch(0.5547 0.087 264.14);
@@ -127,7 +127,7 @@
127127
--color-sidebar-accent: oklch(0 0 0 / 24%);
128128
--color-sidebar-accent-foreground: var(--color-base-100);
129129
--color-sidebar-border: var(--color-base-500);
130-
--color-sidebar-ring: var(--color-primary-500);
130+
--color-sidebar-ring: var(--color-secondary-400);
131131
--color-selection: oklch(0.8462 0.027 262.33);
132132
--color-selection-foreground: var(--color-base-200);
133133
--color-sidebar-selection: oklch(0.5547 0.087 264.14);

‎frontend/src/hooks/use-pagination-params.test.tsx‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,47 @@
11
import { renderHook } from '@testing-library/react';
2-
import { connectQueryWrapper } from 'test-utils';
2+
import { NuqsTestingAdapter } from 'nuqs/adapters/testing';
3+
import type { PropsWithChildren } from 'react';
34

45
import usePaginationParams from './use-pagination-params';
56

7+
const createWrapper =
8+
(searchParams?: Record<string, string>) =>
9+
({ children }: PropsWithChildren) => <NuqsTestingAdapter searchParams={searchParams}>{children}</NuqsTestingAdapter>;
10+
611
describe('usePaginationParams', () => {
712
test('returns default values when URL parameters are absent', () => {
8-
const { wrapper } = connectQueryWrapper();
9-
// Assuming a total data length of 100 for testing
1013
const totalDataLength = 100;
11-
const { result } = renderHook(() => usePaginationParams(totalDataLength, 10), { wrapper });
14+
const { result } = renderHook(() => usePaginationParams(totalDataLength, 10), {
15+
wrapper: createWrapper(),
16+
});
1217
expect(result.current.pageSize).toBe(10);
1318
expect(result.current.pageIndex).toBe(0);
1419
});
1520

1621
test('parses pageSize and pageIndex from URL parameters', () => {
17-
const { wrapper } = connectQueryWrapper();
1822
const totalDataLength = 100;
19-
const { result } = renderHook(() => usePaginationParams(totalDataLength, 10), { wrapper });
20-
// Without setting initial URL, defaults apply
21-
expect(result.current.pageSize).toBe(10);
22-
expect(result.current.pageIndex).toBe(0);
23+
const { result } = renderHook(() => usePaginationParams(totalDataLength, 10), {
24+
wrapper: createWrapper({ pageSize: '20', page: '2' }),
25+
});
26+
expect(result.current.pageSize).toBe(20);
27+
expect(result.current.pageIndex).toBe(2);
2328
});
2429

2530
test('uses defaultPageSize when pageSize is not in URL', () => {
26-
const { wrapper } = connectQueryWrapper();
2731
const totalDataLength = 150;
28-
const { result } = renderHook(() => usePaginationParams(totalDataLength, 15), { wrapper });
32+
const { result } = renderHook(() => usePaginationParams(totalDataLength, 15), {
33+
wrapper: createWrapper(),
34+
});
2935
expect(result.current.pageSize).toBe(15);
3036
expect(result.current.pageIndex).toBe(0);
3137
});
3238

33-
test('returns default pageIndex when URL page param would exceed total pages', () => {
34-
const { wrapper } = connectQueryWrapper();
39+
test('returns bounded pageIndex when URL page param would exceed total pages', () => {
3540
const totalDataLength = 50; // Only 5 pages available with pageSize 10
36-
const { result } = renderHook(() => usePaginationParams(totalDataLength, 10), { wrapper });
41+
const { result } = renderHook(() => usePaginationParams(totalDataLength, 10), {
42+
wrapper: createWrapper({ page: '10' }), // Page 10 exceeds available pages
43+
});
3744
expect(result.current.pageSize).toBe(10);
38-
expect(result.current.pageIndex).toBe(0);
45+
expect(result.current.pageIndex).toBe(4); // Bounded to max valid page (5 pages = index 0-4)
3946
});
4047
});

0 commit comments

Comments
 (0)