fix(components): page:header reads the spec's variant 'danger' as destructive (objectui#12764)

.changeset/12764-header-danger-variant.md

@@ -0,0 +1,15 @@
+---
+'@object-ui/components': patch
+---
+
+fix(components): `page:header` reads the spec's `variant: 'danger'` as destructive (objectui#12764)
+
+Clause-②: no
+
+`@objectstack/spec`'s action `variant` enum is `primary | secondary | danger | ghost | link`, and its description says "danger = destructive". `page:header` read destructive only as Shadcn's `destructive` or the host's `sys_delete`, so a `variant: 'danger'` action (an organization page's "Leave organization") placed first drew as the filled primary button, and folded into the `⋯` it had no red row and no separator. One predicate now decides destructive for the whole header, and its word is the spec's `danger`:
+
+- a `danger` action is never the filled primary; the primary is the first non-destructive action, or the one declaring `variant: 'primary'`;
+- shown inline, it is drawn as Shadcn's `destructive` button instead of an outline button;
+- folded into the `⋯`, it is a red row in the destructive band, below the separator, beside the host's Delete.
+
+`variant: 'destructive'` on a header action is no longer read as destructive: it is not a spec value, and the spec's action schema refuses it. The host's Delete is matched by its name, `sys_delete`, so it keeps its red row below the separator. No prop, export or type member changes.

content/docs/guide/slotted-pages.md

@@ -280,6 +280,11 @@ same rules as `action:bar`:
 - **`variant: 'primary'`** — tie-break within equal `order`: a primary
   action outranks its unordered siblings without needing an explicit
   `order`.
+- **`variant: 'danger'`** — the spec's destructive variant. A danger
+  action is never the filled primary button: shown inline it is a
+  destructive (red) button, and folded into the `⋯` menu it is a red row
+  in the destructive band at the bottom, below a separator, beside the
+  host's Delete.
 - **`component: 'action:menu'`** — pins the action inside the `⋯` menu
   regardless of how few actions exist (the synthesized Share / Delete
   use this).

packages/components/src/__tests__/page-header-danger-12764.test.tsx

