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
21 changes: 21 additions & 0 deletions .changeset/6859-injected-editor-commit-comment.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
---

No behaviour change, and deliberately so: this is the record being corrected, not the
renderer (objectui#6859).

The data table's document-level `pointerdown` listener — the one that exits a host-injected
inline cell editor when you click away — justified itself with "the injected widgets (text,
number, date, lookup, …) have no such handler". That has not been true since objectui#6780 /
#6802: `onBlur` is a declared DOM pass-through key, and all 27 widgets reachable as an inline
editor deliver it to a real control (26 spread `toDomProps` themselves, `UserField` delegates
to `LookupField`).

A source audit read the same absence as silent DATA LOSS on Tab-out. It is not. Driven in a
real browser against the real widgets, a value typed into a text, date or number cell editor
survives tabbing away, and reads back intact: the host wires each widget's `onChange` to the
table's `stage`, so every keystroke is already in `pendingChanges` while the editor is still
open. The listener exits EDIT MODE; it never rescued the value. Tabbing out does leave the
cell in edit mode until Enter, Escape, or a pointer press outside — a wart, not a lost edit.

The comment now says all of that, and both facts are pinned by tests so they cannot rot back.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
/**
* 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.
*/

/**
* CHARACTERIZATION — what actually happens when focus leaves a HOST-INJECTED
* cell editor (objectui#6859).
*
* ## Why this file exists
*
* `data-table.tsx` exits an injected editor through a document-level
* `pointerdown` listener, and the comment at `injectedEditorElRef` used to
* justify that with "the injected widgets (text, number, date, lookup, …) have
* no such handler". That justification is stale — `onBlur` is a declared DOM
* pass-through key every inline-edit widget forwards (objectui#6780, #6802) —
* and correcting a comment leaves nothing behind that can rot loudly. A source
* audit then read the same absence as a DATA-LOSS defect: no `focusout`, no
* `onBlurCapture`, no `relatedTarget` anywhere in the file ⇒ Tab out of an
* injected editor must silently drop the typed value.
*
* It does not, and this file is the measurement that says why. The value never
* depends on the exit event: the host wires the widget's `onChange` to `stage`,
* so every keystroke is already in `pendingChanges` while the editor is still
* open. The `pointerdown` listener exits EDIT MODE; it does not rescue values.
*
* The same sequence was driven in a real Chromium against the real
* `@object-ui/fields` widgets (text / date / number) before this file was
* written; these are the jsdom pins for CI.
*
* ## The control
*
* Test A is the control and must stay green: a BUILT-IN editor DOES commit on
* focus loss. Without it, test C's negative ("Tab commits nothing") would pass
* just as well on a harness that cannot observe a commit at all.
*/
import { describe, it, expect, vi } from 'vitest';
import { fireEvent, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import '@testing-library/jest-dom';
import React from 'react';
import { renderComponent } from './test-utils';
// Module scope, not a hook — see object-ui/no-dynamic-import-in-test-hook.
import '../renderers';

const baseSchema = {
type: 'data-table' as const,
editable: true,
singleClickEdit: true,
columns: [
{ header: 'Name', accessorKey: 'name', editable: false },
{ header: 'Qty', accessorKey: 'qty', type: 'number' },
],
data: [{ id: '1', name: 'row-one', qty: '' }],
} as any;

/**
* The injected editor exactly as the in-repo host builds it
* (`ObjectGrid.renderCellEditor`): a real control whose `onChange` is wired to
* the context's `stage` — non-discrete field types stage, they do not commit.
* `packages/components` does not depend on `@object-ui/fields`, so the wiring
* is reproduced rather than imported; the browser run linked above is what
* pins it to the real widgets.
*/
const stagingEditor = ({ column, value, stage }: any) =>
column.accessorKey === 'qty' ? (
<input
data-testid="injected-editor"
value={value ?? ''}
onChange={(e) => stage(e.target.value)}
/>
) : null;

/** A tabbable element outside the table, so `userEvent.tab()` has somewhere to go. */
function withOutsideTabStop(): HTMLButtonElement {
const btn = document.createElement('button');
btn.id = 'outside-tab-stop';
btn.textContent = 'outside';
document.body.appendChild(btn);
return btn;
}

describe('data-table — focus loss on a host-injected cell editor (objectui#6859)', () => {
it('A) CONTROL: a BUILT-IN editor DOES commit when focus leaves it', async () => {
// Proves the harness can observe a commit driven by focus loss at all, so
// the negative results below are measurements and not dead probes.
const onCellChange = vi.fn();
const { container } = renderComponent({ ...baseSchema, onCellChange });

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);

const input = qtyCell.querySelector('input') as HTMLInputElement;
expect(input).toBeTruthy();
fireEvent.change(input, { target: { value: '42' } });
fireEvent.blur(input);

expect(onCellChange).toHaveBeenCalledWith(0, 'qty', '42', expect.anything());
});

it('B) an injected editor stages every keystroke — the value is pending BEFORE any exit event', () => {
// This is the path the source audit could not see, and the reason Tab-out
// is not lossy: `stage` writes straight into `pendingChanges` while the
// editor is still open and still focused.
const { container, getByText, queryByText } = renderComponent({
...baseSchema,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
// Nothing pending yet — the toolbar's save affordance is the readout.
expect(queryByText(/Save All/i)).toBeNull();

fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
expect(editor).toBeTruthy();
fireEvent.change(editor, { target: { value: 'TYPED' } });

// Still in edit mode, nothing committed — and the value is already staged.
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
expect(getByText(/Save All/i)).toBeInTheDocument();
});

it('C) tabbing out of an injected editor neither commits nor leaves edit mode', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
expect(document.activeElement).toBe(editor);
await userEvent.tab();

// Focus really left the editor …
expect(document.activeElement).not.toBe(editor);
// … and nothing committed: no exit, no onCellChange. This is the fact the
// corrected comment at `injectedEditorElRef` now states, and the reason
// the document-level pointerdown listener is still load-bearing.
expect(onCellChange).not.toHaveBeenCalled();
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
} finally {
outside.remove();
}
});

it('D) …and the typed value is still there — the outside pointer press commits exactly it', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
await userEvent.tab();
// A pointer press truly outside is what exits the editor today.
fireEvent.pointerDown(outside);

await waitFor(() =>
expect(onCellChange).toHaveBeenCalledWith(0, 'qty', 'TYPED', expect.anything()),
);
// Edit mode is over and the cell reads back what was typed — nothing lost
// across the Tab-out.
await waitFor(() =>
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeNull(),
);
expect(qtyCell.textContent).toContain('TYPED');
} finally {
outside.remove();
}
});
});
55 changes: 46 additions & 9 deletions packages/components/src/renderers/complex/data-table.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1002,11 +1002,37 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// don't double-commit (Enter) or resurrect a cancelled value (Escape).
const skipBlurSaveRef = useRef(false);
// DOM node of a host-injected widget editor (rendered via `renderCellEditor`),
// captured while it's mounted. The built-in `<input>` editors commit via their
// own onBlur, but the injected widgets (text, number, date, lookup, …) have no
// such handler — a document-level pointerdown listener (see below) uses this
// node to detect click-outside and commit them. Null ⇒ no injected editor is
// active (a built-in editor, or nothing, is showing).
// captured while it's mounted, so the document-level pointerdown listener
// below can tell "inside this editor" from "outside" and exit edit mode.
// Null ⇒ no injected editor is active (a built-in editor, or nothing, is
// showing).
//
// This used to justify itself with "the injected widgets (text, number, date,
// lookup, …) have no such handler". That claim is no longer true and is no
// longer the reason (objectui#6859). `onBlur` is a DECLARED DOM pass-through
// key — named in `FieldWidgetDomProps` (`@object-ui/fields`), named in
// `SDUI_DOM_PASS_THROUGH_KEYS` (`@object-ui/core`), forwarded by
// `toDomProps` — and every widget reachable as an inline editor spreads that
// whitelist onto a real control (26 of the 27 components in `EDIT_WIDGETS`
// call `toDomProps` directly; `UserField` delegates its whole props object to
// `LookupField`, which does). The five widgets that own a blur handler now
// COMPOSE the host's rather than overriding it (objectui#6780, #6802).
//
// The listener is still needed, for a different reason: NOTHING EVER HANDS
// THE WIDGET ONE. The wrapper below carries `onKeyDown` alone, and the
// context object `renderCellEditor` receives — `{ column, row, value, stage,
// commit, cancel }` — has no DOM-props slot to put an `onBlur` in. The
// in-repo factory behind that seam, `@object-ui/fields`' `FieldEditWidget`,
// forwards `autoFocus` and nothing else out of the DOM block, so a host
// handler could not reach the control through it even if one were passed.
//
// Note also what the listener is NOT load-bearing for. Its job is exiting
// EDIT MODE, not rescuing the value: injected widgets stage on every change
// (the host wires the widget's `onChange` to `stageEdit` below), so a typed
// value is already in `pendingChanges` before any exit event — measured in a
// real browser on the text, date and number editors for objectui#6859.
// Retiring this listener would strand cells in edit mode; it would not drop
// edits.
const injectedEditorElRef = useRef<HTMLDivElement | null>(null);
// Snapshot of the active cell's pending value when editing began, so Escape /
// cancel can revert this session's changes. Injected widgets stage on every
Expand DownExpand Up@@ -1728,10 +1754,12 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {

// Commit a host-injected widget editor on click-outside (objectui#2321).
//
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur), but
// the widgets injected through `renderCellEditor` (text, number, date, lookup,
// …) have no such handler, so without this they stay stuck in edit mode when
// the user clicks away. A capture-phase document listener (capture so a cell's
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur). The
// widgets injected through `renderCellEditor` (text, number, date, lookup, …)
// never receive one — not because they cannot deliver it (they can, and do:
// see `injectedEditorElRef` above and objectui#6859) but because nothing on
// this seam passes it to them — so without this they stay stuck in edit mode
// when the user clicks away. A capture-phase document listener (capture so a cell's
// own `stopPropagation` can't hide it) commits the staged value and exits edit
// mode when the pointer goes down truly outside the editor — but NOT inside a
// Radix overlay the widget itself opened (a lookup popover / record-picker
Expand DownExpand Up@@ -2302,6 +2330,15 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// picker's `<button>` trigger or a multi-line
// textarea it's left alone so Enter opens the
// dropdown / inserts a newline as usual.
//
// Tab is deliberately NOT in that list, and
// tabbing out therefore does not leave edit
// mode — measured, objectui#6859. It costs
// nothing: the widget has already staged
// every keystroke into `pendingChanges`, so
// the value is safe; the cell simply stays
// open until Enter, Escape, or a pointer
// press outside closes it.
return (
<div
ref={(n) => { injectedEditorElRef.current = n; }}
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
docs(components): the injected-editor commit justification is stale — correct it, and pin what Tab-out actually does by claude[bot] · Pull Request #6912 · objectstack-ai/objectui · GitHub
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
21 changes: 21 additions & 0 deletions .changeset/6859-injected-editor-commit-comment.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
---

No behaviour change, and deliberately so: this is the record being corrected, not the
renderer (objectui#6859).

The data table's document-level `pointerdown` listener — the one that exits a host-injected
inline cell editor when you click away — justified itself with "the injected widgets (text,
number, date, lookup, …) have no such handler". That has not been true since objectui#6780 /
#6802: `onBlur` is a declared DOM pass-through key, and all 27 widgets reachable as an inline
editor deliver it to a real control (26 spread `toDomProps` themselves, `UserField` delegates
to `LookupField`).

A source audit read the same absence as silent DATA LOSS on Tab-out. It is not. Driven in a
real browser against the real widgets, a value typed into a text, date or number cell editor
survives tabbing away, and reads back intact: the host wires each widget's `onChange` to the
table's `stage`, so every keystroke is already in `pendingChanges` while the editor is still
open. The listener exits EDIT MODE; it never rescued the value. Tabbing out does leave the
cell in edit mode until Enter, Escape, or a pointer press outside — a wart, not a lost edit.

The comment now says all of that, and both facts are pinned by tests so they cannot rot back.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
/**
* 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.
*/

/**
* CHARACTERIZATION — what actually happens when focus leaves a HOST-INJECTED
* cell editor (objectui#6859).
*
* ## Why this file exists
*
* `data-table.tsx` exits an injected editor through a document-level
* `pointerdown` listener, and the comment at `injectedEditorElRef` used to
* justify that with "the injected widgets (text, number, date, lookup, …) have
* no such handler". That justification is stale — `onBlur` is a declared DOM
* pass-through key every inline-edit widget forwards (objectui#6780, #6802) —
* and correcting a comment leaves nothing behind that can rot loudly. A source
* audit then read the same absence as a DATA-LOSS defect: no `focusout`, no
* `onBlurCapture`, no `relatedTarget` anywhere in the file ⇒ Tab out of an
* injected editor must silently drop the typed value.
*
* It does not, and this file is the measurement that says why. The value never
* depends on the exit event: the host wires the widget's `onChange` to `stage`,
* so every keystroke is already in `pendingChanges` while the editor is still
* open. The `pointerdown` listener exits EDIT MODE; it does not rescue values.
*
* The same sequence was driven in a real Chromium against the real
* `@object-ui/fields` widgets (text / date / number) before this file was
* written; these are the jsdom pins for CI.
*
* ## The control
*
* Test A is the control and must stay green: a BUILT-IN editor DOES commit on
* focus loss. Without it, test C's negative ("Tab commits nothing") would pass
* just as well on a harness that cannot observe a commit at all.
*/
import { describe, it, expect, vi } from 'vitest';
import { fireEvent, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import '@testing-library/jest-dom';
import React from 'react';
import { renderComponent } from './test-utils';
// Module scope, not a hook — see object-ui/no-dynamic-import-in-test-hook.
import '../renderers';

const baseSchema = {
type: 'data-table' as const,
editable: true,
singleClickEdit: true,
columns: [
{ header: 'Name', accessorKey: 'name', editable: false },
{ header: 'Qty', accessorKey: 'qty', type: 'number' },
],
data: [{ id: '1', name: 'row-one', qty: '' }],
} as any;

/**
* The injected editor exactly as the in-repo host builds it
* (`ObjectGrid.renderCellEditor`): a real control whose `onChange` is wired to
* the context's `stage` — non-discrete field types stage, they do not commit.
* `packages/components` does not depend on `@object-ui/fields`, so the wiring
* is reproduced rather than imported; the browser run linked above is what
* pins it to the real widgets.
*/
const stagingEditor = ({ column, value, stage }: any) =>
column.accessorKey === 'qty' ? (
<input
data-testid="injected-editor"
value={value ?? ''}
onChange={(e) => stage(e.target.value)}
/>
) : null;

/** A tabbable element outside the table, so `userEvent.tab()` has somewhere to go. */
function withOutsideTabStop(): HTMLButtonElement {
const btn = document.createElement('button');
btn.id = 'outside-tab-stop';
btn.textContent = 'outside';
document.body.appendChild(btn);
return btn;
}

describe('data-table — focus loss on a host-injected cell editor (objectui#6859)', () => {
it('A) CONTROL: a BUILT-IN editor DOES commit when focus leaves it', async () => {
// Proves the harness can observe a commit driven by focus loss at all, so
// the negative results below are measurements and not dead probes.
const onCellChange = vi.fn();
const { container } = renderComponent({ ...baseSchema, onCellChange });

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);

const input = qtyCell.querySelector('input') as HTMLInputElement;
expect(input).toBeTruthy();
fireEvent.change(input, { target: { value: '42' } });
fireEvent.blur(input);

expect(onCellChange).toHaveBeenCalledWith(0, 'qty', '42', expect.anything());
});

it('B) an injected editor stages every keystroke — the value is pending BEFORE any exit event', () => {
// This is the path the source audit could not see, and the reason Tab-out
// is not lossy: `stage` writes straight into `pendingChanges` while the
// editor is still open and still focused.
const { container, getByText, queryByText } = renderComponent({
...baseSchema,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
// Nothing pending yet — the toolbar's save affordance is the readout.
expect(queryByText(/Save All/i)).toBeNull();

fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
expect(editor).toBeTruthy();
fireEvent.change(editor, { target: { value: 'TYPED' } });

// Still in edit mode, nothing committed — and the value is already staged.
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
expect(getByText(/Save All/i)).toBeInTheDocument();
});

it('C) tabbing out of an injected editor neither commits nor leaves edit mode', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
expect(document.activeElement).toBe(editor);
await userEvent.tab();

// Focus really left the editor …
expect(document.activeElement).not.toBe(editor);
// … and nothing committed: no exit, no onCellChange. This is the fact the
// corrected comment at `injectedEditorElRef` now states, and the reason
// the document-level pointerdown listener is still load-bearing.
expect(onCellChange).not.toHaveBeenCalled();
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
} finally {
outside.remove();
}
});

it('D) …and the typed value is still there — the outside pointer press commits exactly it', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
await userEvent.tab();
// A pointer press truly outside is what exits the editor today.
fireEvent.pointerDown(outside);

await waitFor(() =>
expect(onCellChange).toHaveBeenCalledWith(0, 'qty', 'TYPED', expect.anything()),
);
// Edit mode is over and the cell reads back what was typed — nothing lost
// across the Tab-out.
await waitFor(() =>
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeNull(),
);
expect(qtyCell.textContent).toContain('TYPED');
} finally {
outside.remove();
}
});
});
55 changes: 46 additions & 9 deletions packages/components/src/renderers/complex/data-table.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1002,11 +1002,37 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// don't double-commit (Enter) or resurrect a cancelled value (Escape).
const skipBlurSaveRef = useRef(false);
// DOM node of a host-injected widget editor (rendered via `renderCellEditor`),
// captured while it's mounted. The built-in `<input>` editors commit via their
// own onBlur, but the injected widgets (text, number, date, lookup, …) have no
// such handler — a document-level pointerdown listener (see below) uses this
// node to detect click-outside and commit them. Null ⇒ no injected editor is
// active (a built-in editor, or nothing, is showing).
// captured while it's mounted, so the document-level pointerdown listener
// below can tell "inside this editor" from "outside" and exit edit mode.
// Null ⇒ no injected editor is active (a built-in editor, or nothing, is
// showing).
//
// This used to justify itself with "the injected widgets (text, number, date,
// lookup, …) have no such handler". That claim is no longer true and is no
// longer the reason (objectui#6859). `onBlur` is a DECLARED DOM pass-through
// key — named in `FieldWidgetDomProps` (`@object-ui/fields`), named in
// `SDUI_DOM_PASS_THROUGH_KEYS` (`@object-ui/core`), forwarded by
// `toDomProps` — and every widget reachable as an inline editor spreads that
// whitelist onto a real control (26 of the 27 components in `EDIT_WIDGETS`
// call `toDomProps` directly; `UserField` delegates its whole props object to
// `LookupField`, which does). The five widgets that own a blur handler now
// COMPOSE the host's rather than overriding it (objectui#6780, #6802).
//
// The listener is still needed, for a different reason: NOTHING EVER HANDS
// THE WIDGET ONE. The wrapper below carries `onKeyDown` alone, and the
// context object `renderCellEditor` receives — `{ column, row, value, stage,
// commit, cancel }` — has no DOM-props slot to put an `onBlur` in. The
// in-repo factory behind that seam, `@object-ui/fields`' `FieldEditWidget`,
// forwards `autoFocus` and nothing else out of the DOM block, so a host
// handler could not reach the control through it even if one were passed.
//
// Note also what the listener is NOT load-bearing for. Its job is exiting
// EDIT MODE, not rescuing the value: injected widgets stage on every change
// (the host wires the widget's `onChange` to `stageEdit` below), so a typed
// value is already in `pendingChanges` before any exit event — measured in a
// real browser on the text, date and number editors for objectui#6859.
// Retiring this listener would strand cells in edit mode; it would not drop
// edits.
const injectedEditorElRef = useRef<HTMLDivElement | null>(null);
// Snapshot of the active cell's pending value when editing began, so Escape /
// cancel can revert this session's changes. Injected widgets stage on every
Expand DownExpand Up@@ -1728,10 +1754,12 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {

// Commit a host-injected widget editor on click-outside (objectui#2321).
//
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur), but
// the widgets injected through `renderCellEditor` (text, number, date, lookup,
// …) have no such handler, so without this they stay stuck in edit mode when
// the user clicks away. A capture-phase document listener (capture so a cell's
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur). The
// widgets injected through `renderCellEditor` (text, number, date, lookup, …)
// never receive one — not because they cannot deliver it (they can, and do:
// see `injectedEditorElRef` above and objectui#6859) but because nothing on
// this seam passes it to them — so without this they stay stuck in edit mode
// when the user clicks away. A capture-phase document listener (capture so a cell's
// own `stopPropagation` can't hide it) commits the staged value and exits edit
// mode when the pointer goes down truly outside the editor — but NOT inside a
// Radix overlay the widget itself opened (a lookup popover / record-picker
Expand DownExpand Up@@ -2302,6 +2330,15 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// picker's `<button>` trigger or a multi-line
// textarea it's left alone so Enter opens the
// dropdown / inserts a newline as usual.
//
// Tab is deliberately NOT in that list, and
// tabbing out therefore does not leave edit
// mode — measured, objectui#6859. It costs
// nothing: the widget has already staged
// every keystroke into `pendingChanges`, so
// the value is safe; the cell simply stays
// open until Enter, Escape, or a pointer
// press outside closes it.
return (
<div
ref={(n) => { injectedEditorElRef.current = n; }}
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' docs(components): the injected-editor commit justification is stale — correct it, and pin what Tab-out actually does by claude[bot] · Pull Request #6912 · objectstack-ai/objectui · GitHub
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
21 changes: 21 additions & 0 deletions .changeset/6859-injected-editor-commit-comment.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
---

No behaviour change, and deliberately so: this is the record being corrected, not the
renderer (objectui#6859).

The data table's document-level `pointerdown` listener — the one that exits a host-injected
inline cell editor when you click away — justified itself with "the injected widgets (text,
number, date, lookup, …) have no such handler". That has not been true since objectui#6780 /
#6802: `onBlur` is a declared DOM pass-through key, and all 27 widgets reachable as an inline
editor deliver it to a real control (26 spread `toDomProps` themselves, `UserField` delegates
to `LookupField`).

A source audit read the same absence as silent DATA LOSS on Tab-out. It is not. Driven in a
real browser against the real widgets, a value typed into a text, date or number cell editor
survives tabbing away, and reads back intact: the host wires each widget's `onChange` to the
table's `stage`, so every keystroke is already in `pendingChanges` while the editor is still
open. The listener exits EDIT MODE; it never rescued the value. Tabbing out does leave the
cell in edit mode until Enter, Escape, or a pointer press outside — a wart, not a lost edit.

The comment now says all of that, and both facts are pinned by tests so they cannot rot back.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
/**
* 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.
*/

/**
* CHARACTERIZATION — what actually happens when focus leaves a HOST-INJECTED
* cell editor (objectui#6859).
*
* ## Why this file exists
*
* `data-table.tsx` exits an injected editor through a document-level
* `pointerdown` listener, and the comment at `injectedEditorElRef` used to
* justify that with "the injected widgets (text, number, date, lookup, …) have
* no such handler". That justification is stale — `onBlur` is a declared DOM
* pass-through key every inline-edit widget forwards (objectui#6780, #6802) —
* and correcting a comment leaves nothing behind that can rot loudly. A source
* audit then read the same absence as a DATA-LOSS defect: no `focusout`, no
* `onBlurCapture`, no `relatedTarget` anywhere in the file ⇒ Tab out of an
* injected editor must silently drop the typed value.
*
* It does not, and this file is the measurement that says why. The value never
* depends on the exit event: the host wires the widget's `onChange` to `stage`,
* so every keystroke is already in `pendingChanges` while the editor is still
* open. The `pointerdown` listener exits EDIT MODE; it does not rescue values.
*
* The same sequence was driven in a real Chromium against the real
* `@object-ui/fields` widgets (text / date / number) before this file was
* written; these are the jsdom pins for CI.
*
* ## The control
*
* Test A is the control and must stay green: a BUILT-IN editor DOES commit on
* focus loss. Without it, test C's negative ("Tab commits nothing") would pass
* just as well on a harness that cannot observe a commit at all.
*/
import { describe, it, expect, vi } from 'vitest';
import { fireEvent, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import '@testing-library/jest-dom';
import React from 'react';
import { renderComponent } from './test-utils';
// Module scope, not a hook — see object-ui/no-dynamic-import-in-test-hook.
import '../renderers';

const baseSchema = {
type: 'data-table' as const,
editable: true,
singleClickEdit: true,
columns: [
{ header: 'Name', accessorKey: 'name', editable: false },
{ header: 'Qty', accessorKey: 'qty', type: 'number' },
],
data: [{ id: '1', name: 'row-one', qty: '' }],
} as any;

/**
* The injected editor exactly as the in-repo host builds it
* (`ObjectGrid.renderCellEditor`): a real control whose `onChange` is wired to
* the context's `stage` — non-discrete field types stage, they do not commit.
* `packages/components` does not depend on `@object-ui/fields`, so the wiring
* is reproduced rather than imported; the browser run linked above is what
* pins it to the real widgets.
*/
const stagingEditor = ({ column, value, stage }: any) =>
column.accessorKey === 'qty' ? (
<input
data-testid="injected-editor"
value={value ?? ''}
onChange={(e) => stage(e.target.value)}
/>
) : null;

/** A tabbable element outside the table, so `userEvent.tab()` has somewhere to go. */
function withOutsideTabStop(): HTMLButtonElement {
const btn = document.createElement('button');
btn.id = 'outside-tab-stop';
btn.textContent = 'outside';
document.body.appendChild(btn);
return btn;
}

describe('data-table — focus loss on a host-injected cell editor (objectui#6859)', () => {
it('A) CONTROL: a BUILT-IN editor DOES commit when focus leaves it', async () => {
// Proves the harness can observe a commit driven by focus loss at all, so
// the negative results below are measurements and not dead probes.
const onCellChange = vi.fn();
const { container } = renderComponent({ ...baseSchema, onCellChange });

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);

const input = qtyCell.querySelector('input') as HTMLInputElement;
expect(input).toBeTruthy();
fireEvent.change(input, { target: { value: '42' } });
fireEvent.blur(input);

expect(onCellChange).toHaveBeenCalledWith(0, 'qty', '42', expect.anything());
});

it('B) an injected editor stages every keystroke — the value is pending BEFORE any exit event', () => {
// This is the path the source audit could not see, and the reason Tab-out
// is not lossy: `stage` writes straight into `pendingChanges` while the
// editor is still open and still focused.
const { container, getByText, queryByText } = renderComponent({
...baseSchema,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
// Nothing pending yet — the toolbar's save affordance is the readout.
expect(queryByText(/Save All/i)).toBeNull();

fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
expect(editor).toBeTruthy();
fireEvent.change(editor, { target: { value: 'TYPED' } });

// Still in edit mode, nothing committed — and the value is already staged.
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
expect(getByText(/Save All/i)).toBeInTheDocument();
});

it('C) tabbing out of an injected editor neither commits nor leaves edit mode', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
expect(document.activeElement).toBe(editor);
await userEvent.tab();

// Focus really left the editor …
expect(document.activeElement).not.toBe(editor);
// … and nothing committed: no exit, no onCellChange. This is the fact the
// corrected comment at `injectedEditorElRef` now states, and the reason
// the document-level pointerdown listener is still load-bearing.
expect(onCellChange).not.toHaveBeenCalled();
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
} finally {
outside.remove();
}
});

it('D) …and the typed value is still there — the outside pointer press commits exactly it', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
await userEvent.tab();
// A pointer press truly outside is what exits the editor today.
fireEvent.pointerDown(outside);

await waitFor(() =>
expect(onCellChange).toHaveBeenCalledWith(0, 'qty', 'TYPED', expect.anything()),
);
// Edit mode is over and the cell reads back what was typed — nothing lost
// across the Tab-out.
await waitFor(() =>
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeNull(),
);
expect(qtyCell.textContent).toContain('TYPED');
} finally {
outside.remove();
}
});
});
55 changes: 46 additions & 9 deletions packages/components/src/renderers/complex/data-table.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1002,11 +1002,37 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// don't double-commit (Enter) or resurrect a cancelled value (Escape).
const skipBlurSaveRef = useRef(false);
// DOM node of a host-injected widget editor (rendered via `renderCellEditor`),
// captured while it's mounted. The built-in `<input>` editors commit via their
// own onBlur, but the injected widgets (text, number, date, lookup, …) have no
// such handler — a document-level pointerdown listener (see below) uses this
// node to detect click-outside and commit them. Null ⇒ no injected editor is
// active (a built-in editor, or nothing, is showing).
// captured while it's mounted, so the document-level pointerdown listener
// below can tell "inside this editor" from "outside" and exit edit mode.
// Null ⇒ no injected editor is active (a built-in editor, or nothing, is
// showing).
//
// This used to justify itself with "the injected widgets (text, number, date,
// lookup, …) have no such handler". That claim is no longer true and is no
// longer the reason (objectui#6859). `onBlur` is a DECLARED DOM pass-through
// key — named in `FieldWidgetDomProps` (`@object-ui/fields`), named in
// `SDUI_DOM_PASS_THROUGH_KEYS` (`@object-ui/core`), forwarded by
// `toDomProps` — and every widget reachable as an inline editor spreads that
// whitelist onto a real control (26 of the 27 components in `EDIT_WIDGETS`
// call `toDomProps` directly; `UserField` delegates its whole props object to
// `LookupField`, which does). The five widgets that own a blur handler now
// COMPOSE the host's rather than overriding it (objectui#6780, #6802).
//
// The listener is still needed, for a different reason: NOTHING EVER HANDS
// THE WIDGET ONE. The wrapper below carries `onKeyDown` alone, and the
// context object `renderCellEditor` receives — `{ column, row, value, stage,
// commit, cancel }` — has no DOM-props slot to put an `onBlur` in. The
// in-repo factory behind that seam, `@object-ui/fields`' `FieldEditWidget`,
// forwards `autoFocus` and nothing else out of the DOM block, so a host
// handler could not reach the control through it even if one were passed.
//
// Note also what the listener is NOT load-bearing for. Its job is exiting
// EDIT MODE, not rescuing the value: injected widgets stage on every change
// (the host wires the widget's `onChange` to `stageEdit` below), so a typed
// value is already in `pendingChanges` before any exit event — measured in a
// real browser on the text, date and number editors for objectui#6859.
// Retiring this listener would strand cells in edit mode; it would not drop
// edits.
const injectedEditorElRef = useRef<HTMLDivElement | null>(null);
// Snapshot of the active cell's pending value when editing began, so Escape /
// cancel can revert this session's changes. Injected widgets stage on every
Expand DownExpand Up@@ -1728,10 +1754,12 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {

// Commit a host-injected widget editor on click-outside (objectui#2321).
//
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur), but
// the widgets injected through `renderCellEditor` (text, number, date, lookup,
// …) have no such handler, so without this they stay stuck in edit mode when
// the user clicks away. A capture-phase document listener (capture so a cell's
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur). The
// widgets injected through `renderCellEditor` (text, number, date, lookup, …)
// never receive one — not because they cannot deliver it (they can, and do:
// see `injectedEditorElRef` above and objectui#6859) but because nothing on
// this seam passes it to them — so without this they stay stuck in edit mode
// when the user clicks away. A capture-phase document listener (capture so a cell's
// own `stopPropagation` can't hide it) commits the staged value and exits edit
// mode when the pointer goes down truly outside the editor — but NOT inside a
// Radix overlay the widget itself opened (a lookup popover / record-picker
Expand DownExpand Up@@ -2302,6 +2330,15 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// picker's `<button>` trigger or a multi-line
// textarea it's left alone so Enter opens the
// dropdown / inserts a newline as usual.
//
// Tab is deliberately NOT in that list, and
// tabbing out therefore does not leave edit
// mode — measured, objectui#6859. It costs
// nothing: the widget has already staged
// every keystroke into `pendingChanges`, so
// the value is safe; the cell simply stays
// open until Enter, Escape, or a pointer
// press outside closes it.
return (
<div
ref={(n) => { injectedEditorElRef.current = n; }}
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' docs(components): the injected-editor commit justification is stale — correct it, and pin what Tab-out actually does by claude[bot] · Pull Request #6912 · objectstack-ai/objectui · GitHub
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
21 changes: 21 additions & 0 deletions .changeset/6859-injected-editor-commit-comment.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
---

No behaviour change, and deliberately so: this is the record being corrected, not the
renderer (objectui#6859).

The data table's document-level `pointerdown` listener — the one that exits a host-injected
inline cell editor when you click away — justified itself with "the injected widgets (text,
number, date, lookup, …) have no such handler". That has not been true since objectui#6780 /
#6802: `onBlur` is a declared DOM pass-through key, and all 27 widgets reachable as an inline
editor deliver it to a real control (26 spread `toDomProps` themselves, `UserField` delegates
to `LookupField`).

A source audit read the same absence as silent DATA LOSS on Tab-out. It is not. Driven in a
real browser against the real widgets, a value typed into a text, date or number cell editor
survives tabbing away, and reads back intact: the host wires each widget's `onChange` to the
table's `stage`, so every keystroke is already in `pendingChanges` while the editor is still
open. The listener exits EDIT MODE; it never rescued the value. Tabbing out does leave the
cell in edit mode until Enter, Escape, or a pointer press outside — a wart, not a lost edit.

The comment now says all of that, and both facts are pinned by tests so they cannot rot back.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
/**
* 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.
*/

/**
* CHARACTERIZATION — what actually happens when focus leaves a HOST-INJECTED
* cell editor (objectui#6859).
*
* ## Why this file exists
*
* `data-table.tsx` exits an injected editor through a document-level
* `pointerdown` listener, and the comment at `injectedEditorElRef` used to
* justify that with "the injected widgets (text, number, date, lookup, …) have
* no such handler". That justification is stale — `onBlur` is a declared DOM
* pass-through key every inline-edit widget forwards (objectui#6780, #6802) —
* and correcting a comment leaves nothing behind that can rot loudly. A source
* audit then read the same absence as a DATA-LOSS defect: no `focusout`, no
* `onBlurCapture`, no `relatedTarget` anywhere in the file ⇒ Tab out of an
* injected editor must silently drop the typed value.
*
* It does not, and this file is the measurement that says why. The value never
* depends on the exit event: the host wires the widget's `onChange` to `stage`,
* so every keystroke is already in `pendingChanges` while the editor is still
* open. The `pointerdown` listener exits EDIT MODE; it does not rescue values.
*
* The same sequence was driven in a real Chromium against the real
* `@object-ui/fields` widgets (text / date / number) before this file was
* written; these are the jsdom pins for CI.
*
* ## The control
*
* Test A is the control and must stay green: a BUILT-IN editor DOES commit on
* focus loss. Without it, test C's negative ("Tab commits nothing") would pass
* just as well on a harness that cannot observe a commit at all.
*/
import { describe, it, expect, vi } from 'vitest';
import { fireEvent, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import '@testing-library/jest-dom';
import React from 'react';
import { renderComponent } from './test-utils';
// Module scope, not a hook — see object-ui/no-dynamic-import-in-test-hook.
import '../renderers';

const baseSchema = {
type: 'data-table' as const,
editable: true,
singleClickEdit: true,
columns: [
{ header: 'Name', accessorKey: 'name', editable: false },
{ header: 'Qty', accessorKey: 'qty', type: 'number' },
],
data: [{ id: '1', name: 'row-one', qty: '' }],
} as any;

/**
* The injected editor exactly as the in-repo host builds it
* (`ObjectGrid.renderCellEditor`): a real control whose `onChange` is wired to
* the context's `stage` — non-discrete field types stage, they do not commit.
* `packages/components` does not depend on `@object-ui/fields`, so the wiring
* is reproduced rather than imported; the browser run linked above is what
* pins it to the real widgets.
*/
const stagingEditor = ({ column, value, stage }: any) =>
column.accessorKey === 'qty' ? (
<input
data-testid="injected-editor"
value={value ?? ''}
onChange={(e) => stage(e.target.value)}
/>
) : null;

/** A tabbable element outside the table, so `userEvent.tab()` has somewhere to go. */
function withOutsideTabStop(): HTMLButtonElement {
const btn = document.createElement('button');
btn.id = 'outside-tab-stop';
btn.textContent = 'outside';
document.body.appendChild(btn);
return btn;
}

describe('data-table — focus loss on a host-injected cell editor (objectui#6859)', () => {
it('A) CONTROL: a BUILT-IN editor DOES commit when focus leaves it', async () => {
// Proves the harness can observe a commit driven by focus loss at all, so
// the negative results below are measurements and not dead probes.
const onCellChange = vi.fn();
const { container } = renderComponent({ ...baseSchema, onCellChange });

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);

const input = qtyCell.querySelector('input') as HTMLInputElement;
expect(input).toBeTruthy();
fireEvent.change(input, { target: { value: '42' } });
fireEvent.blur(input);

expect(onCellChange).toHaveBeenCalledWith(0, 'qty', '42', expect.anything());
});

it('B) an injected editor stages every keystroke — the value is pending BEFORE any exit event', () => {
// This is the path the source audit could not see, and the reason Tab-out
// is not lossy: `stage` writes straight into `pendingChanges` while the
// editor is still open and still focused.
const { container, getByText, queryByText } = renderComponent({
...baseSchema,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
// Nothing pending yet — the toolbar's save affordance is the readout.
expect(queryByText(/Save All/i)).toBeNull();

fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
expect(editor).toBeTruthy();
fireEvent.change(editor, { target: { value: 'TYPED' } });

// Still in edit mode, nothing committed — and the value is already staged.
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
expect(getByText(/Save All/i)).toBeInTheDocument();
});

it('C) tabbing out of an injected editor neither commits nor leaves edit mode', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
expect(document.activeElement).toBe(editor);
await userEvent.tab();

// Focus really left the editor …
expect(document.activeElement).not.toBe(editor);
// … and nothing committed: no exit, no onCellChange. This is the fact the
// corrected comment at `injectedEditorElRef` now states, and the reason
// the document-level pointerdown listener is still load-bearing.
expect(onCellChange).not.toHaveBeenCalled();
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
} finally {
outside.remove();
}
});

it('D) …and the typed value is still there — the outside pointer press commits exactly it', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
await userEvent.tab();
// A pointer press truly outside is what exits the editor today.
fireEvent.pointerDown(outside);

await waitFor(() =>
expect(onCellChange).toHaveBeenCalledWith(0, 'qty', 'TYPED', expect.anything()),
);
// Edit mode is over and the cell reads back what was typed — nothing lost
// across the Tab-out.
await waitFor(() =>
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeNull(),
);
expect(qtyCell.textContent).toContain('TYPED');
} finally {
outside.remove();
}
});
});
55 changes: 46 additions & 9 deletions packages/components/src/renderers/complex/data-table.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1002,11 +1002,37 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// don't double-commit (Enter) or resurrect a cancelled value (Escape).
const skipBlurSaveRef = useRef(false);
// DOM node of a host-injected widget editor (rendered via `renderCellEditor`),
// captured while it's mounted. The built-in `<input>` editors commit via their
// own onBlur, but the injected widgets (text, number, date, lookup, …) have no
// such handler — a document-level pointerdown listener (see below) uses this
// node to detect click-outside and commit them. Null ⇒ no injected editor is
// active (a built-in editor, or nothing, is showing).
// captured while it's mounted, so the document-level pointerdown listener
// below can tell "inside this editor" from "outside" and exit edit mode.
// Null ⇒ no injected editor is active (a built-in editor, or nothing, is
// showing).
//
// This used to justify itself with "the injected widgets (text, number, date,
// lookup, …) have no such handler". That claim is no longer true and is no
// longer the reason (objectui#6859). `onBlur` is a DECLARED DOM pass-through
// key — named in `FieldWidgetDomProps` (`@object-ui/fields`), named in
// `SDUI_DOM_PASS_THROUGH_KEYS` (`@object-ui/core`), forwarded by
// `toDomProps` — and every widget reachable as an inline editor spreads that
// whitelist onto a real control (26 of the 27 components in `EDIT_WIDGETS`
// call `toDomProps` directly; `UserField` delegates its whole props object to
// `LookupField`, which does). The five widgets that own a blur handler now
// COMPOSE the host's rather than overriding it (objectui#6780, #6802).
//
// The listener is still needed, for a different reason: NOTHING EVER HANDS
// THE WIDGET ONE. The wrapper below carries `onKeyDown` alone, and the
// context object `renderCellEditor` receives — `{ column, row, value, stage,
// commit, cancel }` — has no DOM-props slot to put an `onBlur` in. The
// in-repo factory behind that seam, `@object-ui/fields`' `FieldEditWidget`,
// forwards `autoFocus` and nothing else out of the DOM block, so a host
// handler could not reach the control through it even if one were passed.
//
// Note also what the listener is NOT load-bearing for. Its job is exiting
// EDIT MODE, not rescuing the value: injected widgets stage on every change
// (the host wires the widget's `onChange` to `stageEdit` below), so a typed
// value is already in `pendingChanges` before any exit event — measured in a
// real browser on the text, date and number editors for objectui#6859.
// Retiring this listener would strand cells in edit mode; it would not drop
// edits.
const injectedEditorElRef = useRef<HTMLDivElement | null>(null);
// Snapshot of the active cell's pending value when editing began, so Escape /
// cancel can revert this session's changes. Injected widgets stage on every
Expand DownExpand Up@@ -1728,10 +1754,12 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {

// Commit a host-injected widget editor on click-outside (objectui#2321).
//
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur), but
// the widgets injected through `renderCellEditor` (text, number, date, lookup,
// …) have no such handler, so without this they stay stuck in edit mode when
// the user clicks away. A capture-phase document listener (capture so a cell's
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur). The
// widgets injected through `renderCellEditor` (text, number, date, lookup, …)
// never receive one — not because they cannot deliver it (they can, and do:
// see `injectedEditorElRef` above and objectui#6859) but because nothing on
// this seam passes it to them — so without this they stay stuck in edit mode
// when the user clicks away. A capture-phase document listener (capture so a cell's
// own `stopPropagation` can't hide it) commits the staged value and exits edit
// mode when the pointer goes down truly outside the editor — but NOT inside a
// Radix overlay the widget itself opened (a lookup popover / record-picker
Expand DownExpand Up@@ -2302,6 +2330,15 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// picker's `<button>` trigger or a multi-line
// textarea it's left alone so Enter opens the
// dropdown / inserts a newline as usual.
//
// Tab is deliberately NOT in that list, and
// tabbing out therefore does not leave edit
// mode — measured, objectui#6859. It costs
// nothing: the widget has already staged
// every keystroke into `pendingChanges`, so
// the value is safe; the cell simply stays
// open until Enter, Escape, or a pointer
// press outside closes it.
return (
<div
ref={(n) => { injectedEditorElRef.current = n; }}
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' docs(components): the injected-editor commit justification is stale — correct it, and pin what Tab-out actually does by claude[bot] · Pull Request #6912 · objectstack-ai/objectui · GitHub
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
21 changes: 21 additions & 0 deletions .changeset/6859-injected-editor-commit-comment.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
---

No behaviour change, and deliberately so: this is the record being corrected, not the
renderer (objectui#6859).

The data table's document-level `pointerdown` listener — the one that exits a host-injected
inline cell editor when you click away — justified itself with "the injected widgets (text,
number, date, lookup, …) have no such handler". That has not been true since objectui#6780 /
#6802: `onBlur` is a declared DOM pass-through key, and all 27 widgets reachable as an inline
editor deliver it to a real control (26 spread `toDomProps` themselves, `UserField` delegates
to `LookupField`).

A source audit read the same absence as silent DATA LOSS on Tab-out. It is not. Driven in a
real browser against the real widgets, a value typed into a text, date or number cell editor
survives tabbing away, and reads back intact: the host wires each widget's `onChange` to the
table's `stage`, so every keystroke is already in `pendingChanges` while the editor is still
open. The listener exits EDIT MODE; it never rescued the value. Tabbing out does leave the
cell in edit mode until Enter, Escape, or a pointer press outside — a wart, not a lost edit.

The comment now says all of that, and both facts are pinned by tests so they cannot rot back.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
/**
* 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.
*/

/**
* CHARACTERIZATION — what actually happens when focus leaves a HOST-INJECTED
* cell editor (objectui#6859).
*
* ## Why this file exists
*
* `data-table.tsx` exits an injected editor through a document-level
* `pointerdown` listener, and the comment at `injectedEditorElRef` used to
* justify that with "the injected widgets (text, number, date, lookup, …) have
* no such handler". That justification is stale — `onBlur` is a declared DOM
* pass-through key every inline-edit widget forwards (objectui#6780, #6802) —
* and correcting a comment leaves nothing behind that can rot loudly. A source
* audit then read the same absence as a DATA-LOSS defect: no `focusout`, no
* `onBlurCapture`, no `relatedTarget` anywhere in the file ⇒ Tab out of an
* injected editor must silently drop the typed value.
*
* It does not, and this file is the measurement that says why. The value never
* depends on the exit event: the host wires the widget's `onChange` to `stage`,
* so every keystroke is already in `pendingChanges` while the editor is still
* open. The `pointerdown` listener exits EDIT MODE; it does not rescue values.
*
* The same sequence was driven in a real Chromium against the real
* `@object-ui/fields` widgets (text / date / number) before this file was
* written; these are the jsdom pins for CI.
*
* ## The control
*
* Test A is the control and must stay green: a BUILT-IN editor DOES commit on
* focus loss. Without it, test C's negative ("Tab commits nothing") would pass
* just as well on a harness that cannot observe a commit at all.
*/
import { describe, it, expect, vi } from 'vitest';
import { fireEvent, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import '@testing-library/jest-dom';
import React from 'react';
import { renderComponent } from './test-utils';
// Module scope, not a hook — see object-ui/no-dynamic-import-in-test-hook.
import '../renderers';

const baseSchema = {
type: 'data-table' as const,
editable: true,
singleClickEdit: true,
columns: [
{ header: 'Name', accessorKey: 'name', editable: false },
{ header: 'Qty', accessorKey: 'qty', type: 'number' },
],
data: [{ id: '1', name: 'row-one', qty: '' }],
} as any;

/**
* The injected editor exactly as the in-repo host builds it
* (`ObjectGrid.renderCellEditor`): a real control whose `onChange` is wired to
* the context's `stage` — non-discrete field types stage, they do not commit.
* `packages/components` does not depend on `@object-ui/fields`, so the wiring
* is reproduced rather than imported; the browser run linked above is what
* pins it to the real widgets.
*/
const stagingEditor = ({ column, value, stage }: any) =>
column.accessorKey === 'qty' ? (
<input
data-testid="injected-editor"
value={value ?? ''}
onChange={(e) => stage(e.target.value)}
/>
) : null;

/** A tabbable element outside the table, so `userEvent.tab()` has somewhere to go. */
function withOutsideTabStop(): HTMLButtonElement {
const btn = document.createElement('button');
btn.id = 'outside-tab-stop';
btn.textContent = 'outside';
document.body.appendChild(btn);
return btn;
}

describe('data-table — focus loss on a host-injected cell editor (objectui#6859)', () => {
it('A) CONTROL: a BUILT-IN editor DOES commit when focus leaves it', async () => {
// Proves the harness can observe a commit driven by focus loss at all, so
// the negative results below are measurements and not dead probes.
const onCellChange = vi.fn();
const { container } = renderComponent({ ...baseSchema, onCellChange });

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);

const input = qtyCell.querySelector('input') as HTMLInputElement;
expect(input).toBeTruthy();
fireEvent.change(input, { target: { value: '42' } });
fireEvent.blur(input);

expect(onCellChange).toHaveBeenCalledWith(0, 'qty', '42', expect.anything());
});

it('B) an injected editor stages every keystroke — the value is pending BEFORE any exit event', () => {
// This is the path the source audit could not see, and the reason Tab-out
// is not lossy: `stage` writes straight into `pendingChanges` while the
// editor is still open and still focused.
const { container, getByText, queryByText } = renderComponent({
...baseSchema,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
// Nothing pending yet — the toolbar's save affordance is the readout.
expect(queryByText(/Save All/i)).toBeNull();

fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
expect(editor).toBeTruthy();
fireEvent.change(editor, { target: { value: 'TYPED' } });

// Still in edit mode, nothing committed — and the value is already staged.
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
expect(getByText(/Save All/i)).toBeInTheDocument();
});

it('C) tabbing out of an injected editor neither commits nor leaves edit mode', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
expect(document.activeElement).toBe(editor);
await userEvent.tab();

// Focus really left the editor …
expect(document.activeElement).not.toBe(editor);
// … and nothing committed: no exit, no onCellChange. This is the fact the
// corrected comment at `injectedEditorElRef` now states, and the reason
// the document-level pointerdown listener is still load-bearing.
expect(onCellChange).not.toHaveBeenCalled();
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
} finally {
outside.remove();
}
});

it('D) …and the typed value is still there — the outside pointer press commits exactly it', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
await userEvent.tab();
// A pointer press truly outside is what exits the editor today.
fireEvent.pointerDown(outside);

await waitFor(() =>
expect(onCellChange).toHaveBeenCalledWith(0, 'qty', 'TYPED', expect.anything()),
);
// Edit mode is over and the cell reads back what was typed — nothing lost
// across the Tab-out.
await waitFor(() =>
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeNull(),
);
expect(qtyCell.textContent).toContain('TYPED');
} finally {
outside.remove();
}
});
});
55 changes: 46 additions & 9 deletions packages/components/src/renderers/complex/data-table.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1002,11 +1002,37 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// don't double-commit (Enter) or resurrect a cancelled value (Escape).
const skipBlurSaveRef = useRef(false);
// DOM node of a host-injected widget editor (rendered via `renderCellEditor`),
// captured while it's mounted. The built-in `<input>` editors commit via their
// own onBlur, but the injected widgets (text, number, date, lookup, …) have no
// such handler — a document-level pointerdown listener (see below) uses this
// node to detect click-outside and commit them. Null ⇒ no injected editor is
// active (a built-in editor, or nothing, is showing).
// captured while it's mounted, so the document-level pointerdown listener
// below can tell "inside this editor" from "outside" and exit edit mode.
// Null ⇒ no injected editor is active (a built-in editor, or nothing, is
// showing).
//
// This used to justify itself with "the injected widgets (text, number, date,
// lookup, …) have no such handler". That claim is no longer true and is no
// longer the reason (objectui#6859). `onBlur` is a DECLARED DOM pass-through
// key — named in `FieldWidgetDomProps` (`@object-ui/fields`), named in
// `SDUI_DOM_PASS_THROUGH_KEYS` (`@object-ui/core`), forwarded by
// `toDomProps` — and every widget reachable as an inline editor spreads that
// whitelist onto a real control (26 of the 27 components in `EDIT_WIDGETS`
// call `toDomProps` directly; `UserField` delegates its whole props object to
// `LookupField`, which does). The five widgets that own a blur handler now
// COMPOSE the host's rather than overriding it (objectui#6780, #6802).
//
// The listener is still needed, for a different reason: NOTHING EVER HANDS
// THE WIDGET ONE. The wrapper below carries `onKeyDown` alone, and the
// context object `renderCellEditor` receives — `{ column, row, value, stage,
// commit, cancel }` — has no DOM-props slot to put an `onBlur` in. The
// in-repo factory behind that seam, `@object-ui/fields`' `FieldEditWidget`,
// forwards `autoFocus` and nothing else out of the DOM block, so a host
// handler could not reach the control through it even if one were passed.
//
// Note also what the listener is NOT load-bearing for. Its job is exiting
// EDIT MODE, not rescuing the value: injected widgets stage on every change
// (the host wires the widget's `onChange` to `stageEdit` below), so a typed
// value is already in `pendingChanges` before any exit event — measured in a
// real browser on the text, date and number editors for objectui#6859.
// Retiring this listener would strand cells in edit mode; it would not drop
// edits.
const injectedEditorElRef = useRef<HTMLDivElement | null>(null);
// Snapshot of the active cell's pending value when editing began, so Escape /
// cancel can revert this session's changes. Injected widgets stage on every
Expand DownExpand Up@@ -1728,10 +1754,12 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {

// Commit a host-injected widget editor on click-outside (objectui#2321).
//
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur), but
// the widgets injected through `renderCellEditor` (text, number, date, lookup,
// …) have no such handler, so without this they stay stuck in edit mode when
// the user clicks away. A capture-phase document listener (capture so a cell's
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur). The
// widgets injected through `renderCellEditor` (text, number, date, lookup, …)
// never receive one — not because they cannot deliver it (they can, and do:
// see `injectedEditorElRef` above and objectui#6859) but because nothing on
// this seam passes it to them — so without this they stay stuck in edit mode
// when the user clicks away. A capture-phase document listener (capture so a cell's
// own `stopPropagation` can't hide it) commits the staged value and exits edit
// mode when the pointer goes down truly outside the editor — but NOT inside a
// Radix overlay the widget itself opened (a lookup popover / record-picker
Expand DownExpand Up@@ -2302,6 +2330,15 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// picker's `<button>` trigger or a multi-line
// textarea it's left alone so Enter opens the
// dropdown / inserts a newline as usual.
//
// Tab is deliberately NOT in that list, and
// tabbing out therefore does not leave edit
// mode — measured, objectui#6859. It costs
// nothing: the widget has already staged
// every keystroke into `pendingChanges`, so
// the value is safe; the cell simply stays
// open until Enter, Escape, or a pointer
// press outside closes it.
return (
<div
ref={(n) => { injectedEditorElRef.current = n; }}
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' docs(components): the injected-editor commit justification is stale — correct it, and pin what Tab-out actually does by claude[bot] · Pull Request #6912 · objectstack-ai/objectui · GitHub
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
21 changes: 21 additions & 0 deletions .changeset/6859-injected-editor-commit-comment.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
---

No behaviour change, and deliberately so: this is the record being corrected, not the
renderer (objectui#6859).

The data table's document-level `pointerdown` listener — the one that exits a host-injected
inline cell editor when you click away — justified itself with "the injected widgets (text,
number, date, lookup, …) have no such handler". That has not been true since objectui#6780 /
#6802: `onBlur` is a declared DOM pass-through key, and all 27 widgets reachable as an inline
editor deliver it to a real control (26 spread `toDomProps` themselves, `UserField` delegates
to `LookupField`).

A source audit read the same absence as silent DATA LOSS on Tab-out. It is not. Driven in a
real browser against the real widgets, a value typed into a text, date or number cell editor
survives tabbing away, and reads back intact: the host wires each widget's `onChange` to the
table's `stage`, so every keystroke is already in `pendingChanges` while the editor is still
open. The listener exits EDIT MODE; it never rescued the value. Tabbing out does leave the
cell in edit mode until Enter, Escape, or a pointer press outside — a wart, not a lost edit.

The comment now says all of that, and both facts are pinned by tests so they cannot rot back.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
/**
* 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.
*/

/**
* CHARACTERIZATION — what actually happens when focus leaves a HOST-INJECTED
* cell editor (objectui#6859).
*
* ## Why this file exists
*
* `data-table.tsx` exits an injected editor through a document-level
* `pointerdown` listener, and the comment at `injectedEditorElRef` used to
* justify that with "the injected widgets (text, number, date, lookup, …) have
* no such handler". That justification is stale — `onBlur` is a declared DOM
* pass-through key every inline-edit widget forwards (objectui#6780, #6802) —
* and correcting a comment leaves nothing behind that can rot loudly. A source
* audit then read the same absence as a DATA-LOSS defect: no `focusout`, no
* `onBlurCapture`, no `relatedTarget` anywhere in the file ⇒ Tab out of an
* injected editor must silently drop the typed value.
*
* It does not, and this file is the measurement that says why. The value never
* depends on the exit event: the host wires the widget's `onChange` to `stage`,
* so every keystroke is already in `pendingChanges` while the editor is still
* open. The `pointerdown` listener exits EDIT MODE; it does not rescue values.
*
* The same sequence was driven in a real Chromium against the real
* `@object-ui/fields` widgets (text / date / number) before this file was
* written; these are the jsdom pins for CI.
*
* ## The control
*
* Test A is the control and must stay green: a BUILT-IN editor DOES commit on
* focus loss. Without it, test C's negative ("Tab commits nothing") would pass
* just as well on a harness that cannot observe a commit at all.
*/
import { describe, it, expect, vi } from 'vitest';
import { fireEvent, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import '@testing-library/jest-dom';
import React from 'react';
import { renderComponent } from './test-utils';
// Module scope, not a hook — see object-ui/no-dynamic-import-in-test-hook.
import '../renderers';

const baseSchema = {
type: 'data-table' as const,
editable: true,
singleClickEdit: true,
columns: [
{ header: 'Name', accessorKey: 'name', editable: false },
{ header: 'Qty', accessorKey: 'qty', type: 'number' },
],
data: [{ id: '1', name: 'row-one', qty: '' }],
} as any;

/**
* The injected editor exactly as the in-repo host builds it
* (`ObjectGrid.renderCellEditor`): a real control whose `onChange` is wired to
* the context's `stage` — non-discrete field types stage, they do not commit.
* `packages/components` does not depend on `@object-ui/fields`, so the wiring
* is reproduced rather than imported; the browser run linked above is what
* pins it to the real widgets.
*/
const stagingEditor = ({ column, value, stage }: any) =>
column.accessorKey === 'qty' ? (
<input
data-testid="injected-editor"
value={value ?? ''}
onChange={(e) => stage(e.target.value)}
/>
) : null;

/** A tabbable element outside the table, so `userEvent.tab()` has somewhere to go. */
function withOutsideTabStop(): HTMLButtonElement {
const btn = document.createElement('button');
btn.id = 'outside-tab-stop';
btn.textContent = 'outside';
document.body.appendChild(btn);
return btn;
}

describe('data-table — focus loss on a host-injected cell editor (objectui#6859)', () => {
it('A) CONTROL: a BUILT-IN editor DOES commit when focus leaves it', async () => {
// Proves the harness can observe a commit driven by focus loss at all, so
// the negative results below are measurements and not dead probes.
const onCellChange = vi.fn();
const { container } = renderComponent({ ...baseSchema, onCellChange });

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);

const input = qtyCell.querySelector('input') as HTMLInputElement;
expect(input).toBeTruthy();
fireEvent.change(input, { target: { value: '42' } });
fireEvent.blur(input);

expect(onCellChange).toHaveBeenCalledWith(0, 'qty', '42', expect.anything());
});

it('B) an injected editor stages every keystroke — the value is pending BEFORE any exit event', () => {
// This is the path the source audit could not see, and the reason Tab-out
// is not lossy: `stage` writes straight into `pendingChanges` while the
// editor is still open and still focused.
const { container, getByText, queryByText } = renderComponent({
...baseSchema,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
// Nothing pending yet — the toolbar's save affordance is the readout.
expect(queryByText(/Save All/i)).toBeNull();

fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
expect(editor).toBeTruthy();
fireEvent.change(editor, { target: { value: 'TYPED' } });

// Still in edit mode, nothing committed — and the value is already staged.
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
expect(getByText(/Save All/i)).toBeInTheDocument();
});

it('C) tabbing out of an injected editor neither commits nor leaves edit mode', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
expect(document.activeElement).toBe(editor);
await userEvent.tab();

// Focus really left the editor …
expect(document.activeElement).not.toBe(editor);
// … and nothing committed: no exit, no onCellChange. This is the fact the
// corrected comment at `injectedEditorElRef` now states, and the reason
// the document-level pointerdown listener is still load-bearing.
expect(onCellChange).not.toHaveBeenCalled();
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
} finally {
outside.remove();
}
});

it('D) …and the typed value is still there — the outside pointer press commits exactly it', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
await userEvent.tab();
// A pointer press truly outside is what exits the editor today.
fireEvent.pointerDown(outside);

await waitFor(() =>
expect(onCellChange).toHaveBeenCalledWith(0, 'qty', 'TYPED', expect.anything()),
);
// Edit mode is over and the cell reads back what was typed — nothing lost
// across the Tab-out.
await waitFor(() =>
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeNull(),
);
expect(qtyCell.textContent).toContain('TYPED');
} finally {
outside.remove();
}
});
});
55 changes: 46 additions & 9 deletions packages/components/src/renderers/complex/data-table.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1002,11 +1002,37 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// don't double-commit (Enter) or resurrect a cancelled value (Escape).
const skipBlurSaveRef = useRef(false);
// DOM node of a host-injected widget editor (rendered via `renderCellEditor`),
// captured while it's mounted. The built-in `<input>` editors commit via their
// own onBlur, but the injected widgets (text, number, date, lookup, …) have no
// such handler — a document-level pointerdown listener (see below) uses this
// node to detect click-outside and commit them. Null ⇒ no injected editor is
// active (a built-in editor, or nothing, is showing).
// captured while it's mounted, so the document-level pointerdown listener
// below can tell "inside this editor" from "outside" and exit edit mode.
// Null ⇒ no injected editor is active (a built-in editor, or nothing, is
// showing).
//
// This used to justify itself with "the injected widgets (text, number, date,
// lookup, …) have no such handler". That claim is no longer true and is no
// longer the reason (objectui#6859). `onBlur` is a DECLARED DOM pass-through
// key — named in `FieldWidgetDomProps` (`@object-ui/fields`), named in
// `SDUI_DOM_PASS_THROUGH_KEYS` (`@object-ui/core`), forwarded by
// `toDomProps` — and every widget reachable as an inline editor spreads that
// whitelist onto a real control (26 of the 27 components in `EDIT_WIDGETS`
// call `toDomProps` directly; `UserField` delegates its whole props object to
// `LookupField`, which does). The five widgets that own a blur handler now
// COMPOSE the host's rather than overriding it (objectui#6780, #6802).
//
// The listener is still needed, for a different reason: NOTHING EVER HANDS
// THE WIDGET ONE. The wrapper below carries `onKeyDown` alone, and the
// context object `renderCellEditor` receives — `{ column, row, value, stage,
// commit, cancel }` — has no DOM-props slot to put an `onBlur` in. The
// in-repo factory behind that seam, `@object-ui/fields`' `FieldEditWidget`,
// forwards `autoFocus` and nothing else out of the DOM block, so a host
// handler could not reach the control through it even if one were passed.
//
// Note also what the listener is NOT load-bearing for. Its job is exiting
// EDIT MODE, not rescuing the value: injected widgets stage on every change
// (the host wires the widget's `onChange` to `stageEdit` below), so a typed
// value is already in `pendingChanges` before any exit event — measured in a
// real browser on the text, date and number editors for objectui#6859.
// Retiring this listener would strand cells in edit mode; it would not drop
// edits.
const injectedEditorElRef = useRef<HTMLDivElement | null>(null);
// Snapshot of the active cell's pending value when editing began, so Escape /
// cancel can revert this session's changes. Injected widgets stage on every
Expand DownExpand Up@@ -1728,10 +1754,12 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {

// Commit a host-injected widget editor on click-outside (objectui#2321).
//
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur), but
// the widgets injected through `renderCellEditor` (text, number, date, lookup,
// …) have no such handler, so without this they stay stuck in edit mode when
// the user clicks away. A capture-phase document listener (capture so a cell's
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur). The
// widgets injected through `renderCellEditor` (text, number, date, lookup, …)
// never receive one — not because they cannot deliver it (they can, and do:
// see `injectedEditorElRef` above and objectui#6859) but because nothing on
// this seam passes it to them — so without this they stay stuck in edit mode
// when the user clicks away. A capture-phase document listener (capture so a cell's
// own `stopPropagation` can't hide it) commits the staged value and exits edit
// mode when the pointer goes down truly outside the editor — but NOT inside a
// Radix overlay the widget itself opened (a lookup popover / record-picker
Expand DownExpand Up@@ -2302,6 +2330,15 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// picker's `<button>` trigger or a multi-line
// textarea it's left alone so Enter opens the
// dropdown / inserts a newline as usual.
//
// Tab is deliberately NOT in that list, and
// tabbing out therefore does not leave edit
// mode — measured, objectui#6859. It costs
// nothing: the widget has already staged
// every keystroke into `pendingChanges`, so
// the value is safe; the cell simply stays
// open until Enter, Escape, or a pointer
// press outside closes it.
return (
<div
ref={(n) => { injectedEditorElRef.current = n; }}
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' docs(components): the injected-editor commit justification is stale — correct it, and pin what Tab-out actually does by claude[bot] · Pull Request #6912 · objectstack-ai/objectui · GitHub
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
21 changes: 21 additions & 0 deletions .changeset/6859-injected-editor-commit-comment.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
---

No behaviour change, and deliberately so: this is the record being corrected, not the
renderer (objectui#6859).

The data table's document-level `pointerdown` listener — the one that exits a host-injected
inline cell editor when you click away — justified itself with "the injected widgets (text,
number, date, lookup, …) have no such handler". That has not been true since objectui#6780 /
#6802: `onBlur` is a declared DOM pass-through key, and all 27 widgets reachable as an inline
editor deliver it to a real control (26 spread `toDomProps` themselves, `UserField` delegates
to `LookupField`).

A source audit read the same absence as silent DATA LOSS on Tab-out. It is not. Driven in a
real browser against the real widgets, a value typed into a text, date or number cell editor
survives tabbing away, and reads back intact: the host wires each widget's `onChange` to the
table's `stage`, so every keystroke is already in `pendingChanges` while the editor is still
open. The listener exits EDIT MODE; it never rescued the value. Tabbing out does leave the
cell in edit mode until Enter, Escape, or a pointer press outside — a wart, not a lost edit.

The comment now says all of that, and both facts are pinned by tests so they cannot rot back.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
/**
* 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.
*/

/**
* CHARACTERIZATION — what actually happens when focus leaves a HOST-INJECTED
* cell editor (objectui#6859).
*
* ## Why this file exists
*
* `data-table.tsx` exits an injected editor through a document-level
* `pointerdown` listener, and the comment at `injectedEditorElRef` used to
* justify that with "the injected widgets (text, number, date, lookup, …) have
* no such handler". That justification is stale — `onBlur` is a declared DOM
* pass-through key every inline-edit widget forwards (objectui#6780, #6802) —
* and correcting a comment leaves nothing behind that can rot loudly. A source
* audit then read the same absence as a DATA-LOSS defect: no `focusout`, no
* `onBlurCapture`, no `relatedTarget` anywhere in the file ⇒ Tab out of an
* injected editor must silently drop the typed value.
*
* It does not, and this file is the measurement that says why. The value never
* depends on the exit event: the host wires the widget's `onChange` to `stage`,
* so every keystroke is already in `pendingChanges` while the editor is still
* open. The `pointerdown` listener exits EDIT MODE; it does not rescue values.
*
* The same sequence was driven in a real Chromium against the real
* `@object-ui/fields` widgets (text / date / number) before this file was
* written; these are the jsdom pins for CI.
*
* ## The control
*
* Test A is the control and must stay green: a BUILT-IN editor DOES commit on
* focus loss. Without it, test C's negative ("Tab commits nothing") would pass
* just as well on a harness that cannot observe a commit at all.
*/
import { describe, it, expect, vi } from 'vitest';
import { fireEvent, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import '@testing-library/jest-dom';
import React from 'react';
import { renderComponent } from './test-utils';
// Module scope, not a hook — see object-ui/no-dynamic-import-in-test-hook.
import '../renderers';

const baseSchema = {
type: 'data-table' as const,
editable: true,
singleClickEdit: true,
columns: [
{ header: 'Name', accessorKey: 'name', editable: false },
{ header: 'Qty', accessorKey: 'qty', type: 'number' },
],
data: [{ id: '1', name: 'row-one', qty: '' }],
} as any;

/**
* The injected editor exactly as the in-repo host builds it
* (`ObjectGrid.renderCellEditor`): a real control whose `onChange` is wired to
* the context's `stage` — non-discrete field types stage, they do not commit.
* `packages/components` does not depend on `@object-ui/fields`, so the wiring
* is reproduced rather than imported; the browser run linked above is what
* pins it to the real widgets.
*/
const stagingEditor = ({ column, value, stage }: any) =>
column.accessorKey === 'qty' ? (
<input
data-testid="injected-editor"
value={value ?? ''}
onChange={(e) => stage(e.target.value)}
/>
) : null;

/** A tabbable element outside the table, so `userEvent.tab()` has somewhere to go. */
function withOutsideTabStop(): HTMLButtonElement {
const btn = document.createElement('button');
btn.id = 'outside-tab-stop';
btn.textContent = 'outside';
document.body.appendChild(btn);
return btn;
}

describe('data-table — focus loss on a host-injected cell editor (objectui#6859)', () => {
it('A) CONTROL: a BUILT-IN editor DOES commit when focus leaves it', async () => {
// Proves the harness can observe a commit driven by focus loss at all, so
// the negative results below are measurements and not dead probes.
const onCellChange = vi.fn();
const { container } = renderComponent({ ...baseSchema, onCellChange });

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);

const input = qtyCell.querySelector('input') as HTMLInputElement;
expect(input).toBeTruthy();
fireEvent.change(input, { target: { value: '42' } });
fireEvent.blur(input);

expect(onCellChange).toHaveBeenCalledWith(0, 'qty', '42', expect.anything());
});

it('B) an injected editor stages every keystroke — the value is pending BEFORE any exit event', () => {
// This is the path the source audit could not see, and the reason Tab-out
// is not lossy: `stage` writes straight into `pendingChanges` while the
// editor is still open and still focused.
const { container, getByText, queryByText } = renderComponent({
...baseSchema,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
// Nothing pending yet — the toolbar's save affordance is the readout.
expect(queryByText(/Save All/i)).toBeNull();

fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
expect(editor).toBeTruthy();
fireEvent.change(editor, { target: { value: 'TYPED' } });

// Still in edit mode, nothing committed — and the value is already staged.
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
expect(getByText(/Save All/i)).toBeInTheDocument();
});

it('C) tabbing out of an injected editor neither commits nor leaves edit mode', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
expect(document.activeElement).toBe(editor);
await userEvent.tab();

// Focus really left the editor …
expect(document.activeElement).not.toBe(editor);
// … and nothing committed: no exit, no onCellChange. This is the fact the
// corrected comment at `injectedEditorElRef` now states, and the reason
// the document-level pointerdown listener is still load-bearing.
expect(onCellChange).not.toHaveBeenCalled();
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
} finally {
outside.remove();
}
});

it('D) …and the typed value is still there — the outside pointer press commits exactly it', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
await userEvent.tab();
// A pointer press truly outside is what exits the editor today.
fireEvent.pointerDown(outside);

await waitFor(() =>
expect(onCellChange).toHaveBeenCalledWith(0, 'qty', 'TYPED', expect.anything()),
);
// Edit mode is over and the cell reads back what was typed — nothing lost
// across the Tab-out.
await waitFor(() =>
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeNull(),
);
expect(qtyCell.textContent).toContain('TYPED');
} finally {
outside.remove();
}
});
});
55 changes: 46 additions & 9 deletions packages/components/src/renderers/complex/data-table.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1002,11 +1002,37 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// don't double-commit (Enter) or resurrect a cancelled value (Escape).
const skipBlurSaveRef = useRef(false);
// DOM node of a host-injected widget editor (rendered via `renderCellEditor`),
// captured while it's mounted. The built-in `<input>` editors commit via their
// own onBlur, but the injected widgets (text, number, date, lookup, …) have no
// such handler — a document-level pointerdown listener (see below) uses this
// node to detect click-outside and commit them. Null ⇒ no injected editor is
// active (a built-in editor, or nothing, is showing).
// captured while it's mounted, so the document-level pointerdown listener
// below can tell "inside this editor" from "outside" and exit edit mode.
// Null ⇒ no injected editor is active (a built-in editor, or nothing, is
// showing).
//
// This used to justify itself with "the injected widgets (text, number, date,
// lookup, …) have no such handler". That claim is no longer true and is no
// longer the reason (objectui#6859). `onBlur` is a DECLARED DOM pass-through
// key — named in `FieldWidgetDomProps` (`@object-ui/fields`), named in
// `SDUI_DOM_PASS_THROUGH_KEYS` (`@object-ui/core`), forwarded by
// `toDomProps` — and every widget reachable as an inline editor spreads that
// whitelist onto a real control (26 of the 27 components in `EDIT_WIDGETS`
// call `toDomProps` directly; `UserField` delegates its whole props object to
// `LookupField`, which does). The five widgets that own a blur handler now
// COMPOSE the host's rather than overriding it (objectui#6780, #6802).
//
// The listener is still needed, for a different reason: NOTHING EVER HANDS
// THE WIDGET ONE. The wrapper below carries `onKeyDown` alone, and the
// context object `renderCellEditor` receives — `{ column, row, value, stage,
// commit, cancel }` — has no DOM-props slot to put an `onBlur` in. The
// in-repo factory behind that seam, `@object-ui/fields`' `FieldEditWidget`,
// forwards `autoFocus` and nothing else out of the DOM block, so a host
// handler could not reach the control through it even if one were passed.
//
// Note also what the listener is NOT load-bearing for. Its job is exiting
// EDIT MODE, not rescuing the value: injected widgets stage on every change
// (the host wires the widget's `onChange` to `stageEdit` below), so a typed
// value is already in `pendingChanges` before any exit event — measured in a
// real browser on the text, date and number editors for objectui#6859.
// Retiring this listener would strand cells in edit mode; it would not drop
// edits.
const injectedEditorElRef = useRef<HTMLDivElement | null>(null);
// Snapshot of the active cell's pending value when editing began, so Escape /
// cancel can revert this session's changes. Injected widgets stage on every
Expand DownExpand Up@@ -1728,10 +1754,12 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {

// Commit a host-injected widget editor on click-outside (objectui#2321).
//
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur), but
// the widgets injected through `renderCellEditor` (text, number, date, lookup,
// …) have no such handler, so without this they stay stuck in edit mode when
// the user clicks away. A capture-phase document listener (capture so a cell's
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur). The
// widgets injected through `renderCellEditor` (text, number, date, lookup, …)
// never receive one — not because they cannot deliver it (they can, and do:
// see `injectedEditorElRef` above and objectui#6859) but because nothing on
// this seam passes it to them — so without this they stay stuck in edit mode
// when the user clicks away. A capture-phase document listener (capture so a cell's
// own `stopPropagation` can't hide it) commits the staged value and exits edit
// mode when the pointer goes down truly outside the editor — but NOT inside a
// Radix overlay the widget itself opened (a lookup popover / record-picker
Expand DownExpand Up@@ -2302,6 +2330,15 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// picker's `<button>` trigger or a multi-line
// textarea it's left alone so Enter opens the
// dropdown / inserts a newline as usual.
//
// Tab is deliberately NOT in that list, and
// tabbing out therefore does not leave edit
// mode — measured, objectui#6859. It costs
// nothing: the widget has already staged
// every keystroke into `pendingChanges`, so
// the value is safe; the cell simply stays
// open until Enter, Escape, or a pointer
// press outside closes it.
return (
<div
ref={(n) => { injectedEditorElRef.current = n; }}
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); docs(components): the injected-editor commit justification is stale — correct it, and pin what Tab-out actually does by claude[bot] · Pull Request #6912 · objectstack-ai/objectui · GitHub
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
21 changes: 21 additions & 0 deletions .changeset/6859-injected-editor-commit-comment.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
---

