diff --git a/.changeset/inert-lifecycle-path-readout-5768.md b/.changeset/inert-lifecycle-path-readout-5768.md new file mode 100644 index 000000000..e95f07204 --- /dev/null +++ b/.changeset/inert-lifecycle-path-readout-5768.md @@ -0,0 +1,46 @@ +--- +'@object-ui/plugin-detail': minor +--- + +`record:path` stops looking like a control it cannot be. + +The record page draws the object's lifecycle across the top from the +`stageField` role, and it drew each stage as a filled, shadowed, equal-width +pill — a segmented button group, sitting exactly where a CRM user reaches for +the stage control. Nothing was behind it. Measured in a browser on a shipped +build (HotCRM `crm_quote`, and this is generic record chrome, so every object +that declares a `stageField` has it): the segments were `role="listitem"` with +`cursor: auto` and `tabindex` null, no ancestor `button`/`tab`/`a`, and a full +pointer sequence (pointerdown → mousedown → pointerup → mouseup → click) left +the record's status untouched. Advancing a record needs the edit form or a bulk +action. Users spent two or three clicks on the path before concluding it was +decoration. + +There is no write path to connect it to, and this change does not open one: +this renderer's only channel is `useRecordContext()`, whose value exposes +`data` / `refresh` / `headerSystemActions` / `onToggleFavorite` and no +record-field mutation. Editing runs through `record:details`' +`` + `` (`dataSource.update(..., +{ ifMatch })`) or an action via `useActionEngine`; neither reaches this +component. + +So the promise is withdrawn rather than honoured. Each stage now renders as a +thin decorative rail segment with its label as plain text beneath it — the +vocabulary app-shell's approval step readout already uses. Gone: the per-stage +filled pill, the shadow, the ring, the bordered chip, the equal-width tap +target. Kept exactly as they were: which stage is current (`aria-current="step"` +and type weight), the travelled/untravelled distinction, the check on completed +stages, and the separated `lost`-terminal group. The accessible semantics did +not move — `role="list"` / `role="listitem"` with no tab stop was already +correct for a readout, and it stays that way. + +Three DOM attributes carry the state that colour used to be the only carrier +of, so the classification is assertable without reading CSS: +`data-stage-state` (`completed` | `current` | `upcoming`), +`data-stage-terminal` (`won` | `lost`), and `data-stage-rail` on the decorative +indicator. + +**Not in this change:** click-to-advance. A stage control that writes +`stageField` through the same permission/validation envelope an edit takes is a +separate feature with its own appetite, and folding it in here was explicitly +ruled out. diff --git a/packages/plugin-detail/src/renderers/__tests__/record-path.inertReadout.test.tsx b/packages/plugin-detail/src/renderers/__tests__/record-path.inertReadout.test.tsx new file mode 100644 index 000000000..26161a9dc --- /dev/null +++ b/packages/plugin-detail/src/renderers/__tests__/record-path.inertReadout.test.tsx @@ -0,0 +1,231 @@ +/** + * 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. + * + * ══════════════════════════════════════════════════════════════════════════ + * `record:path` is a READOUT and must not look like a control (objectui#5768) + * ══════════════════════════════════════════════════════════════════════════ + * + * The card was measured in a running browser against a shipped build, on + * HotCRM `crm_quote` (the object declares a `stageField`, so this is generic + * record chrome, not one app's skin). What the browser reported: + * + * document.querySelectorAll('main [role="listitem"]') // listitem, not button/tab + * getComputedStyle(el).cursor // "auto" + * el.getAttribute('tabindex') // null + * el.closest('button,[role=button],[role=tab],a') // null + * // pointerdown → mousedown → pointerup → mouseup → click: status unchanged + * + * Every one of those readings is CORRECT for a status readout. The defect was + * that the pixels said otherwise: filled, shadowed, equal-width pills — a + * segmented button group — sitting exactly where a CRM user reaches for the + * stage control. Maintainer ruling (2026-08-23): direction 2, stop-loss. The + * markup stops claiming to be a control; a real click-to-advance stage control + * is a separate card with its own appetite, NOT folded in here. + * + * ── What each group is for ──────────────────────────────────────────────── + * + * Group A is the CONTROL — what must not move. It also establishes + * non-vacuity: it proves this harness can move `aria-current` between stages, + * so "the current stage is the current one" below is a verdict rather than an + * inability to tell the stages apart. + * + * Group B is the readout contract. Its legs divide, and the division is + * recorded here because it is what reverse verification measured: + * + * • B1–B3 (no interactive role, no tab stop, inert on a full pointer + * sequence) were ALREADY TRUE before this change — they re-measure the + * browser's readings at the live source and pin them against the *wrong* + * fix, i.e. bolting a control onto a surface that has no write path. + * They do NOT discriminate this PR's change; ablating the styling leaves + * them green. Named, not hidden. + * • B4 is the discriminating leg. A rail has a decorative indicator that is + * a SEPARATE element from its label; a pill IS its own label's surface and + * cannot have one. Ablating the rail turns B4 red and nothing else. + * + * ── Why no CSS is asserted ──────────────────────────────────────────────── + * + * The test DOM resolves no Tailwind, so `getComputedStyle` here answers + * nothing about what a user sees, and class strings are not a contract. The + * state a colour used to be the only carrier of is therefore read off + * `data-stage-state` / `data-stage-terminal`, and the pill-vs-rail difference + * off `data-stage-rail` — semantics and structure, never appearance. + * + * ── Resolution path (ablation validity) ─────────────────────────────────── + * + * The subject is imported RELATIVELY (`../record-path`), i.e. straight from + * this package's source, and the only cross-package import is + * `@object-ui/react`, which the root `vitest.config.mts` alias table maps to + * `packages/react/src`. Nothing in this file resolves through any `dist/`, so + * an ablation of `record-path.tsx` is visible to this suite without a rebuild. + */ + +import * as React from 'react'; +import { describe, it, expect, afterEach } from 'vitest'; +import { render, cleanup, fireEvent, within, type RenderResult } from '@testing-library/react'; +import { RecordContextProvider } from '@object-ui/react'; +import { RecordPathRenderer } from '../record-path'; + +/** The card's own fixture: HotCRM `crm_quote`, six stages, none terminal. */ +const QUOTE_STAGES = [ + { value: 'draft', label: '草稿' }, + { value: 'in_review', label: '审核中' }, + { value: 'submitted', label: '已提交' }, + { value: 'accepted', label: '已接受' }, + { value: 'rejected', label: '已拒绝' }, + { value: 'expired', label: '已过期' }, +]; + +/** A declared `lost` terminal, so the separated alt-group renders too. */ +const WITH_LOST_STAGES = [ + { value: 'draft', label: '草稿' }, + { value: 'in_review', label: '审核中' }, + { value: 'submitted', label: '已提交' }, + { value: 'declined', label: '已拒绝', terminal: 'lost' as const }, +]; + +function mount(status: string, stages = QUOTE_STAGES): RenderResult { + return render( + + + , + ); +} + +/** + * Both the desktop and the mobile row are in the DOM at once (they are + * separated by a CSS breakpoint, which this environment does not apply), so + * every invariant is asserted on BOTH — a regression that reached only one + * viewport would otherwise pass. + */ +const rows = (r: RenderResult): HTMLElement[] => + Array.from(r.container.querySelectorAll('[role="list"]')) as HTMLElement[]; + +const stageOf = (row: HTMLElement, label: string): HTMLElement => { + const el = within(row).getByText(label).closest('[role="listitem"]'); + if (!el) throw new Error(`no [role="listitem"] ancestor for stage ${label}`); + return el as HTMLElement; +}; + +const currentLabels = (row: HTMLElement): string[] => + Array.from(row.querySelectorAll('[aria-current="step"]')).map((el) => (el.textContent || '').trim()); + +afterEach(() => cleanup()); + +describe('#5768 group A — controls: the stages still render, and the current one is still the current one', () => { + it('every stage label is on screen, in both rows', () => { + // Non-vacuity for everything below: if the path painted nothing, an + // "is not a button" assertion would pass for the wrong reason. + const r = mount('draft'); + expect(rows(r)).toHaveLength(2); + for (const row of rows(r)) { + for (const s of QUOTE_STAGES) expect(within(row).getByText(s.label)).toBeTruthy(); + } + }); + + it('exactly one stage per row is marked current, and it is the record\'s stage', () => { + const r = mount('in_review'); + for (const row of rows(r)) { + expect(currentLabels(row)).toEqual(['审核中']); + expect(stageOf(row, '审核中')).toHaveAttribute('data-stage-state', 'current'); + } + }); + + it('the mark MOVES with the record — so the assertion above is a verdict, not a tie', () => { + for (const row of rows(mount('draft'))) expect(currentLabels(row)).toEqual(['草稿']); + cleanup(); + for (const row of rows(mount('submitted'))) { + expect(currentLabels(row)).toEqual(['已提交']); + } + }); + + it('travelled / untravelled stages keep their classification', () => { + const r = mount('submitted'); + for (const row of rows(r)) { + expect(stageOf(row, '草稿')).toHaveAttribute('data-stage-state', 'completed'); + expect(stageOf(row, '审核中')).toHaveAttribute('data-stage-state', 'completed'); + expect(stageOf(row, '已提交')).toHaveAttribute('data-stage-state', 'current'); + expect(stageOf(row, '已接受')).toHaveAttribute('data-stage-state', 'upcoming'); + expect(stageOf(row, '已过期')).toHaveAttribute('data-stage-state', 'upcoming'); + } + }); + + it('a declared `lost` terminal still renders, still separated from the forward stages', () => { + const r = mount('in_review', WITH_LOST_STAGES); + for (const row of rows(r)) { + expect(stageOf(row, '已拒绝')).toHaveAttribute('data-stage-terminal', 'lost'); + // The forward stages did not get swept into the terminal group. + expect(stageOf(row, '已提交')).not.toHaveAttribute('data-stage-terminal'); + expect(within(row).getByText('草稿')).toBeTruthy(); + } + }); +}); + +describe('#5768 group B — the readout does not claim to be a control', () => { + // B1–B3 re-measure the browser's readings at the live source. They pin the + // surface against the wrong fix (a control with no write path behind it); + // they are NOT what this PR changed. See the docblock. + + it('B1 — no element in the path carries an interactive role', () => { + const r = mount('draft'); + for (const row of rows(r)) { + for (const role of ['button', 'tab', 'link', 'menuitem', 'radio', 'checkbox', 'switch']) { + expect(within(row).queryAllByRole(role)).toHaveLength(0); + } + expect(row.querySelectorAll('[role="button"],[role="tab"],[role="link"]')).toHaveLength(0); + } + }); + + it('B2 — nothing in the path is a tab stop or natively focusable', () => { + const r = mount('draft'); + for (const row of rows(r)) { + expect( + row.querySelectorAll('[tabindex],button,a[href],input,select,textarea,summary,[contenteditable]'), + ).toHaveLength(0); + for (const item of Array.from(row.querySelectorAll('[role="listitem"]'))) { + expect(item.getAttribute('tabindex')).toBeNull(); + expect(item.closest('button,[role="button"],[role="tab"],a')).toBeNull(); + } + } + }); + + it('B3 — a full pointer sequence on the next stage changes nothing', () => { + // The card's exact gesture: the user, on a `draft` quote, presses 审核中. + const r = mount('draft'); + for (const row of rows(r)) { + const target = stageOf(row, '审核中'); + fireEvent.pointerDown(target); + fireEvent.mouseDown(target); + fireEvent.pointerUp(target); + fireEvent.mouseUp(target); + fireEvent.click(target); + + expect(currentLabels(row)).toEqual(['草稿']); + expect(target).toHaveAttribute('data-stage-state', 'upcoming'); + // …and the gesture did not park focus on a thing that cannot use it. + expect(document.activeElement).toBe(document.body); + } + }); + + it('B4 — each stage is an indicator PLUS a label, not a label on a pressable surface', () => { + // The discriminating leg. A rail's indicator is a separate, decorative, + // text-free element; a filled pill is its own label's surface and has + // none. Remove the rail and only this leg goes red. + const r = mount('in_review'); + for (const row of rows(r)) { + const items = Array.from(row.querySelectorAll('[role="listitem"]')) as HTMLElement[]; + expect(items.length).toBe(QUOTE_STAGES.length); + for (const item of items) { + const rail = item.querySelector('[data-stage-rail]'); + expect(rail).not.toBeNull(); + expect(rail).toHaveAttribute('aria-hidden', 'true'); + // The indicator carries no label — the label lives beside it. + expect((rail!.textContent || '')).toBe(''); + expect((item.textContent || '').trim().length).toBeGreaterThan(0); + } + } + }); +}); diff --git a/packages/plugin-detail/src/renderers/record-path.tsx b/packages/plugin-detail/src/renderers/record-path.tsx index 49894cfc8..bea8db9a1 100644 --- a/packages/plugin-detail/src/renderers/record-path.tsx +++ b/packages/plugin-detail/src/renderers/record-path.tsx @@ -11,8 +11,41 @@ * completed (with a check); the current renders as active; subsequent * stages render as upcoming. * - * This is a greenfield component (no underlying plugin-detail equivalent), - * intentionally minimal so it can be styled in line with the host page. + * ── This surface is a READOUT, and it must look like one (objectui#5768) ── + * + * It used to draw each stage as a filled, shadowed, equal-width pill — the + * exact shape of a segmented button group — while carrying no handler, no + * cursor and no tab stop. Measured in a browser on a shipped build: the + * segments were `role="listitem"` with `cursor: auto` and `tabindex` null, + * and a full pointer sequence (pointerdown → mousedown → pointerup → + * mouseup → click) left the record's status untouched. Users spent clicks + * on it before concluding it was decoration. + * + * There is no write path to spend those clicks on: this renderer's only + * channel is `useRecordContext()`, whose value (`RecordContextValue` in + * `@object-ui/react`) exposes `data` / `refresh` / `headerSystemActions` / + * `onToggleFavorite` and NO record-field mutation. Editing goes through + * `record:details`' `` + `` + * (`dataSource.update(..., { ifMatch })`) or an action via + * `useActionEngine`; neither reaches here. Click-to-advance is a separate, + * approved-on-its-own-appetite feature — until it exists, the pixels must + * not promise it. + * + * So the presentation below is a PROGRESS RAIL: a thin decorative indicator + * per stage with the label as plain text beneath it, which is the same + * vocabulary app-shell's approval step readout already uses + * (`RecordApprovalsPanel` — marker, rail, bare label, weight for "current", + * never a filled surface). Deliberately absent: per-stage filled pills, + * `shadow`, `ring`, bordered chips, and equal-width tap targets. + * + * The DOM attributes below carry state that used to live only in colour, so + * the classification stays assertable without reading CSS (the test DOM + * resolves no Tailwind): + * • `data-stage-state` — `completed` | `current` | `upcoming` + * • `data-stage-terminal` — `won` | `lost`, when a stage is classified + * • `data-stage-rail` — marks the decorative indicator as an element + * SEPARATE from the label. A rail has one; a + * pill, which is its own label's surface, cannot. */ import React from 'react'; @@ -25,6 +58,8 @@ const splitDesigner = (props: Record) => { return { designer: { 'data-obj-id': id, 'data-obj-type': type, style }, rest }; }; +type StageState = 'completed' | 'current' | 'upcoming'; + export interface RecordPathRendererProps { schema?: RecordPathComponentProps & Record; className?: string; @@ -74,8 +109,8 @@ export const RecordPathRenderer: React.FC = ({ const stageKinds = stages.map(classify); // Find the index of the FIRST lost-class stage so we can render it // (and any subsequent lost terminals) as a visually separated alt - // group. Won-class stages stay inside the forward chevron path — - // they're the successful terminus. + // group. Won-class stages stay inside the forward path — they're the + // successful terminus. const firstLostIdx = stageKinds.findIndex((k) => k === 'lost'); const forwardStages = firstLostIdx === -1 ? stages : stages.slice(0, firstLostIdx); const lostStages = firstLostIdx === -1 ? [] : stages.slice(firstLostIdx); @@ -95,84 +130,104 @@ export const RecordPathRenderer: React.FC = ({ ); } - // iOS-style connected segments (no chevron tessellation): each stage is a - // rounded segment in a gapped row — completed = mint, current = accent, - // upcoming = muted track. + // The rail: a 6px track segment. Completed reads as travelled (emerald, + // matching the approvals readout's `done`), current as where the record + // sits (accent), upcoming as untravelled track. A `lost` terminal tints + // destructive; an unreached `won` terminus stays a faint emerald so the + // goal is legible without being a surface you could press. + const railClass = (state: StageState, terminal?: 'won' | 'lost') => + cn( + 'h-1.5 w-full rounded-full', + terminal === 'lost' && (state === 'current' ? 'bg-destructive' : 'bg-destructive/25'), + terminal !== 'lost' && state === 'current' && 'bg-primary', + terminal !== 'lost' && state === 'completed' && 'bg-emerald-500', + terminal !== 'lost' && state === 'upcoming' && (terminal === 'won' ? 'bg-emerald-500/30' : 'bg-muted'), + ); - const last = forwardStages.length - 1; + // Emphasis by TYPE WEIGHT, not by a filled box — the one cue a readout can + // spend without implying it can be pressed. + const labelClass = (state: StageState, terminal?: 'won' | 'lost') => + cn( + 'block min-w-0 text-xs', + state === 'current' && (terminal === 'lost' ? 'font-semibold text-destructive' : 'font-semibold text-foreground'), + state === 'completed' && 'font-normal text-muted-foreground', + state === 'upcoming' && 'font-normal text-muted-foreground', + ); + + const renderStage = (o: { + key: string; + stage: { label: string }; + state: StageState; + terminal?: 'won' | 'lost'; + className?: string; + labelClassName?: string; + }) => ( +
+
+ ); - // Mobile shows ALL stages as pills (lost too) — visual separation done - // via color, not layout, since there's not enough room to fork the row. + const last = forwardStages.length - 1; return (
- {/* Desktop: chevron path → optional lost-alt group */} + {/* Desktop: forward rail → optional lost-alt group */}
-
+
{forwardStages.map((stage, idx) => { const isCompleted = !currentInLost && currentIdx >= 0 && idx < currentIdx; const isCurrent = !currentInLost && idx === currentIdx; const isWonTerminus = forwardKinds[idx] === 'won' && idx === last; - return ( -
- - {isCompleted && ✓} - {isWonTerminus && !isCurrent && 🏆} - {stage.label} - -
- ); + return renderStage({ + key: `${stage.value}-${idx}`, + stage, + state: isCurrent ? 'current' : isCompleted ? 'completed' : 'upcoming', + terminal: isWonTerminus ? 'won' : undefined, + className: 'flex-1 min-w-0', + labelClassName: 'text-center truncate', + }); })}
{lostStages.length > 0 && ( - // Separated alt-terminus group — gap, muted/destructive tint, - // pill (not chevron) shape so it doesn't read as "step N+1" in - // the forward path. Same affordance that Salesforce/HubSpot use. -
+ // Separated alt-terminus group — a gap and a divider, so it does not + // read as "step N+1" in the forward path. +
{lostStages.map((stage, lIdx) => { const absIdx = firstLostIdx + lIdx; - const isCurrent = absIdx === currentIdx; - return ( -
- - ✗ - {stage.label} - -
- ); + return renderStage({ + key: `${stage.value}-lost-${lIdx}`, + stage, + state: absIdx === currentIdx ? 'current' : 'upcoming', + terminal: 'lost', + className: 'shrink-0', + labelClassName: 'text-center whitespace-nowrap', + }); })}
)}
- {/* Mobile: horizontally scrollable pill row */} + {/* Mobile: horizontally scrollable rail row — same treatment, no chips */}
@@ -181,27 +236,14 @@ export const RecordPathRenderer: React.FC = ({ const isLost = kind === 'lost'; const isCompleted = !isLost && !currentInLost && currentIdx >= 0 && idx < currentIdx; const isCurrent = idx === currentIdx; - return ( -
- - {isLost && ✗} - {!isLost && isCompleted && ✓} - {stage.label} - -
- ); + return renderStage({ + key: `${stage.value}-${idx}-m`, + stage, + state: isCurrent ? 'current' : isCompleted ? 'completed' : 'upcoming', + terminal: kind, + className: 'shrink-0', + labelClassName: 'whitespace-nowrap', + }); })}