mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-09 10:21:03 +00:00
fix(blueprints): stop the deployment detail sheet flickering and show deploy progress (#1278)
* fix(blueprints): stop the blueprint detail sheet flickering while open The Fleet > Deployments blueprint detail sheet reloaded its data on a short timer while open, flickering the body through its loading skeleton every few seconds. The sheet's load effect was keyed on the refresh callback, which was memoized with the parent's onOpenChange prop. The parent passes a fresh onOpenChange closure on every render, so each parent re-render (driven by the Fleet view's polling) recreated refresh, re-ran the load effect, and refetched the blueprint. Every refetch flipped the loading flag, swapping the populated body for skeletons and back. Hold onOpenChange in a ref so refresh depends only on blueprintId. The load effect now runs on open and on blueprint change, not on every parent render. Switching to a different blueprint still refetches. * fix(blueprints): keep the blueprint detail body visible during a refresh After the sheet has loaded, a reload triggered by an action (apply, withdraw, accept, enable/disable, save) flipped the loading flag and swapped the whole body for skeletons until the reload finished, a brief flash on every action. Gate the skeleton on whether the blueprint has loaded yet, not on the loading flag. The skeleton now shows only on the first load; later reloads keep the populated body, deployment table, and compose source on screen while they run. * fix(blueprints): show progress while a fresh deploy runs Confirming a fresh deploy from the blueprint detail sheet starts a remote deploy that can take several seconds. The button only became disabled with no other change, so the action looked frozen. Show a spinner and a "Deploying" label on the button while the deploy runs, and dim it, so the click clearly registers as in progress.
This commit is contained in:
@@ -7,7 +7,7 @@
|
||||
* of them, so the sheet can never issue a request the API answers with 403.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
|
||||
import type { BlueprintSummary } from '@/lib/blueprintsApi';
|
||||
|
||||
vi.mock('@/lib/blueprintsApi', async (importOriginal) => {
|
||||
@@ -57,6 +57,79 @@ beforeEach(() => {
|
||||
vi.mocked(getBlueprint).mockResolvedValue(summary());
|
||||
});
|
||||
|
||||
describe('BlueprintDetail data fetching', () => {
|
||||
it('does not refetch when the parent re-renders with new callback identities', async () => {
|
||||
const { rerender } = render(
|
||||
<BlueprintDetail blueprintId={1} open onOpenChange={() => {}} onChanged={noop} canEdit distinctLabels={[]} />,
|
||||
);
|
||||
|
||||
// Let the initial load settle so the body content is on screen.
|
||||
expect(await screen.findByText('Show compose source')).toBeInTheDocument();
|
||||
const callsAfterLoad = vi.mocked(getBlueprint).mock.calls.length;
|
||||
|
||||
// A parent re-render (e.g. the Fleet view's polling) hands the open sheet a
|
||||
// brand-new onOpenChange closure every render. Before the fix that closure was
|
||||
// a refresh dependency, so the load effect re-ran on every parent render and
|
||||
// flickered the body through its loading skeleton. It must now keep showing the
|
||||
// data it already has instead of refetching.
|
||||
rerender(
|
||||
<BlueprintDetail blueprintId={1} open onOpenChange={() => {}} onChanged={noop} canEdit distinctLabels={[]} />,
|
||||
);
|
||||
rerender(
|
||||
<BlueprintDetail blueprintId={1} open onOpenChange={() => {}} onChanged={noop} canEdit distinctLabels={[]} />,
|
||||
);
|
||||
await Promise.resolve();
|
||||
|
||||
expect(vi.mocked(getBlueprint)).toHaveBeenCalledTimes(callsAfterLoad);
|
||||
});
|
||||
|
||||
it('refetches when blueprintId changes while the sheet stays open', async () => {
|
||||
const { rerender } = render(
|
||||
<BlueprintDetail blueprintId={1} open onOpenChange={noop} onChanged={noop} canEdit distinctLabels={[]} />,
|
||||
);
|
||||
expect(await screen.findByText('Show compose source')).toBeInTheDocument();
|
||||
const callsAfterLoad = vi.mocked(getBlueprint).mock.calls.length;
|
||||
|
||||
// Opening a different blueprint without closing the sheet must load the new one,
|
||||
// so blueprintId has to stay a refresh dependency.
|
||||
rerender(
|
||||
<BlueprintDetail blueprintId={2} open onOpenChange={noop} onChanged={noop} canEdit distinctLabels={[]} />,
|
||||
);
|
||||
await screen.findByText('Show compose source');
|
||||
|
||||
expect(vi.mocked(getBlueprint)).toHaveBeenCalledTimes(callsAfterLoad + 1);
|
||||
expect(vi.mocked(getBlueprint)).toHaveBeenLastCalledWith(2);
|
||||
});
|
||||
|
||||
it('keeps the loaded body on screen while a refresh is in flight', async () => {
|
||||
let settleRefresh = () => {};
|
||||
vi.mocked(getBlueprint)
|
||||
.mockResolvedValueOnce(summary())
|
||||
.mockImplementationOnce(
|
||||
() => new Promise<BlueprintSummary>((resolve) => { settleRefresh = () => resolve(summary()); }),
|
||||
);
|
||||
|
||||
render(
|
||||
<BlueprintDetail blueprintId={1} open onOpenChange={noop} onChanged={noop} canEdit distinctLabels={[]} />,
|
||||
);
|
||||
expect(await screen.findByText('Show compose source')).toBeInTheDocument();
|
||||
const callsAfterLoad = vi.mocked(getBlueprint).mock.calls.length;
|
||||
|
||||
// Applying reloads the blueprint. The populated body must stay mounted instead
|
||||
// of collapsing to the loading skeleton while that reload is in flight.
|
||||
fireEvent.click(screen.getByRole('button', { name: /apply now/i }));
|
||||
await waitFor(() => expect(vi.mocked(getBlueprint).mock.calls.length).toBe(callsAfterLoad + 1));
|
||||
|
||||
// The deployment table only renders in the loaded body branch, never in the
|
||||
// skeleton, so its presence proves the skeleton did not take over the refresh.
|
||||
expect(screen.getByText('Show compose source')).toBeInTheDocument();
|
||||
expect(screen.getByTestId('deployment-table')).toBeInTheDocument();
|
||||
|
||||
settleRefresh();
|
||||
await screen.findByText('Show compose source');
|
||||
});
|
||||
});
|
||||
|
||||
describe('BlueprintDetail action gating', () => {
|
||||
it('shows the Apply / Edit / Delete actions for an admin (canEdit)', async () => {
|
||||
render(
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useEffect, useState, useCallback } from 'react';
|
||||
import { useEffect, useRef, useState, useCallback } from 'react';
|
||||
import { Pencil, Pin, Play, Power, Trash2 } from 'lucide-react';
|
||||
import { SystemSheet, SheetSection } from '@/components/ui/system-sheet';
|
||||
import { Modal, ModalDestructiveHeader, ModalBody, ModalFooter } from '@/components/ui/modal';
|
||||
@@ -48,6 +48,12 @@ export function BlueprintDetail({ blueprintId, open, onOpenChange, onChanged, ca
|
||||
const [deleteConfirmText, setDeleteConfirmText] = useState('');
|
||||
const { nodes } = useNodes();
|
||||
|
||||
// Hold the latest onOpenChange without making it a refresh dependency. Parents
|
||||
// pass a fresh closure on every render, so binding refresh to it would re-run the
|
||||
// load effect on each parent render and flicker the body through its skeleton.
|
||||
const onOpenChangeRef = useRef(onOpenChange);
|
||||
useEffect(() => { onOpenChangeRef.current = onOpenChange; }, [onOpenChange]);
|
||||
|
||||
const refresh = useCallback(async () => {
|
||||
setLoading(true);
|
||||
try {
|
||||
@@ -55,11 +61,11 @@ export function BlueprintDetail({ blueprintId, open, onOpenChange, onChanged, ca
|
||||
setSummary(result);
|
||||
} catch (err) {
|
||||
toast.error(err instanceof Error ? err.message : 'Failed to load blueprint');
|
||||
onOpenChange(false);
|
||||
onOpenChangeRef.current(false);
|
||||
} finally {
|
||||
setLoading(false);
|
||||
}
|
||||
}, [blueprintId, onOpenChange]);
|
||||
}, [blueprintId]);
|
||||
|
||||
useEffect(() => {
|
||||
if (open) {
|
||||
@@ -245,7 +251,7 @@ export function BlueprintDetail({ blueprintId, open, onOpenChange, onChanged, ca
|
||||
footerContext={footerContext}
|
||||
size="lg"
|
||||
>
|
||||
{loading || !blueprint || !summary ? (
|
||||
{!blueprint || !summary ? (
|
||||
<div className="space-y-3">
|
||||
<Skeleton className="h-12 w-full" />
|
||||
<Skeleton className="h-32 w-full" />
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
/**
|
||||
* The fresh-deploy action triggers a remote deploy that can take several seconds.
|
||||
* The dialog must show an in-progress indicator while it runs, otherwise the click
|
||||
* looks like it did nothing.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import { StateReviewDialog } from './StateReviewDialog';
|
||||
|
||||
const baseProps = {
|
||||
open: true,
|
||||
onOpenChange: () => {},
|
||||
blueprintName: 'heimdall',
|
||||
nodeName: 'sencho-test-01',
|
||||
onAccept: () => {},
|
||||
};
|
||||
|
||||
describe('StateReviewDialog', () => {
|
||||
it('offers the fresh deploy action when idle', () => {
|
||||
render(<StateReviewDialog {...baseProps} busy={false} />);
|
||||
expect(screen.getByText('Deploy fresh')).toBeInTheDocument();
|
||||
expect(screen.queryByText(/deploying/i)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('shows an in-progress indicator and disables the action while deploying', () => {
|
||||
render(<StateReviewDialog {...baseProps} busy />);
|
||||
expect(screen.getByText(/deploying/i)).toBeInTheDocument();
|
||||
expect(screen.getByRole('button', { name: /deploying/i })).toBeDisabled();
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Database, Play } from 'lucide-react';
|
||||
import { Database, Loader2, Play } from 'lucide-react';
|
||||
import { Modal, ModalHeader, ModalBody, ModalFooter } from '@/components/ui/modal';
|
||||
import { Button } from '@/components/ui/button';
|
||||
|
||||
@@ -31,11 +31,20 @@ export function StateReviewDialog({
|
||||
type="button"
|
||||
onClick={() => onAccept('fresh')}
|
||||
disabled={busy}
|
||||
className="w-full text-left rounded-lg border border-card-border border-t-card-border-top bg-card hover:border-t-card-border-hover transition-colors p-3 cursor-pointer"
|
||||
className="w-full text-left rounded-lg border border-card-border border-t-card-border-top bg-card hover:border-t-card-border-hover transition-colors p-3 cursor-pointer disabled:opacity-60 disabled:cursor-not-allowed"
|
||||
>
|
||||
<div className="flex items-center gap-2 font-mono text-[10px] uppercase tracking-[0.2em] text-brand">
|
||||
<Play className="w-3 h-3" strokeWidth={1.5} />
|
||||
Deploy fresh
|
||||
{busy ? (
|
||||
<>
|
||||
<Loader2 className="w-3 h-3 animate-spin" strokeWidth={1.5} />
|
||||
Deploying…
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<Play className="w-3 h-3" strokeWidth={1.5} />
|
||||
Deploy fresh
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
<p className="text-xs text-stat-subtitle mt-1.5 leading-relaxed">
|
||||
Create empty named volumes on this node. The container starts with whatever default state its image carries.
|
||||
|
||||
Reference in New Issue
Block a user