No behaviour change, and deliberately so: this is the record being corrected, not the
renderer (objectui#6859).

The data table's document-level `pointerdown` listener — the one that exits a host-injected
inline cell editor when you click away — justified itself with "the injected widgets (text,
number, date, lookup, …) have no such handler". That has not been true since objectui#6780 /
#6802: `onBlur` is a declared DOM pass-through key, and all 27 widgets reachable as an inline
editor deliver it to a real control (26 spread `toDomProps` themselves, `UserField` delegates
to `LookupField`).

A source audit read the same absence as silent DATA LOSS on Tab-out. It is not. Driven in a
real browser against the real widgets, a value typed into a text, date or number cell editor
survives tabbing away, and reads back intact: the host wires each widget's `onChange` to the
table's `stage`, so every keystroke is already in `pendingChanges` while the editor is still
open. The listener exits EDIT MODE; it never rescued the value. Tabbing out does leave the
cell in edit mode until Enter, Escape, or a pointer press outside — a wart, not a lost edit.

The comment now says all of that, and both facts are pinned by tests so they cannot rot back.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
/**
* 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.
*/

/**
* CHARACTERIZATION — what actually happens when focus leaves a HOST-INJECTED
* cell editor (objectui#6859).
*
* ## Why this file exists
*
* `data-table.tsx` exits an injected editor through a document-level
* `pointerdown` listener, and the comment at `injectedEditorElRef` used to
* justify that with "the injected widgets (text, number, date, lookup, …) have
* no such handler". That justification is stale — `onBlur` is a declared DOM
* pass-through key every inline-edit widget forwards (objectui#6780, #6802) —
* and correcting a comment leaves nothing behind that can rot loudly. A source
* audit then read the same absence as a DATA-LOSS defect: no `focusout`, no
* `onBlurCapture`, no `relatedTarget` anywhere in the file ⇒ Tab out of an
* injected editor must silently drop the typed value.
*
* It does not, and this file is the measurement that says why. The value never
* depends on the exit event: the host wires the widget's `onChange` to `stage`,
* so every keystroke is already in `pendingChanges` while the editor is still
* open. The `pointerdown` listener exits EDIT MODE; it does not rescue values.
*
* The same sequence was driven in a real Chromium against the real
* `@object-ui/fields` widgets (text / date / number) before this file was
* written; these are the jsdom pins for CI.
*
* ## The control
*
* Test A is the control and must stay green: a BUILT-IN editor DOES commit on
* focus loss. Without it, test C's negative ("Tab commits nothing") would pass
* just as well on a harness that cannot observe a commit at all.
*/
import { describe, it, expect, vi } from 'vitest';
import { fireEvent, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import '@testing-library/jest-dom';
import React from 'react';
import { renderComponent } from './test-utils';
// Module scope, not a hook — see object-ui/no-dynamic-import-in-test-hook.
import '../renderers';

const baseSchema = {
type: 'data-table' as const,
editable: true,
singleClickEdit: true,
columns: [
{ header: 'Name', accessorKey: 'name', editable: false },
{ header: 'Qty', accessorKey: 'qty', type: 'number' },
],
data: [{ id: '1', name: 'row-one', qty: '' }],
} as any;

/**
* The injected editor exactly as the in-repo host builds it
* (`ObjectGrid.renderCellEditor`): a real control whose `onChange` is wired to
* the context's `stage` — non-discrete field types stage, they do not commit.
* `packages/components` does not depend on `@object-ui/fields`, so the wiring
* is reproduced rather than imported; the browser run linked above is what
* pins it to the real widgets.
*/
const stagingEditor = ({ column, value, stage }: any) =>
column.accessorKey === 'qty' ? (
<input
data-testid="injected-editor"
value={value ?? ''}
onChange={(e) => stage(e.target.value)}
/>
) : null;

/** A tabbable element outside the table, so `userEvent.tab()` has somewhere to go. */
function withOutsideTabStop(): HTMLButtonElement {
const btn = document.createElement('button');
btn.id = 'outside-tab-stop';
btn.textContent = 'outside';
document.body.appendChild(btn);
return btn;
}

describe('data-table — focus loss on a host-injected cell editor (objectui#6859)', () => {
it('A) CONTROL: a BUILT-IN editor DOES commit when focus leaves it', async () => {
// Proves the harness can observe a commit driven by focus loss at all, so
// the negative results below are measurements and not dead probes.
const onCellChange = vi.fn();
const { container } = renderComponent({ ...baseSchema, onCellChange });

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);

const input = qtyCell.querySelector('input') as HTMLInputElement;
expect(input).toBeTruthy();
fireEvent.change(input, { target: { value: '42' } });
fireEvent.blur(input);

expect(onCellChange).toHaveBeenCalledWith(0, 'qty', '42', expect.anything());
});

it('B) an injected editor stages every keystroke — the value is pending BEFORE any exit event', () => {
// This is the path the source audit could not see, and the reason Tab-out
// is not lossy: `stage` writes straight into `pendingChanges` while the
// editor is still open and still focused.
const { container, getByText, queryByText } = renderComponent({
...baseSchema,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
// Nothing pending yet — the toolbar's save affordance is the readout.
expect(queryByText(/Save All/i)).toBeNull();

fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
expect(editor).toBeTruthy();
fireEvent.change(editor, { target: { value: 'TYPED' } });

// Still in edit mode, nothing committed — and the value is already staged.
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
expect(getByText(/Save All/i)).toBeInTheDocument();
});

it('C) tabbing out of an injected editor neither commits nor leaves edit mode', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
expect(document.activeElement).toBe(editor);
await userEvent.tab();

// Focus really left the editor …
expect(document.activeElement).not.toBe(editor);
// … and nothing committed: no exit, no onCellChange. This is the fact the
// corrected comment at `injectedEditorElRef` now states, and the reason
// the document-level pointerdown listener is still load-bearing.
expect(onCellChange).not.toHaveBeenCalled();
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeTruthy();
} finally {
outside.remove();
}
});

it('D) …and the typed value is still there — the outside pointer press commits exactly it', async () => {
const onCellChange = vi.fn();
const outside = withOutsideTabStop();
try {
const { container } = renderComponent({
...baseSchema,
onCellChange,
renderCellEditor: stagingEditor,
});

const qtyCell = container.querySelectorAll('tbody td')[1] as HTMLElement;
fireEvent.click(qtyCell);
const editor = qtyCell.querySelector('[data-testid="injected-editor"]') as HTMLInputElement;
fireEvent.change(editor, { target: { value: 'TYPED' } });

editor.focus();
await userEvent.tab();
// A pointer press truly outside is what exits the editor today.
fireEvent.pointerDown(outside);

await waitFor(() =>
expect(onCellChange).toHaveBeenCalledWith(0, 'qty', 'TYPED', expect.anything()),
);
// Edit mode is over and the cell reads back what was typed — nothing lost
// across the Tab-out.
await waitFor(() =>
expect(qtyCell.querySelector('[data-testid="injected-editor"]')).toBeNull(),
);
expect(qtyCell.textContent).toContain('TYPED');
} finally {
outside.remove();
}
});
});
55 changes: 46 additions & 9 deletions packages/components/src/renderers/complex/data-table.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1002,11 +1002,37 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// don't double-commit (Enter) or resurrect a cancelled value (Escape).
const skipBlurSaveRef = useRef(false);
// DOM node of a host-injected widget editor (rendered via `renderCellEditor`),
// captured while it's mounted. The built-in `<input>` editors commit via their
// own onBlur, but the injected widgets (text, number, date, lookup, …) have no
// such handler — a document-level pointerdown listener (see below) uses this
// node to detect click-outside and commit them. Null ⇒ no injected editor is
// active (a built-in editor, or nothing, is showing).
// captured while it's mounted, so the document-level pointerdown listener
// below can tell "inside this editor" from "outside" and exit edit mode.
// Null ⇒ no injected editor is active (a built-in editor, or nothing, is
// showing).
//
// This used to justify itself with "the injected widgets (text, number, date,
// lookup, …) have no such handler". That claim is no longer true and is no
// longer the reason (objectui#6859). `onBlur` is a DECLARED DOM pass-through
// key — named in `FieldWidgetDomProps` (`@object-ui/fields`), named in
// `SDUI_DOM_PASS_THROUGH_KEYS` (`@object-ui/core`), forwarded by
// `toDomProps` — and every widget reachable as an inline editor spreads that
// whitelist onto a real control (26 of the 27 components in `EDIT_WIDGETS`
// call `toDomProps` directly; `UserField` delegates its whole props object to
// `LookupField`, which does). The five widgets that own a blur handler now
// COMPOSE the host's rather than overriding it (objectui#6780, #6802).
//
// The listener is still needed, for a different reason: NOTHING EVER HANDS
// THE WIDGET ONE. The wrapper below carries `onKeyDown` alone, and the
// context object `renderCellEditor` receives — `{ column, row, value, stage,
// commit, cancel }` — has no DOM-props slot to put an `onBlur` in. The
// in-repo factory behind that seam, `@object-ui/fields`' `FieldEditWidget`,
// forwards `autoFocus` and nothing else out of the DOM block, so a host
// handler could not reach the control through it even if one were passed.
//
// Note also what the listener is NOT load-bearing for. Its job is exiting
// EDIT MODE, not rescuing the value: injected widgets stage on every change
// (the host wires the widget's `onChange` to `stageEdit` below), so a typed
// value is already in `pendingChanges` before any exit event — measured in a
// real browser on the text, date and number editors for objectui#6859.
// Retiring this listener would strand cells in edit mode; it would not drop
// edits.
const injectedEditorElRef = useRef<HTMLDivElement | null>(null);
// Snapshot of the active cell's pending value when editing began, so Escape /
// cancel can revert this session's changes. Injected widgets stage on every
Expand DownExpand Up@@ -1728,10 +1754,12 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {

// Commit a host-injected widget editor on click-outside (objectui#2321).
//
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur), but
// the widgets injected through `renderCellEditor` (text, number, date, lookup,
// …) have no such handler, so without this they stay stuck in edit mode when
// the user clicks away. A capture-phase document listener (capture so a cell's
// Built-in `<input>` editors commit via their own onBlur (handleEditBlur). The
// widgets injected through `renderCellEditor` (text, number, date, lookup, …)
// never receive one — not because they cannot deliver it (they can, and do:
// see `injectedEditorElRef` above and objectui#6859) but because nothing on
// this seam passes it to them — so without this they stay stuck in edit mode
// when the user clicks away. A capture-phase document listener (capture so a cell's
// own `stopPropagation` can't hide it) commits the staged value and exits edit
// mode when the pointer goes down truly outside the editor — but NOT inside a
// Radix overlay the widget itself opened (a lookup popover / record-picker
Expand DownExpand Up@@ -2302,6 +2330,15 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
// picker's `<button>` trigger or a multi-line
// textarea it's left alone so Enter opens the
// dropdown / inserts a newline as usual.
//
// Tab is deliberately NOT in that list, and
// tabbing out therefore does not leave edit
// mode — measured, objectui#6859. It costs
// nothing: the widget has already staged
// every keystroke into `pendingChanges`, so
// the value is safe; the cell simply stays
// open until Enter, Escape, or a pointer
// press outside closes it.
return (
<div
ref={(n) => { injectedEditorElRef.current = n; }}
Expand Down
Loading
Loading