Skip to content

Commit e8bec83

Browse files
committed
feat(app-shell): wire navigation action items to the console action runtime (framework#4509)
A `type: 'action'` nav item rendered, gated like any other item, and did nothing when clicked. NavigationRenderer dispatches such a click to an `onAction` prop it expects the host to supply — it deliberately never reads `item.actionDef` itself — and no shipped sidebar supplied it. So `actionDef.actionName` reached no dispatcher: an author could put an action in the menu, watch it render with icon and label, and never learn that clicking it was a no-op. The framework's liveness ledger recorded this as the single gap in the AppSchema navigation surface. `useNavActionDispatch` resolves the nav item's actionName against `action` metadata at click time — the same source DeclaredActionsBar reads for a record toolbar — and dispatches the resolved def through useAction(). UnifiedSidebar passes it. No new provider: the sidebar already renders inside ConsoleShell's GlobalActionRuntimeProvider, so nav actions get the fully-wired console runner with its confirm/param/result/navigate dialogs. A declared `params` array becomes the runner's param-dialog input; the nav item's own `actionDef.params` rides as the value bag, so a menu entry can pre-fill the action it launches. Nav actions are inherently global — ActionNavItemSchema is strict with exactly { actionName, params? } and carries no objectName — so resolution is by name and no record context rides along. Behaviour change: a shell passing no `onAction` no longer renders action items at all, rather than rendering them dead. This mirrors the existing capability guards and makes the omission diagnosable — a missing prop now reads as "my item is gone", which leads to the prop, instead of "clicking does nothing", which for three releases led nowhere. Every dispatch-time failure warns and toasts instead of returning silently, since silence is the bug being fixed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E5CYr5SDwe85gH2Jr5KSgu
1 parent b67be19 commit e8bec83

8 files changed

Lines changed: 386 additions & 1 deletion

File tree

.changeset/nav-action-dispatch.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
"@object-ui/layout": minor
3+
"@object-ui/app-shell": minor
4+
---
5+
6+
Navigation `action` items actually run now (framework#4509).
7+
8+
A `type: 'action'` nav item rendered, gated like any other item, and did
9+
**nothing** when clicked. `NavigationRenderer` dispatches such a click to an
10+
`onAction` prop it expects the host shell to supply — it deliberately never
11+
reads `item.actionDef` itself — and no shipped sidebar supplied that prop. So
12+
`actionDef.actionName` reached no dispatcher: an author could put an action in
13+
the menu, watch it render with its icon and label, and never find out that
14+
clicking it was a no-op. The framework's liveness ledger recorded this as the
15+
single gap in the AppSchema navigation surface.
16+
17+
**New `useNavActionDispatch`** (`@object-ui/app-shell`) resolves the nav item's
18+
`actionName` against `action` metadata at click time — the same source
19+
`DeclaredActionsBar` reads for a record toolbar — and dispatches the resolved
20+
definition through `useAction()`. `UnifiedSidebar` now passes it. No new
21+
provider is involved: the sidebar already renders inside `ConsoleShell`'s
22+
`GlobalActionRuntimeProvider`, so nav actions get the fully-wired console runner
23+
including the confirm, param-collection, result and navigate dialogs. A declared
24+
`params` array becomes the runner's param-dialog input, and the nav item's own
25+
`actionDef.params` is passed as the value bag, so a menu entry can pre-fill the
26+
action it launches.
27+
28+
Nav actions are inherently **global**: `ActionNavItemSchema` is strict with
29+
exactly `{ actionName, params? }` and carries no `objectName`, so resolution is
30+
by name alone and no record context rides along.
31+
32+
**Behaviour change:** a shell that passes no `onAction` no longer renders
33+
`action` items at all, instead of rendering them dead. This mirrors the existing
34+
capability guards — an item the host cannot serve is hidden — and it makes the
35+
omission diagnosable: a missing prop now shows up as "my action item is gone",
36+
which leads to the prop, rather than "clicking does nothing", which for three
37+
releases led nowhere. Every failure at dispatch time (an unnamed item, an
38+
unresolvable action, a throwing action) warns and toasts instead of returning
39+
silently.
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* useNavActionDispatch — nav `action` items reach the action runtime
11+
* (framework#4509).
12+
*
13+
* The contract has three parts, and the third is the one the bug was made of:
14+
* resolve `actionDef.actionName` against `action` metadata, dispatch the
15+
* resolved def through the runner, and FAIL LOUDLY when either step comes up
16+
* empty — a silent return would reproduce the dead click through a new route.
17+
*/
18+
19+
import { describe, it, expect, vi, beforeEach } from 'vitest';
20+
import React from 'react';
21+
import { renderHook, act } from '@testing-library/react';
22+
23+
const execute = vi.fn();
24+
const getItem = vi.fn();
25+
const toastError = vi.fn();
26+
27+
vi.mock('sonner', () => ({ toast: { error: (...a: unknown[]) => toastError(...a) } }));
28+
vi.mock('@object-ui/react', () => ({ useAction: () => ({ execute }) }));
29+
vi.mock('../../providers/MetadataProvider', () => ({ useMetadata: () => ({ getItem }) }));
30+
31+
import { useNavActionDispatch } from '../useNavActionDispatch';
32+
33+
const navItem = (over: Record<string, unknown> = {}) => ({
34+
id: 'nav_export',
35+
type: 'action',
36+
label: 'Export Data',
37+
actionDef: { actionName: 'export_data' },
38+
...over,
39+
}) as never;
40+
41+
async function dispatch(item: unknown) {
42+
const { result } = renderHook(() => useNavActionDispatch());
43+
await act(async () => {
44+
result.current(item as never);
45+
// The handler is fire-and-forget by design (the renderer's onClick is
46+
// synchronous); let its promise chain settle.
47+
await Promise.resolve();
48+
await Promise.resolve();
49+
await Promise.resolve();
50+
});
51+
}
52+
53+
beforeEach(() => {
54+
execute.mockReset().mockResolvedValue(undefined);
55+
getItem.mockReset();
56+
toastError.mockReset();
57+
vi.spyOn(console, 'warn').mockImplementation(() => {});
58+
});
59+
60+
describe('useNavActionDispatch', () => {
61+
it('resolves the action by name and dispatches the resolved definition', async () => {
62+
getItem.mockResolvedValue({ name: 'export_data', type: 'api', target: '/exports', label: 'Export' });
63+
64+
await dispatch(navItem());
65+
66+
expect(getItem).toHaveBeenCalledWith('action', 'export_data');
67+
expect(execute).toHaveBeenCalledTimes(1);
68+
const [dispatched] = execute.mock.calls[0];
69+
// The whole def rides the dispatch — the runner reads type/target/label.
70+
expect(dispatched).toMatchObject({ name: 'export_data', type: 'api', target: '/exports' });
71+
});
72+
73+
it("moves a declared `params` ARRAY to `actionParams` (the runner's dialog input)", async () => {
74+
getItem.mockResolvedValue({
75+
name: 'export_data',
76+
type: 'api',
77+
params: [{ name: 'format', type: 'text' }],
78+
});
79+
80+
await dispatch(navItem());
81+
82+
const [dispatched] = execute.mock.calls[0];
83+
expect(dispatched.actionParams).toEqual([{ name: 'format', type: 'text' }]);
84+
// `params` on the dispatch is the VALUE bag, not the param declarations —
85+
// leaving the array there would make the runner treat it as values.
86+
expect(Array.isArray(dispatched.params)).toBe(false);
87+
});
88+
89+
it("passes the nav item's own actionDef.params through as the value bag", async () => {
90+
getItem.mockResolvedValue({ name: 'export_data', type: 'api' });
91+
92+
await dispatch(navItem({ actionDef: { actionName: 'export_data', params: { format: 'csv' } } }));
93+
94+
const [dispatched] = execute.mock.calls[0];
95+
expect(dispatched.params).toEqual({ format: 'csv' });
96+
});
97+
98+
it('warns and toasts — without dispatching — when the action is not defined', async () => {
99+
getItem.mockResolvedValue(null);
100+
101+
await dispatch(navItem());
102+
103+
expect(execute).not.toHaveBeenCalled();
104+
expect(toastError).toHaveBeenCalledTimes(1);
105+
expect(String(toastError.mock.calls[0][0])).toContain('export_data');
106+
});
107+
108+
it('warns and toasts when the nav item names no action at all', async () => {
109+
await dispatch(navItem({ actionDef: undefined }));
110+
111+
expect(getItem).not.toHaveBeenCalled();
112+
expect(execute).not.toHaveBeenCalled();
113+
expect(toastError).toHaveBeenCalledTimes(1);
114+
});
115+
116+
it('surfaces a resolution failure instead of swallowing it', async () => {
117+
getItem.mockRejectedValue(new Error('offline'));
118+
119+
await dispatch(navItem());
120+
121+
expect(execute).not.toHaveBeenCalled();
122+
expect(toastError).toHaveBeenCalledTimes(1);
123+
});
124+
125+
it('surfaces a failure thrown by the action itself', async () => {
126+
getItem.mockResolvedValue({ name: 'export_data', type: 'api' });
127+
execute.mockRejectedValue(new Error('boom'));
128+
129+
await dispatch(navItem());
130+
131+
expect(toastError).toHaveBeenCalledTimes(1);
132+
});
133+
});

packages/app-shell/src/hooks/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
export { useFavorites, type FavoriteItem } from './useFavorites';
22
export { useActionModal, type ModalDescriptor } from './useActionModal';
33
export { useMetadataService } from './useMetadataService';
4+
export { useNavActionDispatch } from './useNavActionDispatch';
45
export { useNavPins } from './useNavPins';
56
export {
67
useNavigationSync,
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* useNavActionDispatch — make `type: 'action'` navigation items actually run.
5+
*
6+
* ## The disconnect this closes (framework#4509)
7+
* `NavigationRenderer` renders an `action` nav item, gates it like any other
8+
* (permissions, capabilities, visibility), and dispatches the click to an
9+
* `onAction` prop it expects the HOST to supply — it never reads
10+
* `item.actionDef` itself. No shipped shell supplied that prop, so
11+
* `actionDef.actionName` reached no dispatcher: the item rendered, looked
12+
* enabled, and did nothing on click. The framework liveness ledger recorded it
13+
* as the one gap in the AppSchema navigation walk.
14+
*
15+
* ## Resolution
16+
* A nav item names an action (`actionDef.actionName`); it does not carry the
17+
* definition. `ActionEngine.executeAction` only knows names that were
18+
* registered with it, which a sidebar has no way to arrange — so the name is
19+
* resolved against `action` metadata at click time, the same source
20+
* `DeclaredActionsBar` reads for a record's toolbar.
21+
*
22+
* Note the scope: `ActionNavItemSchema` is `.strict()` with exactly
23+
* `{ actionName, params? }` — there is no `objectName`, so a nav action is
24+
* inherently a GLOBAL action (ADR-0110's `'*'` scope). Resolution is by name
25+
* alone, and an object-scoped action is not addressable from a nav item.
26+
*
27+
* Dispatch goes through `useAction()`, which under `GlobalActionRuntimeProvider`
28+
* (ConsoleShell) is the fully-wired console runner — api/flow/script handlers
29+
* plus the confirm, param-collection, result and navigate dialogs. The sidebar
30+
* already renders inside that provider, so no new wiring is needed.
31+
*
32+
* ## Failure is loud
33+
* Every failure path warns and toasts rather than returning silently. Silence
34+
* is precisely the bug being fixed: a nav item that names an action nobody
35+
* defined should say so, not reproduce the dead click through a different
36+
* route.
37+
*/
38+
39+
import { useCallback } from 'react';
40+
import { toast } from 'sonner';
41+
import { useAction } from '@object-ui/react';
42+
import type { NavigationItem } from '@object-ui/types';
43+
import { useMetadata } from '../providers/MetadataProvider';
44+
45+
type ActionDefLike = Record<string, unknown> & { name?: string; params?: unknown };
46+
47+
export function useNavActionDispatch(): (item: NavigationItem) => void {
48+
const { execute } = useAction();
49+
const { getItem } = useMetadata();
50+
51+
return useCallback((item: NavigationItem) => {
52+
void (async () => {
53+
const actionDef = (item as { actionDef?: { actionName?: string; params?: Record<string, unknown> } }).actionDef;
54+
const actionName = actionDef?.actionName;
55+
56+
// The spec requires `actionDef` on this variant, but objectui's own
57+
// NavigationItem type keeps it optional (the two shapes are reconciled by
58+
// the navigation-spec-parity test, not by the compiler), so a nav item
59+
// can reach here naming nothing.
60+
if (!actionName) {
61+
console.warn('[nav] action item has no actionDef.actionName — nothing to dispatch', item?.id);
62+
toast.error('This menu item is not configured to run an action.');
63+
return;
64+
}
65+
66+
let def: ActionDefLike | null = null;
67+
try {
68+
def = (await getItem('action', actionName)) as ActionDefLike | null;
69+
} catch (err) {
70+
console.warn(`[nav] failed to resolve action '${actionName}'`, err);
71+
toast.error(`Could not load action "${actionName}".`);
72+
return;
73+
}
74+
75+
if (!def) {
76+
console.warn(`[nav] action '${actionName}' is not defined — nav item ${item?.id} cannot run`);
77+
toast.error(`Action "${actionName}" is not defined.`);
78+
return;
79+
}
80+
81+
try {
82+
// Same dispatch shape as DeclaredActionsBar: forward the whole def
83+
// (type/target/confirmText/successMessage/…), and move a `params`
84+
// ARRAY out of the way into `actionParams`, which is the runner's
85+
// param-dialog input. `params` on the dispatch is the value bag, so the
86+
// nav item's own `actionDef.params` lands there — that is how an author
87+
// pre-fills an action from the menu entry.
88+
const { params: declaredParams, ...rest } = def;
89+
const dispatch: Record<string, unknown> = { ...rest };
90+
const staticParams = Array.isArray(declaredParams) ? declaredParams : [];
91+
if (staticParams.length > 0) dispatch.actionParams = staticParams;
92+
if (actionDef?.params && typeof actionDef.params === 'object') {
93+
dispatch.params = { ...(actionDef.params as Record<string, unknown>) };
94+
}
95+
// No `_rowRecord`: a sidebar click carries no record context. An action
96+
// that interpolates `{id}` is object-scoped and was never addressable
97+
// from a nav item anyway.
98+
await execute(dispatch as never);
99+
} catch (err) {
100+
console.warn(`[nav] action '${actionName}' failed`, err);
101+
toast.error(`Action "${actionName}" failed to run.`);
102+
}
103+
})();
104+
}, [execute, getItem]);
105+
}

packages/app-shell/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ export type {
151151
export {
152152
useFavorites,
153153
useMetadataService,
154+
useNavActionDispatch,
154155
useNavPins,
155156
useNavigationSync,
156157
NavigationSyncEffect,

packages/app-shell/src/layout/UnifiedSidebar.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import { useAuth, useIsWorkspaceAdmin } from '@object-ui/auth';
4747
import { useRecentItems } from '../hooks/useRecentItems';
4848
import { useFavorites } from '../hooks/useFavorites';
4949
import { useNavPins } from '../hooks/useNavPins';
50+
import { useNavActionDispatch } from '../hooks/useNavActionDispatch';
5051
import { resolveI18nLabel, matchAppBySegment, appRouteSegment } from '../utils';
5152
import { useObjectTranslation, useObjectLabel } from '@object-ui/i18n';
5253
// useObjectLabel provides appLabel/appDescription for convention-based
@@ -152,6 +153,11 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) {
152153
const { context, currentAppName } = useNavigationContext();
153154
const { user, activeOrganization } = useAuth();
154155
const isWorkspaceAdmin = useIsWorkspaceAdmin();
156+
// `type: 'action'` nav items dispatch through here (framework#4509). The
157+
// sidebar renders inside ConsoleShell's GlobalActionRuntimeProvider, so this
158+
// resolves to the fully-wired console runner — confirm/param/result dialogs
159+
// included — with no provider of its own.
160+
const dispatchNavAction = useNavActionDispatch();
155161

156162
// Swipe-from-left-edge gesture to open sidebar on mobile
157163
React.useEffect(() => {
@@ -464,6 +470,7 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) {
464470
: resolveNavGroupLabel(activeApp.name, itemId, fallback)
465471
) : undefined}
466472
resolveViewLabel={(objectName, viewName, fallback) => resolveNavViewLabel(objectName, viewName, fallback)}
473+
onAction={dispatchNavAction}
467474
t={t}
468475
templateContext={{ currentUserId: user?.id ?? null, currentOrgId: activeOrganization?.id ?? null, contextValues }}
469476
/>

packages/layout/src/NavigationRenderer.tsx

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,19 @@ export interface NavigationRendererProps {
122122
/** Optional runtime-capability checker for `requiresObject` / `requiresService` */
123123
checkCapability?: CapabilityChecker;
124124

125-
/** Called when an `action`-type item is clicked */
125+
/**
126+
* Called when an `action`-type item is clicked.
127+
*
128+
* A shell that renders `action` items MUST supply this: the renderer has no
129+
* dispatcher of its own and never reads `item.actionDef` — unpacking
130+
* `actionName` / `params` and invoking the action is the host's job (see
131+
* `useNavActionDispatch` in `@objectstack/app-shell`). Omitting it does not
132+
* degrade to an inert button; action items are **not rendered at all**,
133+
* because a nav entry that looks clickable and silently does nothing is worse
134+
* than an absent one (framework#4509 — every shipped sidebar omitted this
135+
* prop, so `actionDef.actionName` reached no dispatcher and every such item
136+
* dead-clicked).
137+
*/
126138
onAction?: (item: NavigationItem) => void;
127139

128140
// --- P1.7 Navigation Enhancements ---
@@ -958,6 +970,13 @@ function NavigationItemRenderer({
958970

959971
// --- Action ---
960972
if (item.type === 'action') {
973+
// No dispatcher, no button (framework#4509). This mirrors the capability
974+
// guards above: an item the host cannot actually serve is hidden, not
975+
// rendered dead. It also makes the omission visible to whoever adds a new
976+
// shell — a missing `onAction` shows up as "my action item vanished",
977+
// which leads to the prop, instead of "clicking does nothing", which for
978+
// three releases led nowhere.
979+
if (!onAction) return null;
961980
const Icon = resolveIcon(item.icon);
962981
const actionLabel = resolveLabel(item.label, tProp);
963982
return (

0 commit comments

Comments
 (0)