mirror of
https://github.com/nexu-io/open-design.git
synced 2026-06-01 03:14:35 +07:00
655d561f38
* fix(web): show explicit error/retry state when example preview HTML fails to load Reporter (#860) saw the example preview modal stuck with the toolbar buttons greyed out and only restarting the app got back to a usable state. Lefarcen confirmed the diagnosis: when /api/skills/:id/example fails, fetchSkillExample returns null, the modal stays at preview.loading forever, and the share menu's disabled={!activeHtml} guard sits in the disabled position with no recovery path. Three changes: 1. fetchSkillExample now returns a discriminated { html } | { error } instead of collapsing every failure into null, so callers can tell a real fetch failure from a normal load. 2. PreviewView gains an optional error field. When set, PreviewModal renders a stacked title/body/Retry affordance instead of the indefinite "Loading…" placeholder. Retry re-fires onView so the parent can re-run its fetch. 3. ExamplesTab tracks per-skill errors alongside per-skill html, clears the in-flight value before each fetch, and wires onView from the modal into loadPreview so the Retry button actually retries. i18n: three new keys (preview.errorTitle, preview.errorBody, preview.retry), translated across all 17 locales. The locales-aligned test stays green. CSS: .ds-modal-error stacks the new content vertically inside the existing .ds-modal-empty positioning, no other modals affected. * fix(web): stabilize preview onView and guard parallel preview fetches Codex caught a real bug in the round-1 fix: the inline onView={() => loadPreview(...)} prop was recreated on every parent render, and PreviewModal's mount effect re-fires onView whenever its identity changes. A persistent fetch failure would update state, recreate the prop, re-fire the effect, re-run loadPreview, and burn through the error UI in a flash instead of waiting for a Retry click. Pin a stable onPreviewView via a useRef-backed callback so the modal sees a single identity for the lifetime of the panel; loadPreview is reached through the ref, so its closure refresh on state updates no longer leaks into the modal's effect dependencies. While in this surface, also add lefarcen's race guard: a synchronous inFlightRef Set so two parallel loadPreview calls (e.g. card hover firing while the modal opens) cannot both pass the cache check before either setState lands. The first caller adds the id pre-await; the second sees it and exits early. try/finally clears the entry on both success and failure paths. Adds tests/components/preview-modal-error-state.test.tsx covering: - error UI renders when view.error is set, - Retry click calls onView with the active view id, - re-rendering with the same onView identity does not re-fire the modal's mount effect (pins the no-auto-retry contract). * fix(web): close Retry over the active skill id, not the modal-internal view id mrcfps caught a real regression in round 2: PreviewModal calls onView(activeId) where activeId is the modal-local view id ('preview' in this component). The previous round forwarded that argument straight into loadPreview, so the mount effect and Retry button hit /api/skills/preview/example instead of /api/skills/{skill-id}/example. The new error state could not actually recover. Mirror the active skill id into a ref alongside loadPreviewRef and have onPreviewView ignore the modal-forwarded argument, fetching the selected skill via the ref instead. The callback identity stays stable, so the no-auto-retry contract from round 2 still holds. Adds tests/components/examples-tab-retry.test.tsx that mounts the real ExamplesTab, mocks fetchSkillExample to reject, opens the preview, clicks Retry, and asserts the second call hits the same skill id (and explicitly never gets called with 'preview'). --------- Co-authored-by: Nagendhra <nagendhra405@gmail.com>
84 lines
2.6 KiB
TypeScript
84 lines
2.6 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import {
|
|
act,
|
|
cleanup,
|
|
fireEvent,
|
|
render,
|
|
screen,
|
|
} from '@testing-library/react';
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
|
import type { SkillSummary } from '../../src/types';
|
|
|
|
// Regression coverage for nexu-io/open-design#860 (round 3): the modal's
|
|
// onView fires with the modal-internal view id ('preview'), not the
|
|
// active skill id. The Retry path must close over the selected skill so
|
|
// re-fires hit /api/skills/{skill-id}/example, not /api/skills/preview/example.
|
|
|
|
vi.mock('../../src/providers/registry', () => ({
|
|
fetchSkillExample: vi.fn(),
|
|
}));
|
|
|
|
import { fetchSkillExample } from '../../src/providers/registry';
|
|
import { ExamplesTab } from '../../src/components/ExamplesTab';
|
|
|
|
const mockedFetch = fetchSkillExample as unknown as ReturnType<typeof vi.fn>;
|
|
|
|
const sampleSkill: SkillSummary = {
|
|
id: 'live-dashboard',
|
|
name: 'Live Dashboard',
|
|
description: 'A team dashboard live artifact.',
|
|
triggers: ['dashboard'],
|
|
mode: 'prototype',
|
|
previewType: 'web',
|
|
designSystemRequired: false,
|
|
defaultFor: [],
|
|
upstream: null,
|
|
hasBody: true,
|
|
examplePrompt: 'Build me a Notion-style team dashboard.',
|
|
};
|
|
|
|
async function flushPromises() {
|
|
await act(async () => {
|
|
await Promise.resolve();
|
|
});
|
|
}
|
|
|
|
describe('ExamplesTab preview retry path (#860)', () => {
|
|
beforeEach(() => {
|
|
mockedFetch.mockReset();
|
|
});
|
|
|
|
afterEach(() => {
|
|
cleanup();
|
|
});
|
|
|
|
it('Retry refetches the active skill, not the modal-internal view id', async () => {
|
|
mockedFetch.mockResolvedValue({ error: 'simulated failure' });
|
|
|
|
render(<ExamplesTab skills={[sampleSkill]} onUsePrompt={() => {}} />);
|
|
|
|
// Open the preview modal for the sample skill.
|
|
const openButtons = screen.getAllByText(/open preview/i);
|
|
fireEvent.click(openButtons[0]!);
|
|
|
|
// Initial fetch on mount.
|
|
await flushPromises();
|
|
expect(mockedFetch).toHaveBeenCalledTimes(1);
|
|
expect(mockedFetch).toHaveBeenLastCalledWith('live-dashboard');
|
|
|
|
// Error UI replaces the loading placeholder.
|
|
expect(screen.getByText("Couldn't load this example.")).toBeTruthy();
|
|
const retry = screen.getByRole('button', { name: /retry/i });
|
|
|
|
// Retry must hit the same skill id, NOT 'preview' (the modal view id).
|
|
fireEvent.click(retry);
|
|
await flushPromises();
|
|
|
|
expect(mockedFetch).toHaveBeenCalledTimes(2);
|
|
expect(mockedFetch).toHaveBeenLastCalledWith('live-dashboard');
|
|
// Defensive: a regression that wires the modal view id back into the
|
|
// fetcher would call with 'preview' here.
|
|
expect(mockedFetch).not.toHaveBeenCalledWith('preview');
|
|
});
|
|
});
|