@@ -0,0 +1,168 @@
+/**
+ * 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.
+ */
+
+/**
+ * objectui#12764: `page:header` reads the spec's `variant: 'danger'` as
+ * destructive.
+ *
+ * `@objectstack/spec`'s action `variant` enum is `primary | secondary | danger
+ * | ghost | link` ("danger = destructive"). The header read destructive only
+ * as Shadcn's `destructive` or the host's `sys_delete`, so a `danger` action
+ * (a "Leave organization") placed first drew as the filled primary, had no
+ * red row and no separator in the `⋯`. Now:
+ *
+ *   - a `danger` action is never the filled primary;
+ *   - shown inline, it draws Shadcn's `destructive` button;
+ *   - folded into the `⋯`, it is a red row in the destructive band, below a
+ *     separator;
+ *   - CONTROL: the host's `sys_delete` and the non-destructive actions keep
+ *     their placement and style.
+ *
+ * Styles are read off the Button primitive's own variant classes
+ * (`bg-primary` for `default`, `bg-destructive` for `destructive`,
+ * `border-input` for `outline`).
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { render, screen, fireEvent, waitFor } from '@testing-library/react';
+import { ComponentRegistry } from '@object-ui/core';
+import { ActionProvider, RecordContextProvider } from '@object-ui/react';
+// Registers `page:header` at module scope rather than in a hook
+// (object-ui/no-dynamic-import-in-test-hook, objectui#3010).
+import '../renderers';
+
+function PageHeader({ schema }: { schema: any }) {
+  const Component = ComponentRegistry.get('page:header');
+  if (!Component) throw new Error('page:header not registered');
+  // eslint-disable-next-line react-hooks/static-components -- ComponentRegistry.get returns a registered component (stable), not one created during render
+  return <Component schema={schema} />;
+}
+
+const ORG = { id: 'org-1', name: 'Acme' };
+
+// The record page's host actions, as `RecordDetailView` injects them.
+const HOST_ACTIONS = [
+  { name: 'sys_edit', label: 'Edit', icon: 'pencil', order: 100, onClick: vi.fn() },
+  { name: 'sys_share', label: 'Share', order: 110, component: 'action:menu', onClick: vi.fn() },
+  { name: 'sys_delete', label: 'Delete', order: 120, variant: 'destructive', component: 'action:menu', onClick: vi.fn() },
+];
+
+function renderHeader(actions: any[], extra: Record<string, unknown> = {}) {
+  return render(
+    <ActionProvider>
+      <RecordContextProvider
+        objectName="sys_organization"
+        recordId={ORG.id}
+        data={ORG}
+        objectSchema={{ name: 'sys_organization', label: 'Organization' }}
+        headerSystemActions={HOST_ACTIONS}
+      >
+        <PageHeader schema={{ type: 'page:header', actions, ...extra }} />
+      </RecordContextProvider>
+    </ActionProvider>,
+  );
+}
+
+const act = (name: string, label: string, extra: Record<string, unknown> = {}) => ({
+  name,
+  label,
+  type: 'url',
+  target: `https://example.com/${name}`,
+  locations: ['record_header'],
+  ...extra,
+});
+
+function rowButtons(): HTMLElement[] {
+  const bar = screen.getByRole('toolbar', { name: /Page header actions/i });
+  return Array.from(bar.querySelectorAll('button')).filter(
+    (b) => b.getAttribute('aria-label') !== 'More actions',
+  );
+}
+
+const nameOf = (b: HTMLElement) => (b.textContent ?? '').trim();
+const hasClass = (el: Element, c: string) => el.className.split(/\s+/).includes(c);
+const isFilled = (b: HTMLElement) => hasClass(b, 'bg-primary');
+const isDestructiveButton = (b: HTMLElement) => hasClass(b, 'bg-destructive');
+const isOutline = (b: HTMLElement) => hasClass(b, 'border-input');
+
+/** The open `⋯` menu as an ordered list of rows and separators. */
+async function overflowEntries(): Promise<Array<{ label: string; red: boolean } | '---'>> {
+  const trigger = screen.getByRole('button', { name: 'More actions' });
+  fireEvent.pointerDown(trigger, { button: 0, ctrlKey: false, pointerType: 'mouse' });
+  await waitFor(() => expect(screen.getByRole('menu')).toBeTruthy());
+  const menu = screen.getByRole('menu');
+  return Array.from(menu.querySelectorAll('[role="menuitem"], [role="separator"]')).map((el) =>
+    el.getAttribute('role') === 'separator'
+      ? '---'
+      : { label: (el.textContent ?? '').trim(), red: hasClass(el, 'text-destructive') },
+  );
+}
+
+describe("page:header reads the spec's variant 'danger' as destructive (objectui#12764)", () => {
+  it('a danger action placed first is not the filled primary; it draws the destructive style inline', () => {
+    renderHeader([
+      act('leave_organization', 'Leave Organization', { variant: 'danger' }),
+      act('invite_member', 'Invite Member'),
+    ]);
+    const buttons = rowButtons();
+    expect(buttons.map(nameOf)).toEqual(['Invite Member', 'Leave Organization', 'Edit']);
+    const [invite, leave, edit] = buttons;
+    expect(isFilled(invite)).toBe(true);
+    expect(isFilled(leave)).toBe(false);
+    expect(isDestructiveButton(leave)).toBe(true);
+    expect(isOutline(edit)).toBe(true);
+    expect(buttons.filter(isFilled)).toHaveLength(1);
+  });
+
+  it('with only a danger action, the host Edit is the primary and the danger action stays destructive', () => {
+    renderHeader([act('leave_organization', 'Leave Organization', { variant: 'danger' })]);
+    const buttons = rowButtons();
+    expect(buttons.map(nameOf)).toEqual(['Edit', 'Leave Organization']);
+    expect(isFilled(buttons[0])).toBe(true);
+    expect(isFilled(buttons[1])).toBe(false);
+    expect(isDestructiveButton(buttons[1])).toBe(true);
+  });
+
+  it('folded into the ⋯, a danger action is a red row in the destructive band, below the separator', async () => {
+    renderHeader(
+      [
+        act('invite_member', 'Invite Member'),
+        act('transfer_ownership', 'Transfer Ownership'),
+        act('leave_organization', 'Leave Organization', { variant: 'danger' }),
+      ],
+      { maxVisible: 2 },
+    );
+    expect(rowButtons().map(nameOf)).toEqual(['Invite Member', 'Transfer Ownership', 'Edit']);
+    expect(await overflowEntries()).toEqual([
+      { label: 'Share', red: false },
+      '---',
+      { label: 'Leave Organization', red: true },
+      { label: 'Delete', red: true },
+    ]);
+  });
+
+  it('CONTROL: without a danger action, sys_delete and the non-destructive actions keep their placement', async () => {
+    renderHeader([
+      act('invite_member', 'Invite Member'),
+      act('transfer_ownership', 'Transfer Ownership'),
+      act('export_members', 'Export Members'),
+      act('audit_log', 'Audit Log'),
+    ]);
+    const buttons = rowButtons();
+    expect(buttons.map(nameOf)).toEqual(['Invite Member', 'Transfer Ownership', 'Export Members', 'Edit']);
+    expect(buttons.map(isFilled)).toEqual([true, false, false, false]);
+    expect(buttons.slice(1).every(isOutline)).toBe(true);
+    expect(buttons.some(isDestructiveButton)).toBe(false);
+    expect(await overflowEntries()).toEqual([
+      { label: 'Audit Log', red: false },
+      { label: 'Share', red: false },
+      '---',
+      { label: 'Delete', red: true },
+    ]);
+  });
+});

