open-design/apps/web/tests/components/preview-modal-error-state.test.tsx
Nagendhra Madishetti 655d561f38
fix(web): show explicit error/retry state when example preview HTML fails to load (#863)
* 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>
2026-05-08 11:16:14 +08:00

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);
});
});