From fdc8aecd03f05b79fd8480bef84ab195b32b7b59 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 13:05:56 +0000 Subject: [PATCH] fix(plugin-detail): give record:path one stage classification both rows read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The desktop and mobile rows rendered the same stages[] and each derived its own `terminal` from it, so one stage of one record could paint — and, since the accessible name is derived from the same value, announce — two different ways chosen by viewport width alone. They diverged on two axes. Mid-path goal: WON_TOKENS matches the ordinary word 完成, so 草稿 → 完成 → 已归档 classified index 1 as won; desktop declined it (not the last forward stage) while mobile marked it the goal. Lost slice: desktop hardcoded terminal: 'lost' on every member of its positionally defined alt group (stages.slice(firstLostIdx)) while mobile classified each stage on its own, so a plain stage after a lost one announced closed lost on one row and plain on the other. Both rows now index a single stageTerminals array computed once: lost is a property of the stage, won is the goal terminus and so is the last forward stage or nothing, and positional grouping stays a layout concern. Narrowed on both axes and never widened — no stage gains a terminal on either row that it did not already carry there. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L --- .../5998-record-path-one-classification.md | 32 +++ ...ecord-path.crossRowClassification.test.tsx | 209 ++++++++++++++++++ .../src/renderers/record-path.tsx | 46 +++- 3 files changed, 278 insertions(+), 9 deletions(-) create mode 100644 .changeset/5998-record-path-one-classification.md create mode 100644 packages/plugin-detail/src/renderers/__tests__/record-path.crossRowClassification.test.tsx diff --git a/.changeset/5998-record-path-one-classification.md b/.changeset/5998-record-path-one-classification.md new file mode 100644 index 0000000000..0f17f158a1 --- /dev/null +++ b/.changeset/5998-record-path-one-classification.md @@ -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. diff --git a/packages/plugin-detail/src/renderers/__tests__/record-path.crossRowClassification.test.tsx b/packages/plugin-detail/src/renderers/__tests__/record-path.crossRowClassification.test.tsx new file mode 100644 index 0000000000..f22ab93c57 --- /dev/null +++ b/packages/plugin-detail/src/renderers/__tests__/record-path.crossRowClassification.test.tsx @@ -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>, status = 'draft'): RenderResult { + return render( + + + + + , + ); +} + +/** + * 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 => + 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'); + } + }); +}); diff --git a/packages/plugin-detail/src/renderers/record-path.tsx b/packages/plugin-detail/src/renderers/record-path.tsx index 36e8a76b0a..844c744150 100644 --- a/packages/plugin-detail/src/renderers/record-path.tsx +++ b/packages/plugin-detail/src/renderers/record-path.tsx @@ -120,7 +120,38 @@ export const RecordPathRenderer: React.FC = ({ 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; @@ -247,8 +278,6 @@ export const RecordPathRenderer: React.FC = ({ ); - const last = forwardStages.length - 1; - return (
{/* Desktop: forward rail → optional lost-alt group */} @@ -261,12 +290,11 @@ export const RecordPathRenderer: React.FC = ({ {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', }); @@ -309,7 +337,7 @@ export const RecordPathRenderer: React.FC = ({ key: `${stage.value}-lost-${lIdx}`, stage, state: absIdx === currentIdx ? 'current' : 'upcoming', - terminal: 'lost', + terminal: stageTerminals[absIdx], className: 'shrink-0', labelClassName: 'text-center whitespace-nowrap', }); @@ -325,15 +353,15 @@ export const RecordPathRenderer: React.FC = ({ 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', });