packages/components/src/renderers/layout/containers.tsx

@@ -2073,10 +2073,14 @@ const PageHeaderRenderer: React.FC<any> = ({ schema, className, ...props }) => {
     );
     // "Host" is the key test the disabled-reason branch below uses: a key no
     // authored action holds came from `headerSystemActions`. "Destructive" is
-    // the menu's own red-row test, so the row that is painted red is the row
-    // that is set apart in the `⋯`.
+    // ONE predicate read everywhere below: the `⋯`'s destructive band and its
+    // separator, the red row, the inline button's style and the primary pick
+    // (objectui#12764). Its word is the spec's: `@objectstack/spec`'s action
+    // `variant` enum is `primary | secondary | danger | ghost | link`, and
+    // "danger = destructive". Shadcn's `destructive` is not a spec value, so
+    // it is not read here; the host's Delete is `sys_delete` by name.
     const isDestructiveAction = (a: any): boolean =>
-      a?.variant === 'destructive' || a?.name === 'sys_delete';
+      a?.variant === 'danger' || a?.name === 'sys_delete';
     const isHostAction = (a: any): boolean => {
       const key = (a?.name || a?.id || '') as string;
       return key !== '' && !authoredHeaderActionKeys.has(key);
@@ -2091,11 +2095,14 @@ const PageHeaderRenderer: React.FC<any> = ({ schema, className, ...props }) => {
     // filled style, so a header with three actions showed three primaries and
     // none of them read as the main one.
     // Only an action that can sit in the row competes for it — a destructive
-    // host action never does (see `isHostButton` below).
+    // host action never does (see `isHostButton` below) — and a destructive
+    // action is never the primary: a `variant: 'danger'` action first in the
+    // row drew as the filled primary (objectui#12764).
     const isHostButton = (a: unknown): boolean => isHostAction(a) && !isDestructiveAction(a);
     const rowEligible = buttonable.filter((a) => !isHostAction(a) || isHostButton(a));
-    const primaryIdx = rowEligible.findIndex((a) => a?.variant === 'primary');
-    const primaryAction: unknown = rowEligible[primaryIdx >= 0 ? primaryIdx : 0];
+    const primaryCandidates = rowEligible.filter((a) => !isDestructiveAction(a));
+    const primaryIdx = primaryCandidates.findIndex((a) => a?.variant === 'primary');
+    const primaryAction: unknown = primaryCandidates[primaryIdx >= 0 ? primaryIdx : 0];
     const ranked = primaryAction
       ? [primaryAction, ...buttonable.filter((a) => a !== primaryAction)]
       : buttonable;
@@ -2172,9 +2179,11 @@ const PageHeaderRenderer: React.FC<any> = ({ schema, className, ...props }) => {
     const renderButton = (action: any, idx: number) => {
       const label = resolveLabel(action);
       // The primary (see `primaryAction` above) is the one filled button —
-      // Shadcn's `default`; every other inline button is an outline button
-      // (objectui#12617).
-      const variant = action === primaryAction ? 'default' : 'outline';
+      // Shadcn's `default`; a destructive action shown inline is Shadcn's
+      // `destructive` (objectui#12764); every other inline button is an
+      // outline button (objectui#12617).
+      const variant =
+        action === primaryAction ? 'default' : isDestructiveAction(action) ? 'destructive' : 'outline';
       const size = action.size || 'sm';
       const disabled = isActionDisabled(action);
       const icon = typeof action.icon === 'string' ? action.icon : null;
@@ -2254,8 +2263,7 @@ const PageHeaderRenderer: React.FC<any> = ({ schema, className, ...props }) => {
                 const label = resolveLabel(action);
                 const disabled = isActionDisabled(action);
                 const icon = typeof action.icon === 'string' ? action.icon : null;
-                const isDestructive =
-                  action.variant === 'destructive' || action.name === 'sys_delete';
+                const isDestructive = isDestructiveAction(action);
                 // objectui#11811 — the same reason as the inline button, but
                 // drawn as a visible second line, not a tooltip. Inside the
                 // menu a tooltip trigger is unreachable from the keyboard: a
@@ -2701,7 +2709,7 @@ ComponentRegistry.register('header', PageHeaderRenderer, {
     // express the domain: `ComponentInput.type` has no integer/min/max slot, so
     // `description` stays the only authoring-time expression of it, and spec
     // stays the sole judge of values.)
-    { name: 'maxVisible', type: 'number', description: 'How many header actions render as inline buttons on desktop before the rest fold into the overflow menu. A positive integer — the contract rejects 0 and fractional values. Two kinds of action are routed to the overflow menu regardless of this budget and never occupy an inline slot: an action whose locations declare record_more without record_header, and any action with component action:menu. The host\'s own buttons (the record page\'s Edit) take no slot and never fold. The first inline action, or the one declaring variant primary, is the one filled primary button; the others are outline buttons.' },
+    { name: 'maxVisible', type: 'number', description: 'How many header actions render as inline buttons on desktop before the rest fold into the overflow menu. A positive integer — the contract rejects 0 and fractional values. Two kinds of action are routed to the overflow menu regardless of this budget and never occupy an inline slot: an action whose locations declare record_more without record_header, and any action with component action:menu. The host\'s own buttons (the record page\'s Edit) take no slot and never fold. The first inline action, or the one declaring variant primary, is the one filled primary button; the others are outline buttons. An action declaring variant danger is never the primary: inline it is a destructive button, and in the overflow menu it is a red row below a separator.' },
     { name: 'mobileMaxVisible', type: 'number', description: 'The same inline-button budget on mobile viewports, where horizontal room is scarce. A positive integer, defaulting to 1; the desktop half is Max Inline Actions, and the overflow routing rules stated there apply unchanged.' },
   ],
 });