Skip to content

Commit dda74d3

Browse files
fix(app-shell): disabled Studio Publish gives its nothing-pending reason on hover and keyboard focus (#31561)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9Ubb6bDc6Eia7WGdAfa8YTp42f)_ Fixes #31495 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:Studio 顶栏的「发布」按钮在没有待发布草稿时,不再只在禁用按钮上挂一个 `title`,而是和只读包状态一样,外面套一层可聚焦的包裹元素:鼠标悬停、键盘 Tab 聚焦都会弹出提示「没有待发布的草稿」,屏幕阅读器也能读到这句原因。 - **为什么改**:原生禁用的按钮收不到悬停和焦点事件,`title` 谁也看不到,Tab 还会直接跳过「发布」;作者不知道按钮为什么是灰的。QA 规则 `ux-conventions.disabled-control-reason` 要求每个禁用控件都能说明原因。 - **风险与回滚**:只改一个组件的包裹结构,不动文案、不加多语言 key、不碰服务端或 spec。只读包状态和有草稿时的可点击按钮保持原样。回滚即 revert 本 PR。 - **先看**:`ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx` 中 `publishDisabledReason` 与其下方的 Tooltip 包裹;其次是测试文件 `StudioDesignSurface.publishReadOnlyReason-12303.test.tsx` 末尾新增的 describe 块。 - **两分钟验证**:打开一个没有草稿的可写包的 Studio,在「变更」按钮上按一次 Tab,焦点应落在「发布」上并弹出「没有待发布的草稿」提示;鼠标悬停也应出现同一提示。 - **需要你做的**:无,席位落地。 ## What changes `StudioDesignSurface` computes one `publishDisabledReason`: the read-only reason (`engine.studio.pkg.readonlyHint`) on a read-only package with no pair, else the nothing-pending reason (`engine.studio.publishNoneTitle`) when no draft is pending and no publish is in flight, else none. Whenever there is a reason, Publish is drawn inside the one shared wrapper the read-only state already used: a `tabIndex=0` span marked `data-disabled-reason` that is the Radix tooltip trigger, with the span and the button both pointing `aria-describedby` at an `sr-only` copy of the reason. The button gets `disabled:pointer-events-none` so the wrapper takes the hover. The `title` the disabled button could never show is dropped; an enabled Publish keeps its `engine.studio.publishTitle` title. The nothing-pending `sr-only` reason moves inside the wrapper and keeps its `publish-none-reason` test id and its id. The read-only branch renders exactly as before (same wrapper, same text, same ids). A publish in flight gets no wrapper on purpose: the button shows a spinner for the moment the publish runs, which is the host's own transient state and not a condition the author has to act on, so there is no reason to give. No locale key is added (the existing `engine.studio.publishNoneTitle` is reused). No server or `packages/spec` change. ## Tests The new pins go into the existing shared harness `StudioDesignSurface.publishReadOnlyReason-12303.test.tsx` as a top-level describe, with a `drafts` ledger the fetch stub now answers: - no drafts on a writable package: the wrapper is a tab stop; the button carries no `title`; the wrapper and the button are both described by "No drafts pending publish", through the one `publish-none-reason` element; the button lets the hover through; - hovering the wrapper opens a tooltip with the reason; - Tab from "Changes" lands on Publish's wrapper and opens the tooltip; - CONTROL, a draft pending: a plain enabled Publish with no wrapper, no description and no reason text. Two edits to the file's existing read-only block, both forced by the new behaviour: - the wait helper now waits for the wrapper to be described by the reason it expects, not for any wrapper, because an empty ledger now wraps Publish in the nothing-pending reason before the package list answers `writable: false`; - the writable CONTROL leg asserted "no wrapper" on a writable package with an empty ledger, which is exactly the state this card changes. It now asserts that leg gives no read-only reason and that its wrapper carries the nothing-pending reason instead. Unchanged and green: the objectui#8219 no-draft and pending pins (`StudioDesignSurface.interfacePolish.test.tsx`), the objectui#12517 posture pins (`StudioDesignSurface.publishPosture-12517.test.tsx`), the pending-count pins (`StudioDesignSurface.changesCount-11787.test.tsx`, `StudioDesignSurface.publishCount-11807.test.tsx`). ## Acceptance notes - While the draft ledger's first read is still on the wire, the surface already treats the package as having nothing pending, so the wrapper and the nothing-pending reason show from first paint until the read answers. This is unchanged from main, where the `sr-only` sentence and the `title` showed in the same window; this PR only gives that state the wrapper. - When a draft lands while the wrapper has focus, the wrapper is replaced by the plain button, so focus returns to the document body. The read-only wrapper on main behaves the same way when a package turns writable. ## Verification Head `41c616fc` (fix `0c4adb53`, harness import `d612ad6e`, merge of `origin/main` `41c616fc`, which touched no file under `ui/packages/app-shell`). Branch base `dabef0f66`. <details><summary>Tests (app-shell, vitest from ui/, under the verify lock)</summary> - The five Publish files (`StudioDesignSurface.publishReadOnlyReason-12303`, `interfacePolish`, `publishPosture-12517`, `changesCount-11787`, `publishCount-11807`) at `d612ad6e`: Test Files 5 passed (5), Tests 27 passed (27). - 32 neighbour files that import `StudioDesignSurface` and query Publish, a tooltip, Tab order or a `data-disabled-reason` wrapper, at `0c4adb53`: Test Files 32 passed (32), Tests 289 passed (289). The other app-shell suites are CI's Test UI. - `pnpm exec turbo run type-check --filter=@object-ui/app-shell`: 67 tasks, exit 0 (both `tsc --noEmit` and `tsc -p tsconfig.test.json`). </details> <details><summary>Ablation (one leg, committed first, through scripts/ablation-replace.mjs)</summary> The nothing-pending arm of `publishDisabledReason` was replaced by `false` (anchor 1 to 0, blob `5abb9ba96d85` to `f6ae84fc1790`), so a writable package with no drafts gets neither the wrapper nor the reason. Six cases go red: the three new nothing-pending cases, the writable CONTROL leg of the read-only block, the objectui#8219 no-draft pin and the objectui#12517 nothing-pending pin. The three read-only cases and the new pending CONTROL stay green. Restore: blob back to `5abb9ba96d85`, equal to HEAD, `git diff HEAD` empty. The tests import the surface by relative path, so the mutation is read from `src/` with no build in between. </details> <details><summary>Gates</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derives 36 families; all 36 ran with exit 0 and `--ran` reconciles 36 run, 0 NOT-MEASURED. `check:test-source-alias` first asked for a module-top `@objectstack/lint` import in the edited harness (added in `d612ad6e`, then green). `check:query-options-erasure` and `check:type-check-debt` ran without a timeout, the latter under the lock. After the merge of main the derived list is unchanged, and the ratchet families (`check:test-source-alias`, `check:type-check-coverage`, `check:query-options-erasure`, `check:cross-package-test-inputs`, `check-sdui-manifest`, `check-changeset-no-major`, `check-empty-changeset`) re-ran green on `41c616fc`. `pnpm --dir ui run check:new-line-citations`: 0 new citations. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jRPafwMLdY61r8x4Jn)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 983abd9 commit dda74d3

3 files changed

Lines changed: 133 additions & 50 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/app-shell': patch
3+
---
4+
5+
fix(app-shell): Studio's disabled Publish says why there is nothing to publish, on hover and on keyboard focus, the way it already does on a read-only package
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
With no drafts pending, the Studio header's Publish button was natively disabled and carried its reason ("No drafts pending publish") only as a `title` on the button itself. A disabled button fires no hover or focus events, so no tooltip opened and Tab skipped Publish entirely. It now takes the read-only state's treatment: a focusable wrapper that is the tooltip trigger and a tab stop, with the wrapper and the button both described by the existing `sr-only` reason. The text is the existing `engine.studio.publishNoneTitle` sentence; no locale key is added.
11+
12+
The read-only state is unchanged, a pending draft still shows the plain enabled Publish button, and a publish in flight keeps its spinner without a wrapper.

‎ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.publishReadOnlyReason-12303.test.tsx‎

Lines changed: 87 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@
1616
* other half of the row, is pinned in
1717
* `StudioDesignSurface.automationsReadOnly-11124.test.tsx`.
1818
*
19+
* objectstack#31495 — the other disabled state, a writable package with
20+
* nothing pending ("No drafts pending publish"), kept the `title` on the
21+
* disabled button: no wrapper, no tooltip on hover, and Tab skipped Publish.
22+
* It now takes the same wrapper, described by the same `sr-only` reason
23+
* (`publish-none-reason`); with a draft pending Publish is a plain enabled
24+
* button again. Pinned in the last block below.
25+
*
1926
* The REAL `StudioDesignSurface` over a fetch stub (the harness of
2027
* `StudioDesignSurface.interfacePolish.test.tsx`); only the package list's
2128
* `writable` differs between the case and its control.
@@ -28,10 +35,15 @@ import { act, cleanup, render, screen, waitFor } from '@testing-library/react';
2835
import userEvent from '@testing-library/user-event';
2936
import { MemoryRouter, Route, Routes } from 'react-router-dom';
3037
import { I18nProvider } from '@object-ui/i18n';
38+
// The surface loads the linter lazily; pay that load at collection, not inside
39+
// a case's clocked window.
40+
import '@objectstack/lint';
3141

3242
const PACKAGE_ID = 'com.acme.app';
3343

3444
const pkg = vi.hoisted(() => ({ writable: false }));
45+
/** The draft ledger `/meta/_drafts` answers. */
46+
let drafts: Array<{ type: string; name: string; packageId: string }> = [];
3547

3648
vi.mock('./packages-io', async (importOriginal) => {
3749
const actual = await importOriginal<Record<string, unknown>>();
@@ -73,18 +85,20 @@ window.matchMedia = ((query: string) => ({
7385
};
7486

7587
const REASON = t('engine.studio.pkg.readonlyHint', 'en-US');
88+
const NONE_PENDING = t('engine.studio.publishNoneTitle', 'en-US');
7689

7790
function json(body: unknown, status = 200) {
7891
return new Response(JSON.stringify(body), { status, headers: { 'content-type': 'application/json' } });
7992
}
8093

8194
beforeEach(() => {
8295
pkg.writable = false;
96+
drafts = [];
8397
vi.stubGlobal(
8498
'fetch',
8599
vi.fn(async (input: unknown) => {
86100
const path = new URL(String(input), 'http://localhost').pathname;
87-
if (path === '/api/v1/meta/_drafts') return json([]);
101+
if (path === '/api/v1/meta/_drafts') return json(drafts);
88102
if (path === '/api/v1/meta/app') return json([{ name: 'acme_app', label: 'Acme' }]);
89103
if (path === '/api/v1/meta/app/acme_app/layers') {
90104
return json({ code: null, overlay: null, overlayScope: null, effective: { name: 'acme_app', label: 'Acme', navigation: [] } });
@@ -120,19 +134,27 @@ const describedBy = (el: HTMLElement) => {
120134
return id ? document.getElementById(id) : null;
121135
};
122136

123-
/** Publish once the package list has said the package is read-only. */
124-
async function readOnlyPublish(): Promise<HTMLElement> {
137+
/**
138+
* Publish once its wrapper gives `reason`. The wrapper alone is not enough to
139+
* wait on: before the package list answers, an empty ledger already wraps
140+
* Publish in the "nothing pending" reason (objectstack#31495).
141+
*/
142+
async function wrappedPublish(reason: string): Promise<HTMLElement> {
125143
renderWorkbench();
126144
return waitFor(
127145
() => {
128146
const btn = publish();
129147
expect(btn.parentElement).toHaveAttribute('data-disabled-reason');
148+
expect(describedBy(btn.parentElement as HTMLElement)).toHaveTextContent(reason);
130149
return btn;
131150
},
132151
{ timeout: 8000 },
133152
);
134153
}
135154

155+
/** Publish once the package list has said the package is read-only. */
156+
const readOnlyPublish = () => wrappedPublish(REASON);
157+
136158
describe('Studio header Publish on a read-only package — the reason reaches the author (objectui#12303)', () => {
137159
it('Publish is described by the reason, carries no `title`, and its wrapper is described by it too', async () => {
138160
const btn = await readOnlyPublish();
@@ -160,7 +182,7 @@ describe('Studio header Publish on a read-only package — the reason reaches th
160182
expect(await screen.findByRole('tooltip')).toHaveTextContent(REASON);
161183
});
162184

163-
it('CONTROL — a writable package: Publish has no read-only reason and no wrapper', async () => {
185+
it('CONTROL — a writable package: Publish gives no read-only reason', async () => {
164186
pkg.writable = true;
165187
renderWorkbench();
166188
// Read after the package list has answered `writable: true`.
@@ -170,8 +192,68 @@ describe('Studio header Publish on a read-only package — the reason reaches th
170192
});
171193
const btn = publish();
172194
expect(btn).not.toHaveAttribute('title', REASON);
173-
expect(btn.parentElement).not.toHaveAttribute('data-disabled-reason');
195+
// An empty ledger still wraps it, in its own reason (objectstack#31495).
196+
expect(describedBy(btn.parentElement as HTMLElement)).toHaveTextContent(NONE_PENDING);
174197
expect(describedBy(btn)).not.toHaveTextContent(REASON);
175198
expect(screen.queryByText(REASON)).toBeNull();
176199
});
177200
});
201+
202+
describe('Studio header Publish with nothing pending — the reason reaches the author (objectstack#31495)', () => {
203+
beforeEach(() => {
204+
pkg.writable = true;
205+
});
206+
207+
/** Publish on a writable package once the empty ledger has wrapped it. */
208+
const nonePendingPublish = () => wrappedPublish(NONE_PENDING);
209+
210+
it('the wrapper is a tab stop, and it and the button are described by "No drafts pending publish"', async () => {
211+
const btn = await nonePendingPublish();
212+
expect(btn).toBeDisabled();
213+
// The disabled button cannot show a `title`; the reason is the wrapper's.
214+
expect(btn).not.toHaveAttribute('title');
215+
expect(btn).toHaveAccessibleDescription(NONE_PENDING);
216+
const wrapper = btn.parentElement as HTMLElement;
217+
expect(wrapper).toHaveAttribute('tabindex', '0');
218+
expect(wrapper).toHaveAccessibleDescription(NONE_PENDING);
219+
// One reason element, the `sr-only` one both point at.
220+
expect(describedBy(wrapper)).toBe(screen.getByTestId('publish-none-reason'));
221+
expect(describedBy(btn)).toBe(describedBy(wrapper));
222+
// The wrapper takes the hover: the disabled button lets it through.
223+
expect(btn.className).toMatch(/(^|\s)disabled:pointer-events-none(\s|$)/);
224+
});
225+
226+
it('hovering Publish opens a tooltip with the reason', async () => {
227+
const user = userEvent.setup();
228+
const btn = await nonePendingPublish();
229+
await user.hover(btn.parentElement as HTMLElement);
230+
expect(await screen.findByRole('tooltip')).toHaveTextContent(NONE_PENDING);
231+
});
232+
233+
it('a keyboard user reaches the reason: Tab from "Changes" lands on Publish’s wrapper, which opens the tooltip', async () => {
234+
const user = userEvent.setup();
235+
const btn = await nonePendingPublish();
236+
screen.getByRole('button', { name: /^Changes/ }).focus();
237+
await user.tab();
238+
expect(btn.parentElement).toHaveFocus();
239+
expect(await screen.findByRole('tooltip')).toHaveTextContent(NONE_PENDING);
240+
});
241+
242+
it('CONTROL — a draft pending: a plain enabled Publish, no wrapper and no reason', async () => {
243+
drafts = [{ type: 'dashboard', name: 'acme_dashboard', packageId: PACKAGE_ID }];
244+
renderWorkbench();
245+
const btn = await waitFor(
246+
() => {
247+
const b = publish();
248+
expect(b).toBeEnabled();
249+
return b;
250+
},
251+
{ timeout: 8000 },
252+
);
253+
expect(btn.parentElement).not.toHaveAttribute('data-disabled-reason');
254+
expect(btn).not.toHaveAttribute('aria-describedby');
255+
expect(btn.className).not.toMatch(/(^|\s)disabled:pointer-events-none(\s|$)/);
256+
expect(screen.queryByTestId('publish-none-reason')).toBeNull();
257+
expect(screen.queryByText(NONE_PENDING)).toBeNull();
258+
});
259+
});

‎ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx‎

Lines changed: 34 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1214,6 +1214,15 @@ function StudioDesignSurfaceBody({ aiSlot, area }: StudioDesignSurfaceProps): Re
12141214
);
12151215
const publishNoneReasonId = React.useId();
12161216
const publishReadOnlyReasonId = React.useId();
1217+
// The reason a disabled Publish gives, or `null` when it gives none: the
1218+
// button is enabled, or a publish is in flight (a spinner, for a moment, not
1219+
// a state the author has to get out of). A read-only package outranks an
1220+
// empty ledger; with neither, the button is the primary action.
1221+
const publishDisabledReason: { id: string; text: string; testId?: string } | null = publishReadOnly
1222+
? { id: publishReadOnlyReasonId, text: t('engine.studio.pkg.readonlyHint', locale) }
1223+
: !hasPending && !publishing
1224+
? { id: publishNoneReasonId, text: t('engine.studio.publishNoneTitle', locale), testId: 'publish-none-reason' }
1225+
: null;
12171226

12181227
// Builder → running-app bridge (Airtable's Launch): the builder edits the
12191228
// package (the design surface), the app is its published front-end. If this
@@ -1512,22 +1521,10 @@ function StudioDesignSurfaceBody({ aiSlot, area }: StudioDesignSurfaceProps): Re
15121521
setChangesOpen(true);
15131522
}}
15141523
disabled={publishing || !hasPending || publishReadOnly}
1515-
aria-describedby={
1516-
publishReadOnly
1517-
? publishReadOnlyReasonId
1518-
: !hasPending && !publishing
1519-
? publishNoneReasonId
1520-
: undefined
1521-
}
1522-
// A read-only package's reason is the wrapper's (objectui#12303), not a
1523-
// `title` the disabled button can never show.
1524-
title={
1525-
publishReadOnly
1526-
? undefined
1527-
: hasPending
1528-
? t('engine.studio.publishTitle', locale)
1529-
: t('engine.studio.publishNoneTitle', locale)
1530-
}
1524+
aria-describedby={publishDisabledReason?.id}
1525+
// A disabled Publish's reason is its wrapper's (objectui#12303,
1526+
// objectstack#31495), never a `title` the disabled button cannot show.
1527+
title={hasPending && !publishReadOnly ? t('engine.studio.publishTitle', locale) : undefined}
15311528
// objectui#8219 — the primary style is for a publish the author
15321529
// CAN do. A disabled one (no draft, or a read-only package)
15331530
// drops to an outline, instead of staying the loudest element
@@ -1539,7 +1536,7 @@ function StudioDesignSurfaceBody({ aiSlot, area }: StudioDesignSurfaceProps): Re
15391536
? 'border-transparent bg-primary text-primary-foreground disabled:opacity-50'
15401537
: 'text-muted-foreground',
15411538
// The wrapper takes the hover: the disabled button lets it through.
1542-
publishReadOnly && 'disabled:pointer-events-none',
1539+
publishDisabledReason && 'disabled:pointer-events-none',
15431540
)}
15441541
>
15451542
{publishing ? <Loader2 className="h-3.5 w-3.5 animate-spin" /> : <Rocket className="h-3.5 w-3.5" />}
@@ -1677,46 +1674,38 @@ function StudioDesignSurfaceBody({ aiSlot, area }: StudioDesignSurfaceProps): Re
16771674
</span>
16781675
)}
16791676
</button>
1680-
{/* objectui#8219 — with nothing to publish, say so on the page;
1681-
objectui#12459 moves the sentence off the strip into the
1682-
Publish button's description, where readers and the tooltip
1683-
still find it. */}
1684-
{!hasPending && !publishReadOnly && !publishing && (
1685-
<span
1686-
id={publishNoneReasonId}
1687-
className="sr-only"
1688-
data-testid="publish-none-reason"
1689-
>
1690-
{t('engine.studio.publishNoneTitle', locale)}
1691-
</span>
1692-
)}
1693-
{/* objectui#12303 — on a read-only package the reason rides the
1694-
record page's Archive pattern (`DeclaredActionsBar`,
1695-
objectui#11811) instead of a `title` on the disabled button:
1696-
a natively disabled button fires no hover or focus, so the
1697-
`title` reached no keyboard or assistive-tech user and Tab
1698-
skipped Publish entirely. The wrapping span is the tooltip
1699-
trigger and a tab stop, and the button and the span both
1700-
point `aria-describedby` at an `sr-only` copy of the reason.
1701-
objectstack#28562 — a read-only base with a customization
1702-
package publishes that package's drafts, so it is not this. */}
1703-
{publishReadOnly ? (
1677+
{/* A disabled Publish says why, to the mouse, the keyboard and
1678+
assistive tech alike — the record page's Archive pattern
1679+
(`DeclaredActionsBar`, objectui#11811), for both reasons it
1680+
can be off: a read-only package (objectui#12303) and nothing
1681+
pending (objectui#8219, objectstack#31495). A natively
1682+
disabled button fires no hover or focus, so the wrapping span
1683+
is the tooltip trigger and a tab stop, and the button and the
1684+
span both point `aria-describedby` at an `sr-only` copy of
1685+
the reason. objectstack#28562 — a read-only base with a
1686+
customization package publishes that package's drafts, so it
1687+
is not read-only here. */}
1688+
{publishDisabledReason ? (
17041689
<TooltipProvider delayDuration={200}>
17051690
<Tooltip>
17061691
<TooltipTrigger asChild>
17071692
<span
17081693
tabIndex={0}
1709-
aria-describedby={publishReadOnlyReasonId}
1694+
aria-describedby={publishDisabledReason.id}
17101695
data-disabled-reason=""
17111696
className="inline-flex rounded-md focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2"
17121697
>
17131698
{publishButton}
1714-
<span id={publishReadOnlyReasonId} className="sr-only">
1715-
{t('engine.studio.pkg.readonlyHint', locale)}
1699+
<span
1700+
id={publishDisabledReason.id}
1701+
className="sr-only"
1702+
data-testid={publishDisabledReason.testId}
1703+
>
1704+
{publishDisabledReason.text}
17161705
</span>
17171706
</span>
17181707
</TooltipTrigger>
1719-
<TooltipContent>{t('engine.studio.pkg.readonlyHint', locale)}</TooltipContent>
1708+
<TooltipContent>{publishDisabledReason.text}</TooltipContent>
17201709
</Tooltip>
17211710
</TooltipProvider>
17221711
) : (

0 commit comments

Comments
 (0)