Skip to content

Commit b53322d

Browse files
fix(plugin-detail): reveal the read-only field copy button on keyboard focus (#26487)
Fixes #26329 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:记录详情页(桌面阅读模式)只读字段旁的「复制」按钮,除了鼠标悬停,现在键盘 Tab 聚焦到它或它所在的字段时也会显示。只改了一个 className,另加一个测试和一个 patch changeset。 - 为什么改:这个按钮是真实可聚焦的按钮,但之前只在悬停时显示,键盘用户按 Tab 会停在一个看不见的控件上。旁边的编辑铅笔已在 #26325 用同样写法处理,这次是它的复制按钮同类项。 - 风险与回滚:只加了两个 Tailwind 显示类,悬停、点击复制、按钮名称都不变。回滚就是 revert 这一行。 - 先看:`ui/packages/plugin-detail/src/DetailSection.tsx` 复制按钮的 className(与上方铅笔按钮写法一致)。 - 两分钟验证:打开任意记录详情页,用 Tab 键在只读字段间移动,复制图标应随焦点出现;鼠标悬停行为与之前相同。 - 需要你做的:无。不涉及受管面,CI 绿后由席位按常规合并队列落地。 ## What changed - `ui/packages/plugin-detail/src/DetailSection.tsx`: the copy-to-clipboard button drawn beside a read-only, copyable field in desktop read mode was `opacity-0 group-hover:opacity-100`. It is a real button, so a keyboard Tab stop landed on a control the user could not see, and the shared `Button` focus ring was painted at opacity 0. It now also carries `group-focus-within:opacity-100 focus-visible:opacity-100`, the spelling the inline-edit pencil a few lines above already uses (PR #26325), as do `RowActionMenu` (plugin-grid) and `data-table` (components). - `focus-visible` reveals the button while it has keyboard focus. `group-focus-within` reveals it while anything in its own field has focus, which includes the field's own click-to-copy row (itself a Tab stop with `role="button"`). - Unchanged: the hover reveal, click-to-copy on the row and on the button, the button's accessible name (`detail.copyToClipboard`), the copied-state icon, the mobile grouped-inset row, and every locale pack. - `.changeset/26329-copy-button-focus-reveal.md`: `@object-ui/plugin-detail` patch. - No sdui record is written; `node scripts/check-sdui-manifest.mjs` exits 0 and no registry component contract changed. ## Tests New: `ui/packages/plugin-detail/src/__tests__/copyButton.focusReveal-26329.test.tsx`, 4 tests, modelled on the pencil's pin `inlineEditPencil.focusReveal-25883.test.tsx`. 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 copy button 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 button's own field, so focus in one field reveals only that field's button; - the field's click-to-copy row is a Tab stop inside that same group. Activating the focused button copies its own field's value (`navigator.clipboard.writeText` receives `Paris`), an empty field offers no copy button, and an editable field draws the pencil instead. The button is found by its icon, not by its localized label. - **Red first, at `df130cf0` (the pin committed, the fix absent):** `Tests 1 failed | 3 passed (4)`. The failure is at the `focus-visible:opacity-100` assertion; focusability and the hover reveal were already green there, which is the defect: a focusable but invisible control. - **Green, at head `2f49b3e0`**, from cwd `ui/` under `scripts/pm/os-verify-lock.sh`: - `pnpm exec vitest run packages/plugin-detail --maxWorkers=2`: `Test Files 307 passed | 1 skipped (308)`, `Tests 2857 passed | 18 skipped (2875)`. The new file alone: `Tests 4 passed (4)`. - `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. - The plugin-detail dependency closure was built first, under the lock: `pnpm --filter '@object-ui/plugin-detail^...' build`, VERDICT command-exit 0. ## Gates Derived with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at head `2f49b3e0`: 34 commands, each run locally, each exit 0. They include `node scripts/check-sdui-manifest.mjs` (and its `--self-test`), `pnpm check:nul-bytes`, `pnpm check:cross-package-test-inputs`, `pnpm check:test-source-alias`, `pnpm check:type-check-coverage`, `pnpm check:type-check-debt` (re-measure under the lock: 0 raw tsc errors), `pnpm --dir ui exec vitest run scripts/` (`Tests 5097 passed | 2 skipped`) and the changeset gates. `--ran` reconciliation with exit codes recorded: `34 derived famil(ies) accounted for — 34 run, 0 NOT-MEASURED (a DERIVED zero)`, 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 - **Touch.** Neither the copy button nor the pencil is revealed on a coarse pointer (`RowActionMenu` adds `pointer-coarse:opacity-100`). That belongs with the mobile read row's edit path and is out of this claim's file surface. - **The pencil's tooltip text** ("Double-click to edit") is unchanged here; it needs a new key in every locale pack and stays with #25883. - **Landing order.** This is the same file as #25883 item 2, which the seat holds behind this card. --- _Generated by [Claude Code](https://claude.ai/code/session_01SMps7wq7rBTk53e8KTSEup)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e5131cf commit b53322d

3 files changed

Lines changed: 164 additions & 1 deletion

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): reveal the read-only field's copy button on keyboard focus
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
On a record page in desktop read mode, the copy-to-clipboard button beside a read-only, copyable field was shown on hover only. It is a real button, so a keyboard user's Tab stop landed on a control they could not see. It is now also shown while it has keyboard focus, or while anything else in its field has focus (for example the field's own click-to-copy row), in the same way as the inline-edit pencil beside it. Hover, click-to-copy and the button's accessible name are unchanged.
11+
12+
What to do: nothing.

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -772,7 +772,9 @@ export const DetailSection: React.FC<DetailSectionProps> = ({
772772
<Button
773773
variant="ghost"
774774
size="icon"
775-
className="h-6 w-6 opacity-0 group-hover:opacity-100 transition-opacity shrink-0"
775+
// A real Tab stop, so it is revealed on keyboard focus as
776+
// well as on hover, like the pencil above (objectstack#26329).
777+
className="h-6 w-6 opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 focus-visible:opacity-100 transition-opacity shrink-0"
776778
// objectui#12625 — an icon-only button; the tooltip is
777779
// closed at rest, so without this it has no name.
778780
aria-label={t('detail.copyToClipboard')}
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
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, afterEach } from 'vitest';
10+
import { render, fireEvent, cleanup } from '@testing-library/react';
11+
import type { DetailViewSection } from '@object-ui/types';
12+
import { DetailSection } from '../DetailSection';
13+
14+
/**
15+
* objectstack#26329 — the read-mode copy-to-clipboard button on a copyable,
16+
* non-editable field was revealed on HOVER only (`opacity-0
17+
* group-hover:opacity-100`). It is a real `<button>`, so a keyboard Tab stop
18+
* landed on it while it stayed fully transparent: the focus ring the shared
19+
* `Button` draws was painted at opacity 0. The inline-edit pencil beside it had
20+
* the same defect and was fixed the same way (objectstack#25883; its pin is
21+
* `inlineEditPencil.focusReveal-25883.test.tsx`).
22+
*
23+
* The copy button must now also show while it, or anything else in its field
24+
* (the field's own click-to-copy row is a Tab stop too), has keyboard focus.
25+
* Hover behaves 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 copy button is a focusable button, it carries the
30+
* focus-reveal utilities next to the unchanged hover reveal, the shared focus
31+
* ring is still on it, and the `group` its `group-*` variants resolve against
32+
* is its own field. Activation is asserted behaviourally.
33+
*
34+
* The copy button is found by its icon, not by its localized label, so the
35+
* tests hold in every locale.
36+
*/
37+
38+
const copyButtons = (root: HTMLElement): HTMLButtonElement[] =>
39+
Array.from(root.querySelectorAll('svg.lucide-copy')).map(
40+
(svg) => svg.closest('button') as HTMLButtonElement,
41+
);
42+
43+
function expectFocusRevealContract(copy: HTMLButtonElement) {
44+
// A real, enabled, tabbable control: the Tab stop the defect is about.
45+
expect(copy.tagName).toBe('BUTTON');
46+
expect(copy).not.toBeDisabled();
47+
expect(copy.getAttribute('tabindex')).not.toBe('-1');
48+
copy.focus();
49+
expect(copy).toHaveFocus();
50+
51+
const cls = copy.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 copy button itself …
56+
expect(cls.contains('focus-visible:opacity-100')).toBe(true);
57+
// … and while focus is anywhere inside its field (e.g. its click-to-copy row).
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 button 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 button.
65+
const group = copy.closest('.group');
66+
expect(group).not.toBeNull();
67+
expect(copyButtons(group as HTMLElement)).toEqual([copy]);
68+
}
69+
70+
describe('DetailSection read-mode copy button — keyboard focus reveal (#26329)', () => {
71+
beforeAll(() => {
72+
// useIsMobile() keys off window.innerWidth; the desktop row carries the button.
73+
Object.defineProperty(window, 'innerWidth', { configurable: true, value: 1280 });
74+
});
75+
76+
afterEach(() => {
77+
cleanup();
78+
});
79+
80+
const objectSchema = {
81+
fields: {
82+
title: { type: 'text' },
83+
city: { type: 'text' },
84+
notes: { type: 'text' },
85+
},
86+
};
87+
const section: DetailViewSection = {
88+
fields: [
89+
{ name: 'title', label: 'Title' },
90+
{ name: 'city', label: 'City' },
91+
{ name: 'notes', label: 'Notes' },
92+
],
93+
} as DetailViewSection;
94+
const data = { title: 'Hello', city: 'Paris', notes: '' };
95+
96+
it('draws one focus-revealed copy button per copyable field, none on an empty one', () => {
97+
const { container } = render(
98+
<DetailSection section={section} data={data} objectSchema={objectSchema} />,
99+
);
100+
const buttons = copyButtons(container);
101+
expect(buttons).toHaveLength(2); // title + city; the empty `notes` offers no copy
102+
buttons.forEach(expectFocusRevealContract);
103+
});
104+
105+
it('focus on the field row sits inside the same group as its copy button', () => {
106+
const { container } = render(
107+
<DetailSection section={section} data={data} objectSchema={objectSchema} />,
108+
);
109+
const [titleCopy] = copyButtons(container);
110+
const group = titleCopy.closest('.group') as HTMLElement;
111+
// The click-to-copy row is itself a Tab stop; focusing it is what
112+
// `group-focus-within` turns into the button's reveal.
113+
const row = group.querySelector('[role="button"]') as HTMLElement;
114+
expect(row).not.toBeNull();
115+
expect(row.getAttribute('tabindex')).toBe('0');
116+
row.focus();
117+
expect(row).toHaveFocus();
118+
expect(group.contains(document.activeElement)).toBe(true);
119+
});
120+
121+
it('activating the focused copy button copies its own field', async () => {
122+
const writeText = vi.fn().mockResolvedValue(undefined);
123+
Object.defineProperty(navigator, 'clipboard', {
124+
configurable: true,
125+
value: { writeText },
126+
});
127+
const { container } = render(
128+
<DetailSection section={section} data={data} objectSchema={objectSchema} />,
129+
);
130+
const [, cityCopy] = copyButtons(container);
131+
cityCopy.focus();
132+
// Enter / Space on a native <button> dispatch `click`.
133+
fireEvent.click(cityCopy);
134+
expect(writeText).toHaveBeenCalledTimes(1);
135+
expect(writeText).toHaveBeenCalledWith('Paris');
136+
});
137+
138+
it('draws no copy button on an editable field (the pencil takes its place)', () => {
139+
const { container } = render(
140+
<DetailSection
141+
section={section}
142+
data={data}
143+
objectSchema={objectSchema}
144+
onEnterInlineEdit={vi.fn()}
145+
/>,
146+
);
147+
expect(copyButtons(container)).toHaveLength(0);
148+
});
149+
});

0 commit comments

Comments
 (0)