docs(components): the injected-editor commit justification is stale — correct it, and pin what Tab-out actually does - #6912
Conversation
…nd pin what actually happens (#6859) The data table exits a host-injected inline cell editor through a document-level `pointerdown` listener. The comment at `injectedEditorElRef` justified that with "the injected widgets (text, number, date, lookup, …) have no such handler". That has not been true since #6780 / #6802: `onBlur` is a declared DOM pass-through key, `toDomProps` forwards it, and every widget `EDIT_WIDGETS` resolves to delivers it to a real control — 26 by their own spread, `UserField` by delegating to `LookupField`. A source audit read the same absence (`focusout` / `onBlurCapture` / `relatedTarget` nowhere in the file) as silent data loss on Tab-out. Driven in a real Chromium against the real widgets, it is not: the host wires each widget's `onChange` to the table's `stage`, so a typed value is in `pendingChanges` from the first keystroke, before any exit event. Tabbing out leaves the cell in edit mode until Enter, Escape or an outside pointer press — a wart, not a lost edit. So the listener stays and the record is corrected. The real reason it is needed is that nothing on this seam ever hands the widget an `onBlur`: the wrapper carries `onKeyDown` alone, the `renderCellEditor` context has no DOM-props slot, and `FieldEditWidget` forwards `autoFocus` and nothing else out of the DOM block. Both facts are now pinned: - `data-table-injected-editor-focus-6859.test.tsx` — staging happens before any exit event; Tab neither commits nor exits; the value reads back intact. A built-in editor committing on blur is the control. - `injectedEditorHostDomChannel-6859.test.tsx` — the text / date / lookup widgets the stale sentence named really do carry the host set, `onBlur` included, onto a focusable control. `GeolocationField` is the control: it must answer both ways in one render, because its longitude box deliberately takes no host DOM props. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
Review — ACCEPT, pending CI. The gate did its job: the hypothesis is falsified and the record is corrected.
This is the outcome the dispatch named as complete and valuable, and it arrived the right way round: no code was written until the runtime question had an answer, and the answer was "no data loss". ⭐ Driven in real Chromium via Playwright, with the harness stated — not jsdom described as a browser, which was the specific substitution I asked to be disclosed rather than blurred. ⭐ The most valuable finding is why the source reading was wrongThe R+48 audit scanned for The mechanism is a staging path, not a commit path:
⇒ A typed value is durable from the first keystroke, independent of how edit mode is exited. The ⭐ And the audit was half right, which the PR says rather than burying: Tab-out genuinely has no exit path — the cell stays in edit mode until Enter, Escape, or an outside pointer press. That is a wart, and it is now written down at the wrapper instead of waiting to be rediscovered a fourth time. The enumeration carries its own controlOver ⭐ And the two counter-examples #6859 named are outside the population entirely:
⭐ #6909 inverts the card's own premiseThe card is titled "field widgets now deliver a host ⇒ A host Ablation 2 is the right way to price an optionFour legs, predicted then observed row for row, each mutation proven on disk and each restore proven both ways. ⭐ Leg 2 runs the contemplated swap itself as an experiment — a wrapper The reasoning for not taking the swap now is sound and measured, not reluctant: a wrapper
|
✅ 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
|
CI is terminal and green at
|
| count | |
|---|---|
| total check runs | 30 |
| success | 27 |
| skipped (never ran) | 3 |
| failure / cancelled / timed out | 0 |
| still in progress | 0 |
The three skipped are Test (coverage), Test (coverage shard …) and dependabot —
conditional jobs, NOT MEASURED rather than green, and none of them is a required gate.
All four Test (shard N/4) jobs succeeded, which is where packages/fields runs, so the
181910 ms timeout seen locally is not present on CI. Reading: environmental (transform
saturation on a shared container), consistent with the isolated re-run of that same file
on this same commit passing 15/15 in 7.00 s.
mergeable_state: clean.
Landing path
Not clause ②: the diff is comments, two new test files, and a changeset with empty
frontmatter — no contract accept/reject behaviour changes and no published surface is
widened. No governed surface is touched (docs/adr/**, .claude/**, skills/**,
AGENTS.md, CLAUDE.md are all absent from the diff), and nothing under
content/docs/releases/. Review verdict on the diff was ACCEPT.
Marking ready and arming auto-merge. Enqueue will be confirmed from the
gh-readonly-queue/main/pr-6912-* ref, not from the API's return value.
Generated by Claude Code
2 participants
Fixes #6859
TL;DR — the audit hypothesis is falsified, measured in a real browser
The dispatch order asked one question before any code: type into an injected cell
editor, press Tab, read the cell back — is the value there or gone?
It is there. No data loss. So this PR does not change behaviour: it corrects the
record and pins the facts, which is what #6859 asked for in its own "if the answer is
keep the listener" branch.
The measurement (real Chromium, not jsdom)
Driven with Playwright against the pre-installed Chromium
(
/opt/pw-browsers/chromium-1194/chrome-linux/chrome), on a throwaway Vite entry inapps/console(deleted before commit — nothing under that path is in this diff) thatrenders the REAL
data-tablerenderer with an injected editor mirroringObjectGrid.renderCellEditorexactly:FieldEditWidget, with non-discrete types wiredonChangetostageand discrete ones tocommit.Sequence per column: click the cell, type, press Tab, then Tab four more times, then
click outside.
TextField)TYPED_TEXTDateField)Sep 15, 2026NumberField)77Ground truth on the first run: after Save All,
onBatchSavereceived[{"rowIndex":0,"changes":{"name":"TYPED_VALUE"},...}].The path the source reading missed — and it is not a commit path
It is a staging path, which is why a scan for
focusout/onBlurCapture/relatedTargetcould not see it:data-table.tsx:1558—stageEditwrites straight intopendingChangeswithoutclosing the editor.
packages/plugin-grid/src/ObjectGrid.tsx:3693— the host wires every non-discretewidget's
onChangetoctx.stage(discrete types go toctx.commit).So a typed value is durable from the first keystroke, independent of how edit mode is
exited. The
pointerdownlistener exits edit mode; it never rescued the value.The audit was right about one half: Tab-out genuinely has no exit path, and the cell
stays in edit mode until Enter, Escape, or an outside pointer press. That is a wart, not
a lost edit, and it is now written down at the wrapper instead of being rediscovered.
The enumeration (constraint 1 — counted, not assumed)
Over
EDIT_WIDGETSinpackages/fields/src/FieldEditWidget.tsx(31 type keys, 27distinct components):
toDomPropsin their own module.UserField— has zero direct calls and delivers by handing its wholeprops object to
LookupField. That zero is the control: the query demonstrablydiscriminates, so "26 of 27" is a measurement rather than a tautology.
onBlur. The strips that exist areid,name,disabled,className,aria-invalid,aria-labelledby—onBlursurvives all of them.The two counter-examples #6859 named are outside this population:
ObjectRefField(object-ref) andFileField(file) are both inINLINE_EXCLUDED_FIELD_TYPES, so neither is ever an injected editor. Both stripnameonly;
onBlursurvives there too.And the fact that makes "just pass onBlur" bigger than it looks
FieldEditWidgetdeclaresFieldWidgetComponentProps— which includes the whole DOMpass-through block — but destructures and forwards
field/value/onChange/readonly/autoFocusand nothing else. A hostonBlurcannot reach any widgetthrough the in-repo injected-editor seam today, whatever the widgets support. Filed
separately (see below); not folded in, since it is a different defect class from this
card.
GeolocationField's longitude box
Not touched, per the order. It is now pinned as a deliberate exception by the new
fields test, with a comment saying so, precisely so nobody "repairs" it later.
What changed
packages/components/src/renderers/complex/data-table.tsx— comments only. TheinjectedEditorElRefjustification, thepointerdowneffect's restatement of it, anda new note at the wrapper recording that Tab does not exit and why that costs nothing.
The corrected comment now says: the widgets do deliver
onBlur; the listener is stillneeded because nothing on this seam ever hands them one; and its job is exiting
edit mode, not rescuing values.
packages/components/src/__tests__/data-table-injected-editor-focus-6859.test.tsx— new.packages/fields/src/__tests__/injectedEditorHostDomChannel-6859.test.tsx— new..changeset/6859-injected-editor-commit-comment.md— empty frontmatter (declares"releases nothing"), with the plain-language reason.
No published type or prop surface is widened. No test skipped, disabled or quarantined.
Nothing under
content/docs/releases/.Verification
Run from the repo root (never
pnpm --filter PKG test— this repo's documentedzero-match false-green trap), at
a80e7a89, the final commit.Suites
pnpm exec vitest run packages/components/ packages/fields/pnpm exec vitest run packages/components/src/__tests__/data-table-injected-editor-focus-6859.test.tsxpnpm exec vitest run packages/fields/src/__tests__/injectedEditorHostDomChannel-6859.test.tsxThe one failure is not from this diff.
packages/fields/src/__tests__/LocationField.range.test.tsx— "does not emit 999, 999" — timed out at 181910 ms inside the saturated parallel run
on a shared container. Re-run in isolation on this same commit: 15 tests pass in
7.00 s. That is the flake shape AGENTS.md documents (an unbounded module load billed to
a bounded window under transform saturation), and this diff touches nothing that file
reads — it adds a widget test and edits comments in another package.
Type check
pnpm --filter @object-ui/components run type-check→ exit 0.pnpm --filter @object-ui/fields run type-check→ exit 0.Both packages spell it
type-checkwith the hyphen, and both chaintsc -p tsconfig.test.json. Because "typecheck is clean" is worth nothing if it neversaw the new files,
--listFileswas used to confirm each new test file is a programinput (1 hit each) rather than assumed from the exclude list.
Ablation — predicted, then observed, row for row
Each mutation was proven on disk by anchored counts before the run, and each restore by
git diff HEADempty andhash-objectequal to the HEAD blob, with an absolute-pathtrapon EXIT/INT/TERM. Nodistis involved: both suites reach the code throughsource, so there is no stale-artifact leg.
stageEditstops writingpendingChangesonBlur(the swap #6859 contemplated)TextFieldstops spreadingtoDomPropstextrow onlytextrow only, 3 passeddate,lookup, Geolocation controltext,date,lookupAblation 2 is the interesting one: it is the p3 swap, run as an experiment. It turns test
C red exactly as predicted, which is the honest statement that the swap is a
behaviour change and not a refactor.
Restore proof, final:
git diff HEAD --statandgit status --porcelainboth empty;hash-objectequal to the HEAD blob for all four mutated files.Lint — base versions of the same files, not a bare count
Only one file in this diff exists on
origin/main. It was linted at HEAD, then the baseblob was checked out in place and linted again (blob ids verified both ways).
data-table.tsxorigin/main)Every apparent difference is the same rule at a line number shifted by the comment block
this PR inserts. Nothing new, nothing fixed — which is the expected reading for a
comment-only edit.
New files:
data-table-injected-editor-focus-6859.test.tsx0 errors / 2 warnings,injectedEditorHostDomChannel-6859.test.tsx0 errors / 3 warnings — allno-explicit-anyon test casts, matching the convention of the neighbouring suites.Narrowing declared. The repo-wide lint belongs to CI. The three pieces that make this
a measurement rather than a skip: (1) eslint's own config was asked, via
ESLint.isPathIgnored, whether each of the three files is in scope — all three answer"not ignored"; (2) the file count comes from
--format json, not from a guess; (3)eslint.config.jsdeclares noproject/projectService, so linting is not type-awareand this diff cannot move the verdict on any file it does not touch.
Out of scope, filed
autoFocus— a host'sonBlur/onFocus/id/name/tabIndexsilently never reach the control #6909 —FieldEditWidgetdeclares the whole DOM pass-through block but forwards onlyautoFocus, so a host'sonBlur/onFocus/id/name/tabIndexsilentlynever reach the control. Unassigned. Found here, deliberately not folded in: different
defect class, and it is the reason the "just pass onBlur" option is bigger than finding(components): the data-table's document-level pointerdown commit exists because "injected widgets have no blur handler" — field widgets now deliver a host
onBlur#6859assumed.
What is deliberately NOT here
The swap itself. With Tab-out measured non-lossy, the trade the order framed as
asymmetric is not: a wrapper
onBlurmust re-derive the portal guards thepointerdownlistener already carries — the Radix popper and dialog checks that keep a lookup popover
from committing the cell out from under the user — and must additionally handle a null
relatedTarget. That is more surface, not less, in exchange for a wart nobody hasreported. If the maintainer wants the keyboard exit anyway, it deserves its own graded
card with the portal cases as its verification bar. Ablation 2 above is the first data
point for that decision.
Generated by Claude Code
Generated by Claude Code