fix(plugin-detail): render record:path as a readout, not a fake segmented control - #5917
Merged
Merged
Conversation
…mented control The record page drew the object's lifecycle from the `stageField` role as filled, shadowed, equal-width pills — a segmented button group sitting where a CRM user reaches for the stage control — with nothing behind it. Measured in a browser on a shipped build: `role="listitem"`, `cursor: auto`, `tabindex` null, no ancestor button/tab/a, and a full pointer sequence left the record's status untouched. No write path reaches this renderer: its only channel is `useRecordContext()`, whose value carries `data` / `refresh` / `headerSystemActions` / `onToggleFavorite` and no record-field mutation. Editing runs through `record:details`' InlineEditProvider + InlineEditSaveBar or an action via `useActionEngine`; neither is reachable here. So the visual promise is withdrawn rather than honoured. Each stage renders as a thin decorative rail segment with its label as plain text beneath — the vocabulary app-shell's approval step readout already uses. Removed: per-stage filled pill, shadow, ring, bordered chip, equal-width tap target. Unchanged: which stage is current, the travelled/untravelled distinction, the completed check, the separated lost-terminal group, and the accessible semantics (role="list"/"listitem", no tab stop) which were already correct for a readout. `data-stage-state` / `data-stage-terminal` / `data-stage-rail` carry the state colour used to be the only carrier of, so the classification is assertable without reading CSS. Click-to-advance is deliberately not built here; it is a separate feature with its own appetite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EuPCi56cnGyykygi3z9w4m
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-zhuang
marked this pull request as ready for review
August 24, 2026 00:51
This was referenced Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5768
Which branch the measurement chose
The dispatch asked, before anything else: does a write path for the stage field already exist and reach this surface? It does not, and that decided the shape.
record:path's only channel isuseRecordContext(). The whole of what that context offers (RecordContextValue,packages/react/src/context/RecordContext.tsx) isobjectName/recordId/dataSource/data/objectSchema/refresh/headerSystemActions/isFavorite/onToggleFavorite/embedded— no record-field mutation of any kind. The two write paths that do exist on a record page both terminate elsewhere:record:detailsmounts<InlineEditProvider>+<InlineEditSaveBar>; the save isdataSource.update(..., { ifMatch: data.updated_at }), scoped to that renderer's session (record-details.tsx:63-67);record-quick-actions.tsxgoes throughuseActionEngine'sexecuteAction, withusePermissionsgating.Neither is reachable from this component. Connecting one would mean opening a new write channel plus permission gating, validation and OCC — the medium-appetite feature the maintainer explicitly deferred to its own card. So this PR takes the third branch: the element stops claiming to be a control.
The evidence, re-measured at the live source
The card was measured in a running browser against a shipped build (HotCRM
crm_quote; generic record chrome, so every object declaring astageFieldhas it):Every one of those readings is correct for a status readout. The accessible semantics were never the defect —
role="list"/role="listitem"with no tab stop is what a readout should be, and this PR does not move them. The defect was that the pixels said the opposite: filled, shadowed, equal-width pills — a segmented button group — sitting exactly where a CRM user reaches for the stage control.What changed
Each stage now renders as a thin decorative rail segment with its label as plain text beneath it, which is the vocabulary app-shell's approval step readout already uses (
RecordApprovalsPanel— marker, rail, bare label, weight for "current", never a filled surface).Gone: per-stage filled pill (
rounded-xl+bg-primary/bg-mutedsurface),shadow-sm,ring-1, the borderedrounded-fullchips on the mobile row, and the equal-width tap target.Unchanged: which stage is current (
aria-current="step", plus type weight and an accent rail), the travelled/untravelled distinction, the completed check, the separatedlost-terminal group, and every accessible semantic.Three DOM attributes now carry state that colour used to be the sole carrier of, so the classification is assertable without reading CSS:
data-stage-state(completed|current|upcoming),data-stage-terminal(won|lost),data-stage-railon the decorative indicator.⛔ Not built here: click-to-advance. No handler was added — a no-op handler or a toast would trade a silent false promise for a noisy one.
Verification
All commands below ran at the final commit
292d2f73f, under the shared verify lock, with exit codes captured before any pipe.Gate verdict lines, each quoted from the gate's own output:
The new suite asserts accessible semantics and structure, never CSS — the test DOM resolves no Tailwind, so
getComputedStylethere would answer nothing about what a user sees.Reverse verification
Both ablations were confirmed on disk by grepping the injected and the removed text (never an editor's exit code), each ran under a
trap ... EXIT INT TERMrestore, and the direction was predicted before running. No rebuild was needed and none was skipped: the suite imports the subject relatively (../record-path), and its one cross-package import (@object-ui/react) is mapped by the rootvitest.config.mtsalias table topackages/react/src, so the resolution path never touches anydist/.data-stage-railfrom the indicator1 failed | 8 passed— the failure is B4<button>into each stage2 failed | 7 passed— the failures are B1 and B2Restore leg: file byte-identical to the pre-ablation copy (single unique sha256), no ablation residue, suite back to
17 passed (17), and the union above re-run green afterwards.Legs that do not discriminate this change, named rather than hidden: B1 (no interactive role), B2 (no tab stop), B3 (inert on a full pointer sequence) were already true before this PR — they re-measure the browser's readings at the live source and pin the surface against the wrong repair, i.e. bolting a control onto a surface with no write path behind it. Ablating the styling leaves all three green. B4 is the only discriminating leg: a rail's indicator is a separate, text-free element, while a pill is its own label's surface and cannot have one.
Lint narrowing, declared
The repo-wide scan is CI's run. Locally the whole touched package was linted (
eslint .inpackages/plugin-detail, exit 0), and the narrowing to that package excludes nothing relevant, on three pieces of evidence:eslint --format jsonon the changed paths reports which it actually linted; the changeset.mdcame back flagged as ignored by config, both source files as linted.--format json— 3 paths submitted, 3 reported, 0 errors.eslint.config.jsdeclares noprojectServiceand noparserOptions.project(0 matches), so no type-aware program exists and a file's verdict depends only on that file plus the shared config. This diff therefore cannot move the verdict of any file it does not touch.Warning parity was held deliberately: baseline
record-path.tsxlinted 0 errors / 15 warnings; the rewrite first came in at 16, and the one addedno-explicit-anywas removed (renderStageonly ever readsstage.label), returning it to 0 / 15.Out of scope, filed separately
While in the file I measured a distinct accessibility defect and did not fold it in: travelled / upcoming /
loststages are distinguished by colour alone, because the ✓ and ✗ glyphs arearia-hidden, so the state never reaches the accessible name (WCAG 2.2 SC 1.4.1). Pre-existing, and a proper repair needs new user-facing strings and i18n keys. Filed unassigned as #5916.Generated by Claude Code