Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions .changeset/nav-action-dispatch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
---
"@object-ui/layout": minor
"@object-ui/app-shell": minor
---

Navigation `action` items actually run now (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 shell to supply — it deliberately never
reads `item.actionDef` itself — and no shipped sidebar supplied that prop. So
`actionDef.actionName` reached no dispatcher: an author could put an action in
the menu, watch it render with its icon and label, and never find out that
clicking it was a no-op. The framework's liveness ledger recorded this as the
single gap in the AppSchema navigation surface.

**New `useNavActionDispatch`** (`@object-ui/app-shell`) 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
definition through `useAction()`. `UnifiedSidebar` now passes it. No new
provider is involved: the sidebar already renders inside `ConsoleShell`'s
`GlobalActionRuntimeProvider`, so nav actions get the fully-wired console runner
including the confirm, param-collection, result and navigate dialogs. A declared
`params` array becomes the runner's param-dialog input, and the nav item's own
`actionDef.params` is passed 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 alone and no record context rides along.

**Behaviour change:** a shell that passes no `onAction` no longer renders
`action` items at all, instead of rendering them dead. This mirrors the existing
capability guards — an item the host cannot serve is hidden — and it makes the
omission diagnosable: a missing prop now shows up as "my action item is gone",
which leads to the prop, rather than "clicking does nothing", which for three
releases led nowhere. Every failure at dispatch time (an unnamed item, an
unresolvable action, a throwing action) warns and toasts instead of returning
silently.
132 changes: 132 additions & 0 deletions packages/app-shell/src/hooks/__tests__/useNavActionDispatch.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* useNavActionDispatch — nav `action` items reach the action runtime
* (framework#4509).
*
* The contract has three parts, and the third is the one the bug was made of:
* resolve `actionDef.actionName` against `action` metadata, dispatch the
* resolved def through the runner, and FAIL LOUDLY when either step comes up
* empty — a silent return would reproduce the dead click through a new route.
*/

import { describe, it, expect, vi, beforeEach } from 'vitest';
import { renderHook, act } from '@testing-library/react';

const execute = vi.fn();
const getItem = vi.fn();
const toastError = vi.fn();

vi.mock('sonner', () => ({ toast: { error: (...a: unknown[]) => toastError(...a) } }));
vi.mock('@object-ui/react', () => ({ useAction: () => ({ execute }) }));
vi.mock('../../providers/MetadataProvider', () => ({ useMetadata: () => ({ getItem }) }));

import { useNavActionDispatch } from '../useNavActionDispatch';

const navItem = (over: Record<string, unknown> = {}) => ({
id: 'nav_export',
type: 'action',
label: 'Export Data',
actionDef: { actionName: 'export_data' },
...over,
}) as never;

async function dispatch(item: unknown) {
const { result } = renderHook(() => useNavActionDispatch());
await act(async () => {
result.current(item as never);
// The handler is fire-and-forget by design (the renderer's onClick is
// synchronous); let its promise chain settle.
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
});
}

beforeEach(() => {
execute.mockReset().mockResolvedValue(undefined);
getItem.mockReset();
toastError.mockReset();
vi.spyOn(console, 'warn').mockImplementation(() => {});
});

describe('useNavActionDispatch', () => {
it('resolves the action by name and dispatches the resolved definition', async () => {
getItem.mockResolvedValue({ name: 'export_data', type: 'api', target: '/exports', label: 'Export' });

await dispatch(navItem());

expect(getItem).toHaveBeenCalledWith('action', 'export_data');
expect(execute).toHaveBeenCalledTimes(1);
const [dispatched] = execute.mock.calls[0];
// The whole def rides the dispatch — the runner reads type/target/label.
expect(dispatched).toMatchObject({ name: 'export_data', type: 'api', target: '/exports' });
});

it("moves a declared `params` ARRAY to `actionParams` (the runner's dialog input)", async () => {
getItem.mockResolvedValue({
name: 'export_data',
type: 'api',
params: [{ name: 'format', type: 'text' }],
});

await dispatch(navItem());

const [dispatched] = execute.mock.calls[0];
expect(dispatched.actionParams).toEqual([{ name: 'format', type: 'text' }]);
// `params` on the dispatch is the VALUE bag, not the param declarations —
// leaving the array there would make the runner treat it as values.
expect(Array.isArray(dispatched.params)).toBe(false);
});

it("passes the nav item's own actionDef.params through as the value bag", async () => {
getItem.mockResolvedValue({ name: 'export_data', type: 'api' });

await dispatch(navItem({ actionDef: { actionName: 'export_data', params: { format: 'csv' } } }));

const [dispatched] = execute.mock.calls[0];
expect(dispatched.params).toEqual({ format: 'csv' });
});

it('warns and toasts — without dispatching — when the action is not defined', async () => {
getItem.mockResolvedValue(null);

await dispatch(navItem());

expect(execute).not.toHaveBeenCalled();
expect(toastError).toHaveBeenCalledTimes(1);
expect(String(toastError.mock.calls[0][0])).toContain('export_data');
});

it('warns and toasts when the nav item names no action at all', async () => {
await dispatch(navItem({ actionDef: undefined }));

expect(getItem).not.toHaveBeenCalled();
expect(execute).not.toHaveBeenCalled();
expect(toastError).toHaveBeenCalledTimes(1);
});

it('surfaces a resolution failure instead of swallowing it', async () => {
getItem.mockRejectedValue(new Error('offline'));

await dispatch(navItem());

expect(execute).not.toHaveBeenCalled();
expect(toastError).toHaveBeenCalledTimes(1);
});

it('surfaces a failure thrown by the action itself', async () => {
getItem.mockResolvedValue({ name: 'export_data', type: 'api' });
execute.mockRejectedValue(new Error('boom'));

await dispatch(navItem());

expect(toastError).toHaveBeenCalledTimes(1);
});
});
1 change: 1 addition & 0 deletions packages/app-shell/src/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export { useFavorites, type FavoriteItem } from './useFavorites';
export { useActionModal, type ModalDescriptor } from './useActionModal';
export { useMetadataService } from './useMetadataService';
export { useNavActionDispatch } from './useNavActionDispatch';
export { useNavPins } from './useNavPins';
export {
useNavigationSync,
Expand Down
105 changes: 105 additions & 0 deletions packages/app-shell/src/hooks/useNavActionDispatch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* useNavActionDispatch — make `type: 'action'` navigation items actually run.
*
* ## The disconnect this closes (framework#4509)
* `NavigationRenderer` renders an `action` nav item, gates it like any other
* (permissions, capabilities, visibility), and dispatches the click to an
* `onAction` prop it expects the HOST to supply — it never reads
* `item.actionDef` itself. No shipped shell supplied that prop, so
* `actionDef.actionName` reached no dispatcher: the item rendered, looked
* enabled, and did nothing on click. The framework liveness ledger recorded it
* as the one gap in the AppSchema navigation walk.
*
* ## Resolution
* A nav item names an action (`actionDef.actionName`); it does not carry the
* definition. `ActionEngine.executeAction` only knows names that were
* registered with it, which a sidebar has no way to arrange — so the name is
* resolved against `action` metadata at click time, the same source
* `DeclaredActionsBar` reads for a record's toolbar.
*
* Note the scope: `ActionNavItemSchema` is `.strict()` with exactly
* `{ actionName, params? }` — there is no `objectName`, so a nav action is
* inherently a GLOBAL action (ADR-0110's `'*'` scope). Resolution is by name
* alone, and an object-scoped action is not addressable from a nav item.
*
* Dispatch goes through `useAction()`, which under `GlobalActionRuntimeProvider`
* (ConsoleShell) is the fully-wired console runner — api/flow/script handlers
* plus the confirm, param-collection, result and navigate dialogs. The sidebar
* already renders inside that provider, so no new wiring is needed.
*
* ## Failure is loud
* Every failure path warns and toasts rather than returning silently. Silence
* is precisely the bug being fixed: a nav item that names an action nobody
* defined should say so, not reproduce the dead click through a different
* route.
*/

import { useCallback } from 'react';
import { toast } from 'sonner';
import { useAction } from '@object-ui/react';
import type { NavigationItem } from '@object-ui/types';
import { useMetadata } from '../providers/MetadataProvider';

type ActionDefLike = Record<string, unknown> & { name?: string; params?: unknown };

export function useNavActionDispatch(): (item: NavigationItem) => void {
const { execute } = useAction();
const { getItem } = useMetadata();

return useCallback((item: NavigationItem) => {
void (async () => {
const actionDef = (item as { actionDef?: { actionName?: string; params?: Record<string, unknown> } }).actionDef;
const actionName = actionDef?.actionName;

// The spec requires `actionDef` on this variant, but objectui's own
// NavigationItem type keeps it optional (the two shapes are reconciled by
// the navigation-spec-parity test, not by the compiler), so a nav item
// can reach here naming nothing.
if (!actionName) {
console.warn('[nav] action item has no actionDef.actionName — nothing to dispatch', item?.id);
toast.error('This menu item is not configured to run an action.');
return;
}

let def: ActionDefLike | null;
try {
def = (await getItem('action', actionName)) as ActionDefLike | null;
} catch (err) {
console.warn(`[nav] failed to resolve action '${actionName}'`, err);
toast.error(`Could not load action "${actionName}".`);
return;
}

if (!def) {
console.warn(`[nav] action '${actionName}' is not defined — nav item ${item?.id} cannot run`);
toast.error(`Action "${actionName}" is not defined.`);
return;
}

try {
// Same dispatch shape as DeclaredActionsBar: forward the whole def
// (type/target/confirmText/successMessage/…), and move a `params`
// ARRAY out of the way into `actionParams`, which is the runner's
// param-dialog input. `params` on the dispatch is the value bag, so the
// nav item's own `actionDef.params` lands there — that is how an author
// pre-fills an action from the menu entry.
const { params: declaredParams, ...rest } = def;
const dispatch: Record<string, unknown> = { ...rest };
const staticParams = Array.isArray(declaredParams) ? declaredParams : [];
if (staticParams.length > 0) dispatch.actionParams = staticParams;
if (actionDef?.params && typeof actionDef.params === 'object') {
dispatch.params = { ...(actionDef.params as Record<string, unknown>) };
}
// No `_rowRecord`: a sidebar click carries no record context. An action
// that interpolates `{id}` is object-scoped and was never addressable
// from a nav item anyway.
await execute(dispatch as never);
} catch (err) {
console.warn(`[nav] action '${actionName}' failed`, err);
toast.error(`Action "${actionName}" failed to run.`);
}
})();
}, [execute, getItem]);
}
1 change: 1 addition & 0 deletions packages/app-shell/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ export type {
export {
useFavorites,
useMetadataService,
useNavActionDispatch,
useNavPins,
useNavigationSync,
NavigationSyncEffect,
Expand Down
7 changes: 7 additions & 0 deletions packages/app-shell/src/layout/UnifiedSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import { useAuth, useIsWorkspaceAdmin } from '@object-ui/auth';
import { useRecentItems } from '../hooks/useRecentItems';
import { useFavorites } from '../hooks/useFavorites';
import { useNavPins } from '../hooks/useNavPins';
import { useNavActionDispatch } from '../hooks/useNavActionDispatch';
import { resolveI18nLabel, matchAppBySegment, appRouteSegment } from '../utils';
import { useObjectTranslation, useObjectLabel } from '@object-ui/i18n';
// useObjectLabel provides appLabel/appDescription for convention-based
Expand Down Expand Up @@ -152,6 +153,11 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) {
const { context, currentAppName } = useNavigationContext();
const { user, activeOrganization } = useAuth();
const isWorkspaceAdmin = useIsWorkspaceAdmin();
// `type: 'action'` nav items dispatch through here (framework#4509). The
// sidebar renders inside ConsoleShell's GlobalActionRuntimeProvider, so this
// resolves to the fully-wired console runner — confirm/param/result dialogs
// included — with no provider of its own.
const dispatchNavAction = useNavActionDispatch();

// Swipe-from-left-edge gesture to open sidebar on mobile
React.useEffect(() => {
Expand Down Expand Up @@ -464,6 +470,7 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) {
: resolveNavGroupLabel(activeApp.name, itemId, fallback)
) : undefined}
resolveViewLabel={(objectName, viewName, fallback) => resolveNavViewLabel(objectName, viewName, fallback)}
onAction={dispatchNavAction}
t={t}
templateContext={{ currentUserId: user?.id ?? null, currentOrgId: activeOrganization?.id ?? null, contextValues }}
/>
Expand Down
21 changes: 20 additions & 1 deletion packages/layout/src/NavigationRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,19 @@ export interface NavigationRendererProps {
/** Optional runtime-capability checker for `requiresObject` / `requiresService` */
checkCapability?: CapabilityChecker;

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

// --- P1.7 Navigation Enhancements ---
Expand Down Expand Up @@ -958,6 +970,13 @@ function NavigationItemRenderer({

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