Skip to content

Commit c4c515a

Browse files
fix(app-shell): announce the route object-read and page record-load failures (#27366)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ8MCjwDfBRBbh1qJBm2tbXAr)_ Part of #25915 (slice 3 of ruling B; slice 4, the retirement of `ViewSkeleton` / `ShimmerSkeleton`, is the PR that closes the card). Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:控制台里两处"加载失败"画面(路由对象读取失败、自定义页面记录读取失败)改用共享的 `DataErrorState` 组件绘制。 - 为什么改:原来这两处用无 `role` 的 `Empty` 手绘,读屏软件在读取失败时什么都听不到;共享组件天生 `role="alert"`,失败会被播报,也与"空"区分开。 - 风险与回滚:只换呈现组件,文案、i18n 键、测试 id、重试按钮都不变;视觉上多一个红色小图标方块。回滚即还原这两个文件。 - 先看:`ui/packages/app-shell/src/views/PageView.tsx` 的失败分支;`ui/packages/app-shell/src/console/AppContent.tsx` 的 `RouteObjectReadFailed`。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/__tests__/PageView.appPageRecord-12931.test.tsx`,预期全部通过,含新增的 `#25915` 用例。 - 需要你做的:无,席位落地。 ## What changes - `RouteObjectReadFailed` (`ui/packages/app-shell/src/console/AppContent.tsx`) and the page record-scope load failure (`ui/packages/app-shell/src/views/PageView.tsx`) render `DataErrorState` from `@object-ui/components` instead of a hand-drawn `Empty`. The failure node is now `role="alert"` by construction. - Kept unchanged: the i18n keys and defaults (`list.loadErrorTitle` / `list.loadErrorMessage`, `empty.recordLoadFailed` / `empty.recordLoadFailedDescription`, `common.retry`), the test ids (`route-object-read-failed`, `page-record-load-failed` and their `-retry` buttons) and both Retry handlers. - No new prop on the shared component: the test id spreads onto the root through `...props`, and Retry is passed as `children` (the component's documented slot for a call site's own control) rather than `onRetry`, so it keeps its own test id and, on the page, its icon. The shared default icon (destructive `AlertCircle` square) replaces the page block's muted `AlertTriangle`; `AlertTriangle` drops from the page's imports. - Changeset: `@object-ui/app-shell` is `private: true`, but its code ships inside the published `@object-ui/console` SPA, so the changeset bumps both as `patch` (the same pair recent app-shell fixes carry). ## Tests - New pin in `AppContent.disabledPackageDeepLink-13052.test.tsx`: on a 5xx read the `route-object-read-failed` node has `role="alert"` and contains its Retry; Retry asks once more and the served object renders with no alert. The existing served-object CONTROL also asserts no alert. - New pin in `PageView.appPageRecord-12931.test.tsx`: the failed read's node has `role="alert"` and contains Retry; Retry re-reads, the loaded row renders, and no alert remains. ## Acceptance notes Other app-shell blocks that draw a load failure by hand (listed for the card; not moved here, per the order): `views/metadata-admin/AuditPanel.tsx` (`audit-error-state`, an `Empty`), `views/metadata-admin/ResourceEditPage.tsx` (`refs-error`, an `Empty`), `views/metadata-admin/DirectoryPage.tsx` (the `engine.directory.loadFailed` line, a bare `text-destructive` div), and `chrome/ErrorBoundary.tsx` (the boundary fallback, an `Empty`). `objectViewHeaderChrome.tsx` also has one but is under the `ObjectView*` exclusion. ## Verification All readings at head `72976b7a` (base `eef83734`). <details><summary>Tests and ablation</summary> - The two touched files, under `os-verify-lock`: `pnpm --dir ui exec vitest run --maxWorkers=2` on both. VERDICT command-exit 0, Test Files 2 passed (2), Tests 23 passed (23). - Ablation, with the fix committed: both source files reverted to `origin/main` (the `Empty` blocks: a `grep -c` for the `DataErrorState` JSX opener reads 0 in each file, and for the old `Empty` opener carrying the test id reads 1 in each). VERDICT command-exit 1, Tests 2 failed | 21 passed (23). The two `#25915` pins went red on `toHaveAttribute('role', 'alert')`, and every other test stayed green. Restored with `git checkout HEAD --`: both blob hashes equal `HEAD`, `git diff HEAD` empty. - Every app-shell test file that imports `AppContent` or `PageView` (56 files), under the lock: VERDICT command-exit 0, Test Files 56 passed (56), Tests 358 passed (358). - `pnpm --filter @object-ui/app-shell type-check` (`tsc --noEmit` + `tsc -p tsconfig.test.json`), after building `@object-ui/app-shell^...`: VERDICT command-exit 0, 0 `error TS`. </details> <details><summary>Gates</summary> - `dispatch-gates --commands --repo objectstack-ai/objectstack` derived 34 families. All 34 ran, each with its exit code captured before any pipe, and all 34 exited 0 (`pnpm --dir ui exec vitest run scripts/` ran under the lock: 174 files passed, 2 skipped). `--ran` reconciliation: 34 derived, 34 run, 0 NOT-MEASURED. - `dispatch-gates --tier`: no path-derived mandate. - `pnpm --dir ui run check:new-line-citations`: 0 new citations, exit 0. - Changed lines: 129 (+89 / -40), under the 3000 human-merge threshold. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01JQwxbZK9w6fe2Y6mGQjj6L)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7587fd4 commit c4c515a

5 files changed

Lines changed: 89 additions & 40 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
'@object-ui/console': patch
4+
---
5+
6+
fix(app-shell): the route's object-read failure and the page's record-load failure are announced to assistive technology
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
Two load-failure blocks in the Console drew an `Empty` with no `role`, so a screen reader heard nothing when the read failed: the route slot shown when the by-name read of the route's object gets no answer (`route-object-read-failed`), and the custom page's record-scope read failure (`page-record-load-failed`). Both are now drawn by the shared `DataErrorState` from `@object-ui/components`, which is `role="alert"` by construction, so a failed read is announced and reads differently from an empty one.
12+
13+
Their titles, messages, i18n keys and defaults, test ids and Retry controls are unchanged; Retry rides in the component's `children` so it keeps its own test id. The visible change is the shared component's look: a small destructive icon square above the title.

‎ui/packages/app-shell/src/console/AppContent.tsx‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { Routes, Route, Navigate, matchRoutes, useNavigate, useLocation, usePara
1212
import { useState, useEffect, useCallback, useRef, lazy, Suspense, useMemo, createElement, type ComponentType, type ReactNode } from 'react';
1313
import { useAssistant } from '../assistant/assistantBus.js';
1414
import { ModalForm } from '@object-ui/plugin-form';
15-
import { Empty, EmptyTitle, EmptyDescription, Button } from '@object-ui/components';
15+
import { Empty, EmptyTitle, EmptyDescription, Button, DataErrorState } from '@object-ui/components';
1616
import { toast } from 'sonner';
1717
import { useActionRunner, useGlobalUndo, useMutationInvalidationBridge, notifyDataChanged, FilterScopeProvider, resolveApprovalLock } from '@object-ui/react';
1818
import { useRecordApprovals, openApprovalOf } from '../hooks/useRecordApprovals.js';
@@ -1868,21 +1868,22 @@ function RouteNotFound() {
18681868
*/
18691869
function RouteObjectReadFailed({ onRetry }: { onRetry: () => void }) {
18701870
const { t } = useObjectTranslation();
1871+
// #25915 — drawn by the shared `DataErrorState`, so the failure is
1872+
// `role="alert"` by construction. Retry is passed as `children` (not
1873+
// `onRetry`) so it keeps its own test id.
18711874
return (
18721875
<div className="flex h-full w-full items-center justify-center p-8">
1873-
<Empty data-testid="route-object-read-failed">
1874-
<EmptyTitle>{t('list.loadErrorTitle', { defaultValue: 'Couldn\u2019t load records' })}</EmptyTitle>
1875-
<EmptyDescription>
1876-
{t('list.loadErrorMessage', {
1877-
defaultValue: 'Something went wrong while loading this data. Check your connection and try again.',
1878-
})}
1879-
</EmptyDescription>
1880-
<div className="mt-4">
1881-
<Button onClick={onRetry} data-testid="route-object-read-failed-retry">
1882-
{t('common.retry', { defaultValue: 'Retry' })}
1883-
</Button>
1884-
</div>
1885-
</Empty>
1876+
<DataErrorState
1877+
data-testid="route-object-read-failed"
1878+
title={t('list.loadErrorTitle', { defaultValue: 'Couldn\u2019t load records' })}
1879+
message={t('list.loadErrorMessage', {
1880+
defaultValue: 'Something went wrong while loading this data. Check your connection and try again.',
1881+
})}
1882+
>
1883+
<Button onClick={onRetry} data-testid="route-object-read-failed-retry">
1884+
{t('common.retry', { defaultValue: 'Retry' })}
1885+
</Button>
1886+
</DataErrorState>
18861887
</div>
18871888
);
18881889
}

‎ui/packages/app-shell/src/console/__tests__/AppContent.disabledPackageDeepLink-13052.test.tsx‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,24 @@ describe('a deep link to an object read by name always leaves the splash (object
272272
expect(probeReads()).toBe(2);
273273
});
274274

275+
it('(c) #25915 — the failed-read state is announced (role="alert"), and its Retry still asks again', async () => {
276+
server.next = 'server-error';
277+
boot(DEEP_LINK);
278+
await waitFor(() => expect(screen.getByTestId('route-object-read-failed')).toBeInTheDocument());
279+
const failed = screen.getByTestId('route-object-read-failed');
280+
expect(failed).toHaveAttribute('role', 'alert');
281+
expect(failed).toContainElement(screen.getByTestId('route-object-read-failed-retry'));
282+
await settle();
283+
expect(probeReads()).toBe(1);
284+
285+
server.next = 'served';
286+
fireEvent.click(screen.getByTestId('route-object-read-failed-retry'));
287+
await waitFor(() => expect(screen.getByTestId('object-view')).toHaveTextContent(PROBE));
288+
await settle();
289+
expect(probeReads()).toBe(2);
290+
expect(screen.queryByRole('alert')).not.toBeInTheDocument();
291+
});
292+
275293
it('(d) a 404 carrying a code this build does not know draws the failed state', async () => {
276294
server.next = 'unknown-code';
277295
boot(DEEP_LINK);
@@ -286,5 +304,6 @@ describe('a deep link to an object read by name always leaves the splash (object
286304
await settle(150);
287305
expect(probeReads()).toBe(1);
288306
expect(screen.queryByTestId('route-object-read-failed')).not.toBeInTheDocument();
307+
expect(screen.queryByRole('alert')).not.toBeInTheDocument();
289308
});
290309
});

‎ui/packages/app-shell/src/views/PageView.tsx‎

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ import {
2929
} from '@object-ui/react';
3030
import { isElementDataSourceConfig } from '@object-ui/core';
3131
import type { DataSource, QueryParams } from '@object-ui/types';
32-
import { Button, Empty, EmptyTitle, EmptyDescription, Spinner } from '@object-ui/components';
33-
import { AlertTriangle, FileText, Pencil, RotateCw } from 'lucide-react';
32+
import { Button, DataErrorState, Empty, EmptyTitle, EmptyDescription, Spinner } from '@object-ui/components';
33+
import { FileText, Pencil, RotateCw } from 'lucide-react';
3434
import { useObjectTranslation } from '@object-ui/i18n';
3535
import { MetadataPanel, useMetadataInspector } from './MetadataInspector.js';
3636
import { useMetadata } from '../providers/MetadataProvider.js';
@@ -268,30 +268,25 @@ function AppPageRecordRead({
268268
if (current.status === 'failed') {
269269
return (
270270
<div className="flex h-full items-center justify-center p-4">
271-
<Empty data-testid="page-record-load-failed">
272-
<div className="mx-auto mb-4 flex h-12 w-12 items-center justify-center rounded-full bg-muted">
273-
<AlertTriangle className="h-6 w-6 text-muted-foreground" />
274-
</div>
275-
<EmptyTitle>
276-
{t('empty.recordLoadFailed', { defaultValue: 'Couldn’t load this record' })}
277-
</EmptyTitle>
278-
<EmptyDescription>
279-
{t('empty.recordLoadFailedDescription', {
280-
defaultValue: 'Something went wrong while loading it. Check your connection and try again.',
281-
})}
282-
</EmptyDescription>
283-
<div className="mt-4">
284-
<Button
285-
variant="outline"
286-
size="sm"
287-
data-testid="page-record-load-failed-retry"
288-
onClick={() => setAttempt((n) => n + 1)}
289-
>
290-
<RotateCw className="mr-1.5 h-4 w-4" />
291-
{t('common.retry', { defaultValue: 'Retry' })}
292-
</Button>
293-
</div>
294-
</Empty>
271+
{/* #25915 — the shared `DataErrorState` (`role="alert"` by
272+
construction); Retry rides in `children` to keep its test id. */}
273+
<DataErrorState
274+
data-testid="page-record-load-failed"
275+
title={t('empty.recordLoadFailed', { defaultValue: 'Couldn’t load this record' })}
276+
message={t('empty.recordLoadFailedDescription', {
277+
defaultValue: 'Something went wrong while loading it. Check your connection and try again.',
278+
})}
279+
>
280+
<Button
281+
variant="outline"
282+
size="sm"
283+
data-testid="page-record-load-failed-retry"
284+
onClick={() => setAttempt((n) => n + 1)}
285+
>
286+
<RotateCw className="mr-1.5 h-4 w-4" />
287+
{t('common.retry', { defaultValue: 'Retry' })}
288+
</Button>
289+
</DataErrorState>
295290
</div>
296291
);
297292
}

‎ui/packages/app-shell/src/views/__tests__/PageView.appPageRecord-12931.test.tsx‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,4 +337,25 @@ describe('objectui#12931 — the controls', () => {
337337
expect(shown('New environment')).toBe(true);
338338
expect(shown('Production')).toBe(true);
339339
});
340+
341+
it('#25915 — the failed read is announced (role="alert"); Retry re-reads; the loaded row renders no alert', async () => {
342+
let fail = true;
343+
const ds = mount(envPage(), async () => {
344+
if (fail) throw new Error('upstream exploded');
345+
return { data: [{ id: 'o1', plan: 'team', env_count: 2 }] };
346+
});
347+
await settle();
348+
const failed = screen.getByTestId('page-record-load-failed');
349+
expect(failed).toHaveAttribute('role', 'alert');
350+
expect(failed).toContainElement(screen.getByTestId('page-record-load-failed-retry'));
351+
352+
fail = false;
353+
await act(async () => {
354+
fireEvent.click(screen.getByTestId('page-record-load-failed-retry'));
355+
});
356+
await settle();
357+
expect(orgReads(ds), 'Retry is one more read of the same row').toHaveLength(2);
358+
expect(shown('New environment')).toBe(true);
359+
expect(screen.queryByRole('alert')).toBeNull();
360+
});
340361
});

0 commit comments

Comments
 (0)