Skip to content

Commit 411fc00

Browse files
fix(plugin-detail): reveal the inline-edit pencil on keyboard focus (#26325)
Part of #25883 Clause-②: no Metadata-protocol: none Item 1 of #25883: the record page's read-mode inline-edit pencil now shows on keyboard focus, not only on hover. Item 2 (remember the empty-field toggle) has an open storage-key choice and stays on the card; #25883 remains open. ## What changed - `ui/packages/plugin-detail/src/DetailSection.tsx`, the details body: the pencil beside an editable field was `opacity-0 group-hover:opacity-100`. It is a real button, so a Tab stop landed on a control the user could not see, and the shared `Button` focus ring was drawn at opacity 0. It now also carries `group-focus-within:opacity-100 focus-visible:opacity-100`. - `ui/packages/plugin-detail/src/HeaderHighlight.tsx`, the highlights strip: the same pencil gets the same two utilities. - The spelling is the one `RowActionMenu` (plugin-grid) and `data-table` (components) already use for their hover-revealed row buttons. `focus-visible` reveals the pencil while it has keyboard focus. `group-focus-within` reveals it while anything in its own field has focus, for example a lookup's link in the value. - Unchanged: the hover reveal, the row's hover background and `title` hint, double-click to edit, the pencil's accessible name (`detail.editInlineHint`), and every locale pack. - `scripts/sdui-manifest.sources/plugin-detail.json` was re-recorded with `node scripts/gen-sdui-manifest-node.mjs` under the verify lock. The artefact is unchanged; only this package's record moved. - `.changeset/25883-pencil-focus-reveal.md`: `@object-ui/plugin-detail` patch. ## Tests New: `ui/packages/plugin-detail/src/__tests__/inlineEditPencil.focusReveal-25883.test.tsx`, 8 tests, covering both places. jsdom has no layout and no CSS engine, so it cannot measure "visible on focus". The tests pin the contract a browser turns into visibility: - the pencil is an enabled, focusable `button` and takes focus; - it carries `focus-visible:opacity-100` and `group-focus-within:opacity-100` next to the unchanged `opacity-0 group-hover:opacity-100`; - the shared Button's `focus-visible:ring-2` survives the className merge; - the nearest `.group` ancestor is the pencil's own field, so focus in one field reveals only that field's pencil. Activation of the focused pencil, double-click on the value, and no pencil on computed or non-editable fields are asserted as behaviour. The pencil is found by its icon, not by its localized label. - **Red first, at base `2409c9c8d7` with the fix absent:** `Tests 2 failed | 6 passed (8)`. Both failures are at the `focus-visible:opacity-100` assertion, one for `DetailSection` and one for `HeaderHighlight`. Focusability, hover and double-click were already green there, which is the defect: a focusable but invisible control. - **Green, at head `76d5a63f`** (the fix, plus `origin/main` 6633075 merged in), from cwd `ui/` under `scripts/pm/os-verify-lock.sh`: - `pnpm exec vitest run packages/plugin-detail --maxWorkers=2`: `Test Files 305 passed | 1 skipped (306)`, `Tests 2833 passed | 18 skipped (2851)`. All 306 test files under plugin-detail ran, the new one included. The same run on the pre-merge head `55fe5e19` read the same counts. - `pnpm --filter @object-ui/plugin-detail run type-check` (both `tsc --noEmit` and `tsc -p tsconfig.test.json`, so the new test is compiled): exit 0. - `node scripts/check-sdui-manifest.mjs`: exit 0. The artefact is unchanged, and 27 package records are current. ## Gates Derived with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at head `76d5a63f`: 45 commands. - 44 run locally, each exit 0, including `node scripts/check-sdui-manifest.mjs`, `pnpm check:nul-bytes`, `pnpm check:cross-package-test-inputs`, `pnpm check:test-source-alias`, `pnpm check:type-check-coverage` and the changeset gates. - NOT-MEASURED: `pnpm check:type-check-debt`. Its re-measure waits on the shared verify lock for a closure build and then runs tsc over every package. It was still waiting on the lock at a 300 s cap on a contended box, so it is left to CI's `Lint & Repo Gates`. - `--ran` reconciliation: `45 derived famil(ies) accounted for — 44 run, 1 NOT-MEASURED`, exit 0. - CI runs every other family (lint, the type-check lanes, Build and E2E UI). This PR cites CI rather than re-running them locally. ## Acceptance notes - **Copy button, same hover-only class, not changed here.** In `DetailSection.tsx` the copy button drawn beside a read-only, copyable field has the same `opacity-0 group-hover:opacity-100` and is also a Tab stop. The claim scopes this PR to the pencils, so it is reported to the seat rather than fixed here. - **The pencil's tooltip text.** The tooltip, which Radix opens on keyboard focus, reads `detail.editInlineHint` ("Double-click to edit"). A keyboard user activates the pencil with Enter or Space. Locale packs are out of this claim's scope; this is left as an open question to the seat. - **Touch.** The pencil is still not revealed on a coarse pointer. `RowActionMenu` adds `pointer-coarse:opacity-100` for that case. The mobile read row's missing edit path is out of this claim's scope. - **Landing order.** On the plugin-detail line this lands after #26055 and #26277. Neither touches `DetailSection.tsx` or `HeaderHighlight.tsx`. --- _Generated by [Claude Code](https://claude.ai/code/session_01SMps7wq7rBTk53e8KTSEup)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d15f99d commit 411fc00

4 files changed

Lines changed: 224 additions & 2 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@object-ui/plugin-detail': patch
3+
---
4+
5+
fix(plugin-detail): the record page's inline-edit pencil shows on keyboard focus, not only on hover
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
In read mode, the pencil beside an editable field (in the details body and in the highlights strip) was transparent until the pointer hovered its field. The pencil is a real button, so a keyboard user tabbing through the record landed on a control they could not see, focus ring included. It now also shows while it has keyboard focus, or while anything else in its field does. Hover, the row's hint and double-click to edit are unchanged.
11+
12+
What to do: nothing.

‎ui/packages/plugin-detail/src/DetailSection.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,10 @@ export const DetailSection: React.FC<DetailSectionProps> = ({
747747
<Button
748748
variant="ghost"
749749
size="icon"
750-
className="h-6 w-6 opacity-0 group-hover:opacity-100 transition-opacity shrink-0"
750+
// A real Tab stop, so it is revealed on keyboard focus as
751+
// well as on hover — itself, or anything in its field
752+
// (objectstack#25883). The Button's own ring marks it.
753+
className="h-6 w-6 opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 focus-visible:opacity-100 transition-opacity shrink-0"
751754
aria-label={t('detail.editInlineHint')}
752755
onClick={(e) => {
753756
e.stopPropagation();

‎ui/packages/plugin-detail/src/HeaderHighlight.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,9 @@ export const HeaderHighlight: React.FC<HeaderHighlightProps> = ({
448448
<Button
449449
variant="ghost"
450450
size="icon"
451-
className="h-5 w-5 shrink-0 opacity-0 group-hover:opacity-100 transition-opacity"
451+
// Revealed on keyboard focus too, as in the details
452+
// body (objectstack#25883).
453+
className="h-5 w-5 shrink-0 opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 focus-visible:opacity-100 transition-opacity"
452454
aria-label={t('detail.editInlineHint')}
453455
onClick={(e) => {
454456
e.stopPropagation();
Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
import { describe, it, expect, vi, beforeAll } from 'vitest';
10+
import { render, fireEvent, screen } from '@testing-library/react';
11+
import { InlineEditProvider } from '@object-ui/react';
12+
import type { DetailViewSection } from '@object-ui/types';
13+
import { DetailSection } from '../DetailSection';
14+
import { HeaderHighlight } from '../HeaderHighlight';
15+
16+
/**
17+
* objectstack#25883 item 1 — the read-mode inline-edit pencil was revealed on
18+
* HOVER only (`opacity-0 group-hover:opacity-100`). It is a real `<button>`, so
19+
* a keyboard Tab stop landed on it while it stayed fully transparent: the
20+
* focus ring the shared `Button` draws was painted at opacity 0.
21+
*
22+
* The pencil must now also show while it, or anything else in its field, has
23+
* keyboard focus, in both places it is drawn: the details body
24+
* (`DetailSection`) and the highlights strip (`HeaderHighlight`). Hover and
25+
* double-click behave exactly as before.
26+
*
27+
* jsdom has no layout and no CSS engine, so "visible on focus" cannot be
28+
* measured here. What these tests pin is the contract a browser turns into
29+
* visibility: the pencil is a focusable button, it carries the focus-reveal
30+
* utilities next to the unchanged hover reveal, the shared focus ring is still
31+
* on it, and the `group` its `group-*` variants resolve against is its own
32+
* field. Activation and double-click are asserted behaviourally.
33+
*
34+
* The pencil is found by its icon, not by its localized label, so the tests
35+
* hold in every locale.
36+
*/
37+
38+
const pencilButtons = (root: HTMLElement): HTMLButtonElement[] =>
39+
Array.from(root.querySelectorAll('svg.lucide-pencil')).map(
40+
(svg) => svg.closest('button') as HTMLButtonElement,
41+
);
42+
43+
function expectFocusRevealContract(pencil: HTMLButtonElement) {
44+
// A real, enabled, tabbable control: the Tab stop the defect is about.
45+
expect(pencil.tagName).toBe('BUTTON');
46+
expect(pencil).not.toBeDisabled();
47+
expect(pencil.getAttribute('tabindex')).not.toBe('-1');
48+
pencil.focus();
49+
expect(pencil).toHaveFocus();
50+
51+
const cls = pencil.classList;
52+
// Hidden at rest, revealed on hover: unchanged.
53+
expect(cls.contains('opacity-0')).toBe(true);
54+
expect(cls.contains('group-hover:opacity-100')).toBe(true);
55+
// Revealed on keyboard focus of the pencil itself …
56+
expect(cls.contains('focus-visible:opacity-100')).toBe(true);
57+
// … and while focus is anywhere inside its field (e.g. a lookup's link).
58+
expect(cls.contains('group-focus-within:opacity-100')).toBe(true);
59+
// The shared Button's focus ring survives the className merge, so the
60+
// revealed pencil also shows where focus is.
61+
expect(cls.contains('focus-visible:ring-2')).toBe(true);
62+
63+
// `group-*` variants resolve against the nearest `.group` ancestor; it must
64+
// be this field's own wrapper, so focus in one field reveals only its pencil.
65+
const group = pencil.closest('.group');
66+
expect(group).not.toBeNull();
67+
expect(pencilButtons(group as HTMLElement)).toEqual([pencil]);
68+
}
69+
70+
describe('DetailSection read-mode pencil — keyboard focus reveal (#25883)', () => {
71+
beforeAll(() => {
72+
// useIsMobile() keys off window.innerWidth; the desktop row carries the pencil.
73+
Object.defineProperty(window, 'innerWidth', { configurable: true, value: 1280 });
74+
});
75+
76+
const objectSchema = {
77+
fields: {
78+
title: { type: 'text' },
79+
city: { type: 'text' },
80+
score: { type: 'formula' },
81+
},
82+
};
83+
const section: DetailViewSection = {
84+
fields: [
85+
{ name: 'title', label: 'Title' },
86+
{ name: 'city', label: 'City' },
87+
{ name: 'score', label: 'Score' },
88+
],
89+
} as DetailViewSection;
90+
const data = { title: 'Hello', city: 'Paris', score: 42 };
91+
92+
it('draws one focus-revealed pencil per editable field, none on a computed one', () => {
93+
const { container } = render(
94+
<DetailSection
95+
section={section}
96+
data={data}
97+
objectSchema={objectSchema}
98+
onEnterInlineEdit={vi.fn()}
99+
/>,
100+
);
101+
const pencils = pencilButtons(container);
102+
expect(pencils).toHaveLength(2); // title + city; the formula has none
103+
pencils.forEach(expectFocusRevealContract);
104+
});
105+
106+
it('activating the focused pencil enters inline edit on its own field', () => {
107+
const onEnter = vi.fn();
108+
const { container } = render(
109+
<DetailSection
110+
section={section}
111+
data={data}
112+
objectSchema={objectSchema}
113+
onEnterInlineEdit={onEnter}
114+
/>,
115+
);
116+
const [titlePencil, cityPencil] = pencilButtons(container);
117+
cityPencil.focus();
118+
// Enter / Space on a native <button> dispatch `click`.
119+
fireEvent.click(cityPencil);
120+
expect(onEnter).toHaveBeenCalledTimes(1);
121+
expect(onEnter).toHaveBeenCalledWith('city');
122+
expect(titlePencil).not.toHaveFocus();
123+
});
124+
125+
it('double-click on the value still enters inline edit', () => {
126+
const onEnter = vi.fn();
127+
render(
128+
<DetailSection
129+
section={section}
130+
data={data}
131+
objectSchema={objectSchema}
132+
onEnterInlineEdit={onEnter}
133+
/>,
134+
);
135+
fireEvent.doubleClick(screen.getByText('Hello'));
136+
expect(onEnter).toHaveBeenCalledWith('title');
137+
});
138+
139+
it('draws no pencil when the record offers no inline edit', () => {
140+
const { container } = render(
141+
<DetailSection section={section} data={data} objectSchema={objectSchema} />,
142+
);
143+
expect(pencilButtons(container)).toHaveLength(0);
144+
});
145+
});
146+
147+
describe('HeaderHighlight pencil — keyboard focus reveal (#25883)', () => {
148+
beforeAll(() => {
149+
Object.defineProperty(window, 'innerWidth', { configurable: true, value: 1280 });
150+
});
151+
152+
const objectSchema = {
153+
fields: {
154+
owner: { type: 'text' },
155+
score: { type: 'formula' },
156+
},
157+
};
158+
const fields = [
159+
{ name: 'owner', label: 'Owner' },
160+
{ name: 'score', label: 'Score' },
161+
] as any;
162+
const data = { owner: 'Alice', score: '99' };
163+
164+
it('draws a focus-revealed pencil on the editable highlight only', () => {
165+
const { container } = render(
166+
<InlineEditProvider canEdit>
167+
<HeaderHighlight fields={fields} data={data} objectSchema={objectSchema} />
168+
</InlineEditProvider>,
169+
);
170+
const pencils = pencilButtons(container);
171+
expect(pencils).toHaveLength(1); // owner; the formula has none
172+
expectFocusRevealContract(pencils[0]);
173+
});
174+
175+
it('activating the focused pencil enters the shared edit session', () => {
176+
const { container } = render(
177+
<InlineEditProvider canEdit>
178+
<HeaderHighlight fields={fields} data={data} objectSchema={objectSchema} />
179+
</InlineEditProvider>,
180+
);
181+
const [pencil] = pencilButtons(container);
182+
pencil.focus();
183+
fireEvent.click(pencil);
184+
expect(screen.getByDisplayValue('Alice')).toBeInTheDocument();
185+
});
186+
187+
it('double-click on the highlight still enters the shared edit session', () => {
188+
render(
189+
<InlineEditProvider canEdit>
190+
<HeaderHighlight fields={fields} data={data} objectSchema={objectSchema} />
191+
</InlineEditProvider>,
192+
);
193+
fireEvent.doubleClick(screen.getByText('Alice'));
194+
expect(screen.getByDisplayValue('Alice')).toBeInTheDocument();
195+
});
196+
197+
it('draws no pencil when the record is not editable', () => {
198+
const { container } = render(
199+
<InlineEditProvider canEdit={false}>
200+
<HeaderHighlight fields={fields} data={data} objectSchema={objectSchema} />
201+
</InlineEditProvider>,
202+
);
203+
expect(pencilButtons(container)).toHaveLength(0);
204+
});
205+
});

0 commit comments

Comments
 (0)