fix(components): the record header draws one primary action, the rest as outline buttons

.changeset/12617-record-header-actions.md

@@ -0,0 +1,14 @@
+---
+'@object-ui/components': patch
+'@object-ui/app-shell': patch
+---
+
+The record header draws one primary action and the rest as secondary buttons (objectui#12617).
+
+`@objectstack/spec`'s action contract says that in `record_header` the first visible action becomes the primary button, and that a renderer may prefer a `variant: 'primary'` action. `page:header` drew every action without a declared variant with the filled style, so a header with three actions showed three primary buttons. Now:
+
+- **One primary.** The primary is the first action declaring `variant: 'primary'`, else the first by `order`. It leads the row and is the only filled button. The other inline actions are outline buttons. A record with no app action shows its Edit as the primary.
+- **The rest in the `⋯`.** The page's own actions share the `maxVisible` budget (3 on desktop, 1 on mobile). What does not fit goes to the `⋯` menu, above the host's favorites, Share and Delete.
+- **Edit stays a labelled button.** The record page's Edit is the record's base action. It stays a labelled outline button in the row at every width, takes none of the app's slots, and no longer folds into the `⋯` behind an icon-only pencil once the app's actions fill the row. A destructive host action (Delete) always goes to the `⋯`, never inline.
+
+A declared action whose `visible` predicate holds is still drawn, and one whose predicate fails is still absent. Nothing is added to either package entry: no export, prop, type member or language-pack key.

packages/app-shell/src/views/RecordDetailView.headerMenuIcons-12529.test.tsx

@@ -9,13 +9,15 @@
 /**
  * objectui#12529: the record header's system entries (Edit, favorites, Share,
  * Delete) declare icons through the same `icon` key the app's actions use, so
- * the `⋯` menu draws one kind of row; Delete is last, behind a separator; and
- * when the app's own actions fill the header row, so Edit folds into the `⋯`,
- * an icon-only Edit button stays beside the `⋯` and opens the same edit
- * session the folded `Edit` entry opens.
+ * the `⋯` menu draws one kind of row; Delete is last, behind a separator.
  *
- * CONTROL: with room in the row, Edit is today's labelled button and no icon
- * button is drawn.
+ * objectui#12617 replaced this file's third rule: Edit no longer folds into
+ * the `⋯` behind an icon-only twin once the app's actions fill the row. It is
+ * the record's base action, so it stays a labelled outline button in the row
+ * at every width and takes none of the app's slots; the first app action is
+ * the one filled primary button.
+ *
+ * CONTROL: with a single app action, Edit is the same labelled button.
  *
  * Harness: the `RecordDetailView.favoritesEntry-12427` mount (the real record
  * page under a real `I18nProvider`, a metadata context and the real
@@ -180,12 +182,13 @@ afterEach(() => {
   vi.clearAllMocks();
 });
 
-describe('record header ⋯ menu: iconed system entries, Delete set apart, an Edit icon button when the row is full (objectui#12529)', () => {
-  it('each of the four system entries carries its icon', async () => {
+describe('record header: iconed system entries, Delete set apart in the ⋯, Edit a labelled button in the row (objectui#12529, objectui#12617)', () => {
+  it('each system entry carries its icon: Edit in the row, the other three in the ⋯', async () => {
     await mountRecordPage(FULL_ROW);
+    const edit = screen.getByRole('button', { name: 'Edit' });
+    await waitFor(() => expect(edit.querySelector('svg.lucide-pencil')).not.toBeNull());
     const menu = await openOverflow();
     const expected: Record<string, string> = {
-      Edit: 'lucide-pencil',
       'Add to favorites': 'lucide-star',
       Share: 'lucide-share',
       Delete: 'lucide-trash',
@@ -197,35 +200,29 @@ describe('record header ⋯ menu: iconed system entries, Delete set apart, an Ed
     }
   });
 
-  it('Edit leads the system group, and Delete is last, after a separator', async () => {
+  it('the ⋯ holds the system group with Delete last, after a separator; Edit is not in it', async () => {
     await mountRecordPage(FULL_ROW);
     const menu = await openOverflow();
-    expect(menuRows(menu)).toEqual(['Edit', 'Add to favorites', 'Share', '---', 'Delete']);
+    expect(menuRows(menu)).toEqual(['Add to favorites', 'Share', '---', 'Delete']);
   });
 
-  it('with three app actions in the header, an icon-only Edit button sits beside the ⋯ and opens the session the folded Edit entry opens', async () => {
+  it('with three app actions in the header, Edit stays a labelled button after them, opens the edit session, and nothing is icon-only', async () => {
     const { onEdit } = await mountRecordPage(FULL_ROW);
     const bar = toolbar();
     const buttons = Array.from(bar.querySelectorAll(':scope > button, :scope > span > button'));
     const names = buttons.map((b) => b.getAttribute('aria-label') ?? text(b));
     expect(names).toEqual(['Convert Lead', 'Schedule Follow-up', 'Log a Call', 'Edit', 'More actions']);
 
-    const editIcon = screen.getByRole('button', { name: 'Edit' });
-    // Icon-only: no visible text, the pencil is all it draws.
-    expect(text(editIcon)).toBe('');
-    await waitFor(() => expect(editIcon.querySelector('svg.lucide-pencil')).not.toBeNull());
+    const edit = screen.getByRole('button', { name: 'Edit' });
+    expect(text(edit)).toBe('Edit');
+    expect(screen.getAllByRole('button', { name: 'Edit' })).toHaveLength(1);
+    expect(bar.querySelector('[data-folded-header-action]')).toBeNull();
 
-    fireEvent.click(editIcon);
+    fireEvent.click(edit);
     expect(onEdit).toHaveBeenCalledTimes(1);
-    const fromIcon = onEdit.mock.calls[0];
-
-    await openOverflow();
-    fireEvent.click(screen.getByRole('menuitem', { name: 'Edit' }));
-    expect(onEdit).toHaveBeenCalledTimes(2);
-    expect(onEdit.mock.calls[1]).toEqual(fromIcon);
   });
 
-  it('CONTROL: with room in the row, Edit is the labelled button it was and no icon button is drawn', async () => {
+  it('CONTROL: with a single app action, Edit is the same labelled button and no icon button is drawn', async () => {
     await mountRecordPage([appAction('convert_lead', 'Convert Lead')]);
     const bar = toolbar();
     const edit = screen.getByRole('button', { name: 'Edit' });

packages/app-shell/src/views/RecordDetailView.tsx

@@ -3084,14 +3084,16 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
         name: 'sys_edit',
         label: t('detail.edit', { defaultValue: 'Edit' }),
         // objectui#12529 — each system entry declares its icon through the
-        // same `icon` key the app's actions use, so the `⋯` menu draws one
-        // kind of row. When this CTA folds into the `⋯` because the row is
-        // full, `page:header` keeps an icon-only twin of it (this pencil)
-        // beside the `⋯`.
+        // same `icon` key the app's actions use, so every row draws one kind
+        // of entry. `page:header` keeps this CTA a labelled button in the row
+        // at every width: it takes none of the app's `maxVisible` slots and
+        // never folds into the `⋯` (objectui#12617). It is the filled primary
+        // button only when the record has no app action ahead of it; next to
+        // app actions it is an outline button, which `page:header` decides —
+        // no `variant` is declared here for it to read.
         icon: 'pencil',
         type: 'script',
         locations: ['record_header'],
-        variant: 'default',
         order: 100,
         // objectui#2572 item 4 — while a shared inline-edit draft is live,
         // the page:header renderer greys this CTA out so the classic form

packages/components/src/__tests__/page-header-actions.test.tsx

@@ -114,7 +114,7 @@ describe('PageHeaderRenderer — actions slot', () => {
     expect(screen.queryByRole('button', { name: /Empty Placement/i })).toBeNull();
   });
 
-  it('still renders host system actions that declare no placement — chrome is placed by the host', () => {
+  it('still renders host system actions that declare no placement — chrome is placed by the host', async () => {
     // The host injects Edit/Share/Delete; nobody authored a `locations` for
     // them, and `action:bar` has always exempted its `systemActions` slot for
     // exactly that reason. Before #3142 this renderer filtered them, so the
@@ -126,7 +126,12 @@ describe('PageHeaderRenderer — actions slot', () => {
         headerSystemActions: [{ name: 'sys_delete', label: 'Delete Record' }],
       },
     );
-    expect(screen.getByRole('button', { name: /Delete Record/i })).toBeTruthy();
+    // A destructive host action renders in the `⋯`, never as an inline
+    // button (objectui#12617) — rendered either way, which is this pin.
+    expect(screen.queryByRole('button', { name: /Delete Record/i })).toBeNull();
+    const trigger = screen.getByRole('button', { name: /More actions/i });
+    fireEvent.pointerDown(trigger, { button: 0, ctrlKey: false, pointerType: 'mouse' });
+    await waitFor(() => expect(screen.getByRole('menuitem', { name: /Delete Record/i })).toBeTruthy());
   });
 
   it('shows action when visible expression matches record', () => {
@@ -238,11 +243,11 @@ describe('PageHeaderRenderer — actions slot', () => {
         ],
       },
     );
-    // With 4 actions and the default maxVisible of 3, the first three stay
-    // inline and the rest collapse into a `⋯` overflow menu. We assert the
-    // authored action is rendered as a button and the overflow trigger is
-    // present (full menu contents are exercised by separate dropdown
-    // interaction tests).
+    // The host's buttons stay inline beside the authored action and the
+    // host's destructive Delete folds into the `⋯` (objectui#12617). We
+    // assert the authored action is rendered as a button and the overflow
+    // trigger is present (full menu contents are exercised by separate
+    // dropdown interaction tests).
     expect(screen.getByRole('button', { name: /Convert Lead/i })).toBeTruthy();
     expect(screen.getByRole('button', { name: /More actions/i })).toBeTruthy();
   });

packages/components/src/__tests__/page-header-primary-12617.test.tsx

@@ -0,0 +1,160 @@
+/**
+ * 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#12617: the record header draws ONE primary action and the rest as
+ * secondary buttons.
+ *
+ * `@objectstack/spec`'s action contract says "in `record_header` the first
+ * visible action becomes the primary button" and lets a renderer prefer a
+ * `variant: 'primary'` action. `page:header` drew every action without a
+ * declared variant with the filled style, so a header with three actions
+ * showed three primaries. Now:
+ *
+ *   - the primary is the first action declaring `variant: 'primary'`, else the
+ *     first by `order`; it leads the row and is the only filled button;
+ *   - the next two inline actions are outline buttons;
+ *   - a fourth folds into the `⋯`, above the host's Share / Delete;
+ *   - the host's Edit stays a labelled button in the row and takes no slot;
+ *   - a declared action whose `visible` holds is present, and (CONTROL) one
+ *     whose `visible` fails is absent (objectui#12303).
+ *
+ * "Filled" and "outline" are read off the Button primitive's own variant
+ * classes (`bg-primary` for Shadcn's `default`, `border-input` for `outline`).
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { render, screen, fireEvent, waitFor, within } 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 LEAD = { id: 'lead-1', name: 'Mira Costa', is_converted: false, status: 'contacted' };
+
+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 renderRecordHeader(actions: any[], record: any = LEAD) {
+  return render(
+    <ActionProvider>
+      <RecordContextProvider
+        objectName="crm_lead"
+        recordId={record.id}
+        data={record}
+        objectSchema={{ name: 'crm_lead', label: 'Lead' }}
+        headerSystemActions={HOST_ACTIONS}
+      >
+        <PageHeader schema={{ type: 'page:header', actions }} />
+      </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 toolbarButtons(): HTMLElement[] {
+  const bar = screen.getByRole('toolbar', { name: /Page header actions/i });
+  return Array.from(bar.querySelectorAll('button'));
+}
+
+const nameOf = (b: HTMLElement) => b.getAttribute('aria-label') ?? (b.textContent ?? '').trim();
+const isFilled = (b: HTMLElement) => b.className.split(/\s+/).includes('bg-primary');
+const isOutline = (b: HTMLElement) => b.className.split(/\s+/).includes('border-input');
+
+async function overflowRows(): Promise<string[]> {
+  const trigger = screen.getByRole('button', { name: 'More actions' });
+  fireEvent.pointerDown(trigger, { button: 0, ctrlKey: false, pointerType: 'mouse' });
+  await waitFor(() => expect(screen.getByRole('menu')).toBeTruthy());
+  return within(screen.getByRole('menu'))
+    .getAllByRole('menuitem')
+    .map((el) => (el.textContent ?? '').trim());
+}
+
+describe('page:header draws one primary action, the rest secondary (objectui#12617)', () => {
+  it('three record_header actions render as one filled primary + two outline buttons, then a labelled outline Edit', () => {
+    renderRecordHeader([
+      act('convert_lead', 'Convert Lead'),
+      act('schedule_followup', 'Schedule Follow-up'),
+      act('log_call', 'Log a Call'),
+    ]);
+    const buttons = toolbarButtons().filter((b) => nameOf(b) !== 'More actions');
+    expect(buttons.map(nameOf)).toEqual(['Convert Lead', 'Schedule Follow-up', 'Log a Call', 'Edit']);
+    expect(buttons.map(isFilled)).toEqual([true, false, false, false]);
+    expect(buttons.slice(1).every(isOutline)).toBe(true);
+    // Edit is a labelled button, never icon-only.
+    expect((buttons[3].textContent ?? '').trim()).toBe('Edit');
+  });
+
+  it('a fourth action folds into the ⋯, above the host Share / Delete; Edit stays in the row', async () => {
+    renderRecordHeader([
+      act('convert_lead', 'Convert Lead'),
+      act('schedule_followup', 'Schedule Follow-up'),
+      act('log_call', 'Log a Call'),
+      act('log_meeting', 'Log a Meeting'),
+    ]);
+    const names = toolbarButtons().map(nameOf);
+    expect(names).toEqual(['Convert Lead', 'Schedule Follow-up', 'Log a Call', 'Edit', 'More actions']);
+    expect(await overflowRows()).toEqual(['Log a Meeting', 'Share', 'Delete']);
+  });
+
+  it("`variant: 'primary'` wins over `order`: it leads the row as the only filled button", () => {
+    renderRecordHeader([
+      act('log_call', 'Log a Call', { order: -10 }),
+      act('log_meeting', 'Log a Meeting'),
+      act('convert_lead', 'Convert Lead', { variant: 'primary', order: 50 }),
+    ]);
+    const buttons = toolbarButtons().filter((b) => nameOf(b) !== 'More actions');
+    expect(buttons.map(nameOf)).toEqual(['Convert Lead', 'Log a Call', 'Log a Meeting', 'Edit']);
+    expect(buttons.map(isFilled)).toEqual([true, false, false, false]);
+  });
+
+  it('a declared action whose `visible` holds is present; CONTROL: one whose `visible` fails is absent', () => {
+    const gate = {
+      dialect: 'cel',
+      source: 'has(record.is_converted) && record.is_converted == false && record.status != "converted"',
+    };
+    renderRecordHeader([
+      act('convert_lead', 'Convert Lead', { visible: gate }),
+      act('reopen_lead', 'Reopen Lead', { visible: { dialect: 'cel', source: 'record.status == "converted"' } }),
+      act('log_call', 'Log a Call'),
+    ]);
+    const names = toolbarButtons().map(nameOf);
+    expect(names).toContain('Convert Lead');
+    expect(names).not.toContain('Reopen Lead');
+    // The visible gate decides presence, not rank: the surviving first action
+    // is still the primary.
+    const convert = toolbarButtons().find((b) => nameOf(b) === 'Convert Lead')!;
+    expect(isFilled(convert)).toBe(true);
+  });
+
+  it('with no app action, the host Edit is the primary button', () => {
+    renderRecordHeader([]);
+    const buttons = toolbarButtons().filter((b) => nameOf(b) !== 'More actions');
+    expect(buttons.map(nameOf)).toEqual(['Edit']);
+    expect(isFilled(buttons[0])).toBe(true);
+  });
+});