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
32 changes: 32 additions & 0 deletions .changeset/5998-record-path-one-classification.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
---
'@object-ui/plugin-detail': patch
---

`record:path` now derives ONE stage classification and hands it to both of its rows, so a
stage can no longer paint and announce two different ways depending on viewport width
(objectui#5998).

The renderer draws a desktop row (`hidden sm:flex`) and a mobile row (`flex sm:hidden`) from
the same `stages[]`, and each used to compute its own `terminal` from that array.
`renderStage` passes the same `terminal` to `railClass` and — since objectui#5957 — to
`stageAriaLabel`, so any disagreement surfaced in the colour and in the accessible name at
once. The rows disagreed on two axes:

Mid-path goal. `WON_TOKENS` matches `完成`, an ordinary word rather than a Salesforce-style
`closed_won` value, so a path like `草稿 → 完成 → 已归档` classified index 1 as `won`.
Desktop declined it, because only the last forward stage can be the goal terminus; mobile
marked it `bg-emerald-500/30` and announced `goal stage, not reached`.

The lost slice. Desktop renders `stages.slice(firstLostIdx)` as a visually separated alt
group and hardcoded `terminal: 'lost'` on every member of that positionally-defined group,
while mobile classified each stage on its own. A plain stage after a `lost` one
(`草稿 → 失败 → 已归档`) therefore painted destructive and announced `closed lost` on
desktop and plain on mobile; a `won`-classified stage in the same position drew `'lost'`
from one row and `'won'` from the other.

Both rows now index a single `stageTerminals` array: `lost` is a property of the stage
itself, `won` is the goal terminus and so is the last forward stage or nothing, and the
positional grouping is a layout concern that no longer overrides what a stage is. Behaviour
is narrowed on both axes and never widened — no stage gains a `terminal` on either row that
it did not already carry there, and a goal terminus that really is last keeps its faint
emerald rail and its `goal stage, not reached` name on both rows.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
/**
* 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.
*
* ══════════════════════════════════════════════════════════════════════════
* ONE record, ONE classification — not one per viewport (objectui#5998)
* ══════════════════════════════════════════════════════════════════════════
*
* `record:path` renders two rows from the same `stages[]` — a desktop row
* (`hidden sm:flex`) and a mobile row (`flex sm:hidden`) — and each used to
* derive its own `terminal` classification from that array. Because
* `renderStage` hands the same `terminal` to `railClass` AND (since
* objectui#5957) to `stageAriaLabel`, a disagreement showed up in the paint and
* in the accessible name at once: one stage of one record, described two
* different ways, chosen by nothing but the width of the window.
*
* ── What this file pins, and why it is a CROSS-ROW assertion ──────────────
*
* The fix is not "make the two rows agree" — two independently-derived
* classifications that happen to agree today leave the defect one edit away.
* The fix is ONE array (`stageTerminals`) that both rows index. So the pin is
* stated as the invariant that array exists to guarantee: for every stage, the
* desktop row and the mobile row report the SAME `data-stage-terminal`, the
* same `data-stage-state`, and the same accessible name.
*
* ── Why the fixtures are permutations of one another ──────────────────────
*
* `MID_PATH_WON` and `WON_LAST` carry the SAME three stages with the same
* labels; only the position of `完成` differs. That is deliberate on two counts:
*
* • A fixture whose `won`-classified stage happens to sit last cannot
* distinguish the two readings of this card — it goes green either way. The
* load-bearing case therefore puts `完成` mid-path, which is exactly the
* card's reproducer (`草稿 → 完成 → 已归档`, `last` at index 2).
* • `WON_LAST` is the COUNTER-PROBE. Without it, "the two rows agree" would
* also be satisfied by never marking a goal terminus at all — the suite
* would pin the wrong invariant and nobody would notice. Since it differs
* from the load-bearing fixture only in WHERE `完成` sits, its green also
* proves the heuristic still fires on `完成`: the mid-path result is a
* positional decision, not `WON_TOKENS` failing to match.
*
* ── The `lost` axis, which the card does not name ─────────────────────────
*
* The rows diverged on a SECOND axis. Desktop renders `stages.slice(firstLostIdx)`
* as a separated alt group and used to hardcode `terminal: 'lost'` on every
* member of it — a POSITIONAL group — while mobile classified each stage on its
* own. So a plain stage sitting after a `lost` one (`草稿 → 失败 → 已归档`)
* painted destructive and announced `closed lost` on desktop and painted plain
* on mobile; and a `won`-classified stage there (`草稿 → 失败 → 完成`) drew
* `'lost'` from one row and `'won'` from the other. Same defect, same fix, so
* it is pinned in the same file.
*
* ── Resolution ────────────────────────────────────────────────────────────
*
* Nothing here resolves through any `dist/`: `../record-path` is this package's
* own source and `@object-ui/react` / `@object-ui/i18n` are mapped to their
* `src` by the root `vitest.config.mts` alias table. 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 '@testing-library/jest-dom';
import { render, cleanup, within, type RenderResult } from '@testing-library/react';
import { I18nProvider } from '@object-ui/i18n';
import { RecordContextProvider } from '@object-ui/react';
import { RecordPathRenderer } from '../record-path';

/**
* The card's reproducer. `WON_TOKENS` matches `完成`, an ordinary mid-path word,
* so index 1 classifies `won` while `last` is index 2.
*/
const MID_PATH_WON = [
{ value: 'draft', label: '草稿' },
{ value: 'done', label: '完成' },
{ value: 'archived', label: '已归档' },
];

/** The counter-probe: the same three stages, with `完成` moved to the end. */
const WON_LAST = [
{ value: 'draft', label: '草稿' },
{ value: 'archived', label: '已归档' },
{ value: 'done', label: '完成' },
];

/** A plain stage sitting AFTER a `lost` one — desktop's positional alt group. */
const PLAIN_AFTER_LOST = [
{ value: 'draft', label: '草稿' },
{ value: 'failed', label: '失败' },
{ value: 'archived', label: '已归档' },
];

/** A `won`-classified stage sitting after a `lost` one: `'lost'` vs `'won'`. */
const WON_AFTER_LOST = [
{ value: 'draft', label: '草稿' },
{ value: 'failed', label: '失败' },
{ value: 'done', label: '完成' },
];

function mount(stages: ReadonlyArray<Record<string, unknown>>, status = 'draft'): RenderResult {
return render(
<I18nProvider config={{ defaultLanguage: 'en', detectBrowserLanguage: false }}>
<RecordContextProvider objectName="crm_quote" recordId="q1" data={{ id: 'q1', status }}>
<RecordPathRenderer schema={{ statusField: 'status', stages } as never} />
</RecordContextProvider>
</I18nProvider>,
);
}

/**
* Both rows are in the DOM at once — they are separated by a CSS breakpoint,
* which jsdom does not apply. Index 0 is desktop, index 1 is mobile.
*/
const rows = (r: RenderResult): HTMLElement[] =>
Array.from(r.container.querySelectorAll('[role="list"]')) as HTMLElement[];

const stagesOf = (row: HTMLElement): HTMLElement[] => within(row).getAllByRole('listitem');

/**
* Everything about a row that `terminal` can move: the classification itself,
* the state it combines with, and the name the pair composes. Desktop renders
* its forward slice then its alt group, and the alt group is the tail of the
* same array, so both rows list the stages in `stages[]` order.
*/
const signature = (row: HTMLElement): string[] =>
stagesOf(row).map((el) =>
[
(el.querySelector('span:last-of-type')?.textContent ?? '').trim(),
el.getAttribute('data-stage-state') ?? '-',
el.getAttribute('data-stage-terminal') ?? '-',
el.getAttribute('aria-label') ?? '-',
].join(' | '),
);

const terminalsOf = (row: HTMLElement): Array<string | null> =>
stagesOf(row).map((el) => el.getAttribute('data-stage-terminal'));

afterEach(() => cleanup());

describe('the two rows read ONE classification (objectui#5998)', () => {
it('a mid-path `won` heuristic hit is classified the same on both rows', () => {
// THE load-bearing case. Before the fix: desktop gave index 1
// `terminal: undefined` (`bg-muted`, "upcoming") because it is not the last
// forward stage, while mobile gave it `terminal: 'won'`
// (`bg-emerald-500/30`, "goal stage, not reached").
const [desktop, mobile] = rows(mount(MID_PATH_WON));

expect(signature(desktop)).toEqual(signature(mobile));
// ...and the value they agree on is the narrowed one: the goal terminus is
// the terminus of the forward path, so a mid-path `完成` is not it.
expect(terminalsOf(desktop)).toEqual([null, null, null]);
});

it('COUNTER-PROBE: the same `完成`, moved last, IS the goal terminus on both rows', () => {
// Without this, "both rows agree" would be satisfied by never marking a goal
// at all. It also proves `WON_TOKENS` still fires on `完成` — the fixture
// above differs from this one only in where that stage sits.
const [desktop, mobile] = rows(mount(WON_LAST));

expect(signature(desktop)).toEqual(signature(mobile));
expect(terminalsOf(desktop)).toEqual([null, null, 'won']);
expect(stagesOf(mobile)[2]).toHaveAccessibleName('完成, goal stage, not reached');
});

it('the goal terminus is not lost when the record has reached the stage before it', () => {
// The `won` classification must not depend on where the record sits, only
// on where the stage sits — checked on both rows for the same reason.
const [desktop, mobile] = rows(mount(WON_LAST, 'archived'));

expect(signature(desktop)).toEqual(signature(mobile));
expect(terminalsOf(desktop)).toEqual([null, null, 'won']);
expect(stagesOf(desktop)[1]).toHaveAttribute('data-stage-state', 'current');
});
});

describe('the `lost` axis diverged too — desktop grouped positionally (objectui#5998)', () => {
it('a plain stage after a `lost` one is not `lost` on either row', () => {
// Before the fix desktop swept `已归档` into the alt group and hardcoded
// `terminal: 'lost'` on it — destructive paint, "closed lost" announcement —
// while mobile left it plain.
const [desktop, mobile] = rows(mount(PLAIN_AFTER_LOST));

expect(signature(desktop)).toEqual(signature(mobile));
expect(terminalsOf(desktop)).toEqual([null, 'lost', null]);
});

it('a `won`-classified stage after a `lost` one is not read as two different terminals', () => {
// The sharpest form of the second axis: the rows disagreed on the VALUE,
// not merely on whether one was present — desktop `'lost'`, mobile `'won'`.
// Neither survives: it is not the last forward stage, and it is not lost.
const [desktop, mobile] = rows(mount(WON_AFTER_LOST));

expect(signature(desktop)).toEqual(signature(mobile));
expect(terminalsOf(desktop)).toEqual([null, 'lost', null]);
});

it('the declared `lost` terminal itself still announces as one on both rows', () => {
// Non-vacuity for the two cases above: the narrowing is confined to stages
// that are not themselves lost-classified.
const [desktop, mobile] = rows(mount(PLAIN_AFTER_LOST));

for (const row of [desktop, mobile]) {
expect(stagesOf(row)[1]).toHaveAccessibleName('失败, closed lost, not reached');
}
});
});
46 changes: 37 additions & 9 deletions packages/plugin-detail/src/renderers/record-path.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -120,7 +120,38 @@ export const RecordPathRenderer: React.FC<RecordPathRendererProps> = ({
const firstLostIdx = stageKinds.findIndex((k) => k === 'lost');
const forwardStages = firstLostIdx === -1 ? stages : stages.slice(0, firstLostIdx);
const lostStages = firstLostIdx === -1 ? [] : stages.slice(firstLostIdx);
const forwardKinds = firstLostIdx === -1 ? stageKinds : stageKinds.slice(0, firstLostIdx);
// ── ONE classification, computed once, read by BOTH rows (objectui#5998) ──
//
// The desktop and the mobile row below render the same `stages[]`, and each
// used to derive its own `terminal` from it independently — desktop from a
// `forwardKinds` slice under an `idx === last` restriction, mobile from
// `stageKinds` under none — so ONE stage of ONE record could paint (and,
// since objectui#5957, announce) two different ways chosen by nothing but
// viewport width. They diverged on TWO axes, not only the one the card named:
//
// 1. `idx === last`. `WON_TOKENS` matches `完成`, an ordinary mid-path word,
// so `草稿 → 完成 → 已归档` classified index 1 as `won`: desktop declined
// it (not the last forward stage), mobile marked it the goal.
// 2. The lost slice. Desktop hardcoded `terminal: 'lost'` onto EVERY stage
// of the separated alt group — a group defined POSITIONALLY, as
// `stages.slice(firstLostIdx)` — while mobile classified each stage on
// its own. So in `草稿 → 失败 → 已归档`, desktop painted `已归档`
// destructive and announced it `closed lost`; mobile painted it plain.
//
// Both are settled here, in one array both rows index, so a future divergence
// is IMPOSSIBLE rather than merely absent — two rows that happen to agree
// would leave the defect one edit away. The rule: `lost` is a property of the
// STAGE; `won` is the GOAL TERMINUS, so it is the last forward stage or it is
// nothing. Positional grouping stays a LAYOUT concern and no longer overrides
// what a stage is.
//
// Conservative on both axes: this can only ever STOP marking a stage as a
// terminus, never start. No stage gains a `terminal` on either row that it
// did not already carry on that row.
const lastForwardIdx = forwardStages.length - 1;
const stageTerminals: Array<'won' | 'lost' | undefined> = stageKinds.map((kind, idx) =>
kind === 'won' ? (idx === lastForwardIdx ? 'won' : undefined) : kind,
);

let currentIdx = stages.findIndex((s) => s.value === current);
if (currentIdx < 0) currentIdx = -1;
Expand DownExpand Up@@ -247,8 +278,6 @@ export const RecordPathRenderer: React.FC<RecordPathRendererProps> = ({
</div>
);

const last = forwardStages.length - 1;

return (
<div className={cn('w-full', className)} {...designer}>
{/* Desktop: forward rail → optional lost-alt group */}
Expand All@@ -261,12 +290,11 @@ export const RecordPathRenderer: React.FC<RecordPathRendererProps> = ({
{forwardStages.map((stage, idx) => {
const isCompleted = !currentInLost && currentIdx >= 0 && idx < currentIdx;
const isCurrent = !currentInLost && idx === currentIdx;
const isWonTerminus = forwardKinds[idx] === 'won' && idx === last;
return renderStage({
key: `${stage.value}-${idx}`,
stage,
state: isCurrent ? 'current' : isCompleted ? 'completed' : 'upcoming',
terminal: isWonTerminus ? 'won' : undefined,
terminal: stageTerminals[idx],
className: 'flex-1 min-w-0',
labelClassName: 'text-center truncate',
});
Expand DownExpand Up@@ -309,7 +337,7 @@ export const RecordPathRenderer: React.FC<RecordPathRendererProps> = ({
key: `${stage.value}-lost-${lIdx}`,
stage,
state: absIdx === currentIdx ? 'current' : 'upcoming',
terminal: 'lost',
terminal: stageTerminals[absIdx],
className: 'shrink-0',
labelClassName: 'text-center whitespace-nowrap',
});
Expand All@@ -325,15 +353,15 @@ export const RecordPathRenderer: React.FC<RecordPathRendererProps> = ({
aria-label={(schema.aria as any)?.label || t('detail.pathLabel')}
>
{stages.map((stage, idx) => {
const kind = stageKinds[idx];
const isLost = kind === 'lost';
const terminal = stageTerminals[idx];
const isLost = terminal === 'lost';
const isCompleted = !isLost && !currentInLost && currentIdx >= 0 && idx < currentIdx;
const isCurrent = idx === currentIdx;
return renderStage({
key: `${stage.value}-${idx}-m`,
stage,
state: isCurrent ? 'current' : isCompleted ? 'completed' : 'upcoming',
terminal: kind,
terminal,
className: 'shrink-0',
labelClassName: 'whitespace-nowrap',
});
Expand Down
Loading