Skip to content

Commit 4711b3d

Browse files
committed
fix(scripts): keep the script list mounted during a same-entity refresh
The list was cleared and the loading spinner shown synchronously on every reload, including a manual Refresh or the reload triggered after an upload or a delete, not just an actual entity switch. That unmounted every row on screen for the duration of the refetch, dropping whatever the user was doing inside one, such as an expanded parameter form. An entity-key ref now tells an actual entity switch apart from a same-entity reload. Only the former clears the list and shows the spinner; the latter keeps the current rows rendered until the refetch resolves.
1 parent 651bbbd commit 4711b3d

2 files changed

Lines changed: 49 additions & 6 deletions

File tree

src/components/ScriptsPanel.test.tsx

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,36 @@ describe('ScriptsPanel', () => {
229229
});
230230
});
231231

232+
it('keeps the existing rows on screen while a same-entity refresh is in flight', async () => {
233+
let resolveSecond: (value: ScriptsFetchResult) => void = () => {};
234+
mockFetchEntityScripts.mockResolvedValueOnce({ items: [makeScript({ id: 'a' })] }).mockImplementationOnce(
235+
() =>
236+
new Promise<ScriptsFetchResult>((resolve) => {
237+
resolveSecond = resolve;
238+
})
239+
);
240+
241+
render(<ScriptsPanel entityId="ecu" entityType="components" />);
242+
await waitFor(() => {
243+
expect(screen.getByTestId('script-row-a')).toBeInTheDocument();
244+
});
245+
246+
const user = userEvent.setup();
247+
await user.click(screen.getByRole('button', { name: /refresh/i }));
248+
249+
// The refetch above is for the same entity, so the row already on
250+
// screen must stay mounted and the loading spinner must not replace
251+
// it - unmounting it here is exactly what would collapse an expanded
252+
// row's in-progress parameter form back to nothing.
253+
expect(screen.getByTestId('script-row-a')).toBeInTheDocument();
254+
expect(screen.queryByText(/Loading scripts/i)).not.toBeInTheDocument();
255+
256+
await act(async () => {
257+
resolveSecond({ items: [makeScript({ id: 'a' })] });
258+
await Promise.resolve();
259+
});
260+
});
261+
232262
it('reloads the list after a successful upload', async () => {
233263
mockFetchEntityScripts
234264
.mockResolvedValueOnce({ items: [] })

src/components/ScriptsPanel.tsx

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,18 +57,30 @@ export function ScriptsPanel({ entityId, entityType }: ScriptsPanelProps) {
5757
const [uploadOpen, setUploadOpen] = useState(false);
5858

5959
const abortRef = useRef<AbortController | null>(null);
60+
// Identifies the entity the currently-rendered `scripts` belong to, so a
61+
// reload of the *same* entity (Refresh, or the reload after an upload or
62+
// a delete) can be told apart from an actual entity switch.
63+
const currentEntityKeyRef = useRef<string | null>(null);
6064

6165
useEffect(() => {
6266
abortRef.current?.abort();
6367
const controller = new AbortController();
6468
abortRef.current = controller;
6569

66-
// Clear whatever the previous entity (or previous attempt) rendered so
67-
// it never lingers under the new heading while the fresh request is
68-
// in flight.
69-
setScripts([]);
70-
setErrorStatus(null);
71-
setIsLoading(true);
70+
const entityKey = `${entityType}:${entityId}`;
71+
const isEntityChange = currentEntityKeyRef.current !== entityKey;
72+
currentEntityKeyRef.current = entityKey;
73+
74+
if (isEntityChange) {
75+
// Only an actual entity switch discards what is currently shown.
76+
// A same-entity reload keeps rendering the existing rows while
77+
// the refetch is in flight - clearing them here would unmount
78+
// every row (and whatever the user was doing inside one, like a
79+
// half-filled parameter form) for the duration of the request.
80+
setScripts([]);
81+
setErrorStatus(null);
82+
setIsLoading(true);
83+
}
7284

7385
const load = async () => {
7486
try {
@@ -79,6 +91,7 @@ export function ScriptsPanel({ entityId, entityType }: ScriptsPanelProps) {
7991
setErrorStatus(result.errorStatus);
8092
} else {
8193
setScripts(result.items);
94+
setErrorStatus(null);
8295
}
8396
} catch (err) {
8497
if ((err as { name?: string }).name === 'AbortError') return;

0 commit comments

Comments
 (0)