Skip to content
This repository was archived by the owner on Jun 26, 2026. It is now read-only.

Commit 3059f49

Browse files
matejvasekclaude
andcommitted
fix: address second round of PR #26 review comments
Remove tooltip from refresh button. Drop useCallback from onRefresh. Extract loadFunctions as a standalone async function called directly from onRefresh and from useEffect for initial load, eliminating the refreshKey state. Signed-off-by: Matej Vašek <matejvasek@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
1 parent ac2a97c commit 3059f49

1 file changed

Lines changed: 36 additions & 52 deletions

File tree

src/views/FunctionsListPage.tsx

Lines changed: 36 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,9 @@ import {
1313
Toolbar,
1414
ToolbarContent,
1515
ToolbarItem,
16-
Tooltip,
1716
} from '@patternfly/react-core';
1817
import { SyncAltIcon } from '@patternfly/react-icons';
19-
import { useCallback, useContext, useEffect, useMemo, useState } from 'react';
18+
import { useContext, useEffect, useMemo, useState } from 'react';
2019
import { useTranslation } from 'react-i18next';
2120
import { Link, useNavigate } from 'react-router-dom-v5-compat';
2221
import { FunctionsEmptyState } from '../components/EmptyState';
@@ -28,7 +27,6 @@ import {
2827
} from '../context/ForgeConnectionProvider';
2928
import { useClusterService } from '../services/cluster/useClusterService';
3029
import { useSourceControlService } from '../services/source-control/useSourceControlService';
31-
import { RepoMetadata } from '../services/types';
3230
import { errorMessage, parseNamespaceAndRuntime } from '../utils/utils';
3331

3432
export default function FunctionsListPage() {
@@ -87,17 +85,15 @@ function FunctionsListPageContent() {
8785
</ToolbarItem>
8886
<ToolbarItem variant="separator" />
8987
<ToolbarItem>
90-
<Tooltip content={t('Refresh')}>
91-
<Button
92-
variant="plain"
93-
aria-label={t('Refresh')}
94-
onClick={onRefresh}
95-
isLoading={refreshing}
96-
spinnerAriaLabel={t('Refreshing')}
97-
isDisabled={refreshing}
98-
icon={<SyncAltIcon />}
99-
/>
100-
</Tooltip>
88+
<Button
89+
variant="plain"
90+
aria-label={t('Refresh')}
91+
onClick={onRefresh}
92+
isLoading={refreshing}
93+
spinnerAriaLabel={t('Refreshing')}
94+
isDisabled={refreshing}
95+
icon={<SyncAltIcon />}
96+
/>
10197
</ToolbarItem>
10298
</ToolbarContent>
10399
</Toolbar>
@@ -127,13 +123,7 @@ function useFunctionListPage(): {
127123
const [prevConnectionId, setPrevConnectionId] = useState(connectionId);
128124

129125
const [error, setError] = useState<string>('');
130-
const [refreshKey, setRefreshKey] = useState(0);
131126
const [refreshing, setRefreshing] = useState(false);
132-
const onRefresh = useCallback(() => {
133-
if (!isConnectedToForge) return;
134-
setRefreshing(true);
135-
setRefreshKey((k) => k + 1);
136-
}, [isConnectedToForge]);
137127

138128
// Reset state when connection changes (initial connect or user switch)
139129
if (connectionId !== prevConnectionId) {
@@ -143,44 +133,38 @@ function useFunctionListPage(): {
143133
setReposLoaded(false);
144134
}
145135

146-
useEffect(() => {
147-
if (!isConnectedToForge) return;
148-
149-
let ignore = false;
150-
151-
async function loadFunctionTableItems() {
152-
let repos: RepoMetadata[];
153-
let items: FunctionTableItem[];
154-
try {
155-
repos = await sourceControl.listFunctionRepos();
156-
items = await Promise.all(
157-
repos.map(async (repo) => {
158-
const funcYaml = await sourceControl.fetchFileContent(repo, 'func.yaml');
159-
const { namespace, runtime } = parseNamespaceAndRuntime(funcYaml);
160-
return newItem(repo.name, namespace, runtime);
161-
}),
162-
);
163-
} catch (err) {
164-
if (!ignore) {
165-
setReposLoaded(true);
166-
setRefreshing(false);
167-
setError(errorMessage(err));
168-
}
169-
return;
170-
}
171-
if (ignore) return;
172-
136+
async function loadFunctions() {
137+
try {
138+
const repos = await sourceControl.listFunctionRepos();
139+
const items = await Promise.all(
140+
repos.map(async (repo) => {
141+
const funcYaml = await sourceControl.fetchFileContent(repo, 'func.yaml');
142+
const { namespace, runtime } = parseNamespaceAndRuntime(funcYaml);
143+
return newItem(repo.name, namespace, runtime);
144+
}),
145+
);
173146
setFunctionItems(items);
174147
setReposLoaded(true);
175148
setRefreshing(false);
176149
setError('');
150+
} catch (err) {
151+
setReposLoaded(true);
152+
setRefreshing(false);
153+
setError(errorMessage(err));
177154
}
155+
}
178156

179-
loadFunctionTableItems();
180-
return () => {
181-
ignore = true;
182-
};
183-
}, [sourceControl, isConnectedToForge, connectionId, refreshKey]);
157+
function onRefresh() {
158+
if (!isConnectedToForge) return;
159+
setRefreshing(true);
160+
loadFunctions();
161+
}
162+
163+
useEffect(() => {
164+
if (!isConnectedToForge) return;
165+
loadFunctions();
166+
// eslint-disable-next-line react-hooks/exhaustive-deps
167+
}, [sourceControl, isConnectedToForge, connectionId]);
184168

185169
const functionNames = useMemo(() => functionItems.map((item) => item.name), [functionItems]);
186170

0 commit comments

Comments
 (0)