mirror of
https://github.com/nexu-io/open-design.git
synced 2026-06-01 03:14:35 +07:00
* 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>
118 lines
3.3 KiB
TypeScript
118 lines
3.3 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import { cleanup, fireEvent, render, screen } from '@testing-library/react';
|
|
import { afterEach, describe, expect, it, vi } from 'vitest';
|
|
import { PreviewModal } from '../../src/components/PreviewModal';
|
|
|
|
// Regression coverage for nexu-io/open-design#860: when the example HTML
|
|
// fetch fails, the modal must render an explicit error/retry affordance
|
|
// instead of staying stuck at "Loading…" with the share menu disabled
|
|
// and no recovery path.
|
|
|
|
const baseProps = {
|
|
title: 'Example',
|
|
exportTitleFor: (id: string) => id,
|
|
};
|
|
|
|
describe('PreviewModal error state', () => {
|
|
afterEach(() => {
|
|
cleanup();
|
|
});
|
|
|
|
it('renders the error UI when the active view carries an error', () => {
|
|
render(
|
|
<PreviewModal
|
|
{...baseProps}
|
|
views={[
|
|
{
|
|
id: 'preview',
|
|
label: 'Preview',
|
|
html: undefined,
|
|
error: 'simulated failure',
|
|
},
|
|
]}
|
|
onView={() => {}}
|
|
onClose={() => {}}
|
|
/>,
|
|
);
|
|
|
|
expect(
|
|
screen.getByText("Couldn't load this example."),
|
|
).toBeTruthy();
|
|
expect(
|
|
screen.getByRole('button', { name: /retry/i }),
|
|
).toBeTruthy();
|
|
// Loading copy must NOT show alongside the error state.
|
|
expect(screen.queryByText(/loading/i)).toBeNull();
|
|
});
|
|
|
|
it('fires onView when Retry is clicked so the parent can re-run the fetch', () => {
|
|
const onView = vi.fn();
|
|
render(
|
|
<PreviewModal
|
|
{...baseProps}
|
|
views={[
|
|
{
|
|
id: 'preview',
|
|
label: 'Preview',
|
|
html: undefined,
|
|
error: 'simulated failure',
|
|
},
|
|
]}
|
|
onView={onView}
|
|
onClose={() => {}}
|
|
/>,
|
|
);
|
|
|
|
// Mount fires onView once with the initial activeId; clear the spy
|
|
// so the assertion targets only the Retry click.
|
|
onView.mockClear();
|
|
fireEvent.click(screen.getByRole('button', { name: /retry/i }));
|
|
expect(onView).toHaveBeenCalledTimes(1);
|
|
expect(onView).toHaveBeenCalledWith('preview');
|
|
});
|
|
|
|
it('does not re-fire onView on re-render when the same callback identity is passed', () => {
|
|
// Codex P2 regression: an inline `onView={() => loadPreview(...)}` was
|
|
// recreated on every parent render, and PreviewModal's mount effect
|
|
// re-fired onView on identity change, turning a persistent error into
|
|
// an automatic retry loop. The fix in ExamplesTab is to pass a
|
|
// stable-identity callback; this test pins that contract on the
|
|
// modal side by asserting that re-rendering with the same onView
|
|
// reference does not re-fire it.
|
|
const onView = vi.fn();
|
|
const { rerender } = render(
|
|
<PreviewModal
|
|
{...baseProps}
|
|
views={[
|
|
{
|
|
id: 'preview',
|
|
label: 'Preview',
|
|
html: undefined,
|
|
error: 'simulated failure',
|
|
},
|
|
]}
|
|
onView={onView}
|
|
onClose={() => {}}
|
|
/>,
|
|
);
|
|
expect(onView).toHaveBeenCalledTimes(1);
|
|
|
|
rerender(
|
|
<PreviewModal
|
|
{...baseProps}
|
|
views={[
|
|
{
|
|
id: 'preview',
|
|
label: 'Preview',
|
|
html: undefined,
|
|
error: 'simulated failure',
|
|
},
|
|
]}
|
|
onView={onView}
|
|
onClose={() => {}}
|
|
/>,
|
|
);
|
|
expect(onView).toHaveBeenCalledTimes(1);
|
|
});
|
|
});
|