Skip to content

Commit 7dd93c0

Browse files
yinlianghuiclaude
andauthored
fix(plugin-detail): quick_actions 读 aria 契约拼写,并撤下不存在的 actionNames 回退承诺 (#4663) (#5133)
同一组件的两处生产侧缺陷,objectstack#8744 测量该 renderer 读点时发现。 一、aria 读点双向落空。工具条只读 `schema.aria?.label` —— 恰是 `@objectstack/spec` 的 `AriaPropsSchema` 唯一拒绝的拼写:在那个封闭形上 `label` 是 alias 条目(指向 `ariaLabel` 的改名指示),存在意义是给出更好的 拒绝消息,从不被接受(实测:`safeParse({ label })` 返回 unrecognized_keys 并点名 `label`,`safeParse({ ariaLabel })` 通过)。于是 spec-valid 的 `aria: { ariaLabel: … }` 到达 renderer 后无人读取,内建 "Quick actions" 每次都赢;而 renderer 认的拼写,作者一写文档就被契约拒收。 `SchemaRenderer` 的通用 ARIA 通道也不是出口:它读扁平 `schema.ariaLabel` 并以组件 prop 注入 `aria-label`,而本 renderer 的 `splitDesigner` 连同其他 非 designer prop 一并丢弃。 读点改为 `(aria.ariaLabel ?? aria.label) || 'Quick actions'`:legacy 一腿 仅为契约封闭前的存量文档保留,两者同现时 canonical 赢。两半都照本仓既有的 同键惯例 —— `normalizeListViewSchema` 的 aria fold 仅在 canonical 为 `undefined` 时才搬运 legacy 键(故 `ariaLabel: ''` 遮蔽陈旧的 `label`), 而 `ListView` 自身读点把空串视为「没有可访问名」;因 `role="toolbar"` 必须 有名字,此处「没有名字」落到内建兜底,而非 ListView 的省略属性。 二、`actionNames` 注册描述承诺了不存在的回退(「else every action declared for the object at this location」)。实测:无 `actionNames` 且宿主不传 `actions` 时 `namesToResolve` 为空、`needsLookup` 为 false、对象元数据从不 被查询、渲染虚线占位符。注册 `inputs` 是发布面(序列化进 `sdui.manifest.json` 与 JSX 授权类型,Studio 据此教作者),该承诺已流入工具 链 —— objectstack#8744 的派发提示逐字引用了它。按分诊口径改句子:实现该回退 属能力扩张,另立卡。回归测试驱动一个「有货」的元数据 provider,证明对象上已 声明的 action 确实不会被拉进来(并附控制用例证明同一套接线在有名字时照常 交付)。 Fixes #4663 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 958d757 commit 7dd93c0

5 files changed

Lines changed: 349 additions & 3 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
'@object-ui/plugin-detail': patch
3+
---
4+
5+
`record:quick_actions` reads the toolbar's accessible name under the spelling the ARIA contract accepts, and stops advertising an action fallback it never had.
6+
7+
Two producer-side defects in the same component (objectui#4663), found while
8+
objectstack#8744 measured this renderer's read points.
9+
10+
**The `aria` read point was dead in both directions.** The toolbar read
11+
`schema.aria?.label` and nothing else — the ONE spelling `@objectstack/spec`'s
12+
`AriaPropsSchema` refuses. On that closed shape `label` is an ALIAS ENTRY: a
13+
rename prescription pointing at `ariaLabel`, there to produce a better rejection
14+
message, never accepted (measured: `safeParse({ label })` returns
15+
`unrecognized_keys` naming `label`, while `safeParse({ ariaLabel })` passes). So
16+
a spec-valid `aria: { ariaLabel: 'Account actions' }` reached the renderer and
17+
was read by nothing — the built-in "Quick actions" default won every time — and
18+
the spelling that did work was one no author can write without the contract
19+
rejecting the document. `SchemaRenderer`'s generic ARIA channel was no escape
20+
hatch either: it reads the FLAT `schema.ariaLabel` and injects `aria-label` as a
21+
component prop, which this renderer drops along with every other non-designer
22+
prop.
23+
24+
The read is now `(aria.ariaLabel ?? aria.label) || 'Quick actions'`. The legacy
25+
leg is back-compat only, for documents stored before the contract closed;
26+
canonical wins when both are present. Both halves follow how the repo already
27+
handles this key: `normalizeListViewSchema`'s aria fold copies the legacy key
28+
across only when the canonical one is `undefined` (so a declared `ariaLabel: ''`
29+
shadows a stale `label`), and `ListView`'s own read point treats an empty string
30+
as no accessible name at all — which here resolves to the built-in default,
31+
since `role="toolbar"` needs a name.
32+
33+
**The `actionNames` description promised a fallback that exists on no path.** It
34+
read "(else every action declared for the object at this location)". Measured:
35+
with no `actionNames` and no host-supplied `actions`, `namesToResolve` is empty,
36+
`needsLookup` is false, the object metadata is never queried, and the bar renders
37+
its dashed placeholder. The registry `inputs` are published — they are serialized
38+
into `sdui.manifest.json` and the JSX authoring types, and Studio teaches authors
39+
from them — so the promise reached tooling; objectstack#8744's dispatch prompt
40+
quoted it verbatim as a declared input. Per the triage ruling the sentence is
41+
what changes: implementing the fallback would be a behaviour expansion and needs
42+
its own card. No runtime behaviour changes with it, and a regression test now
43+
drives a LOADED metadata provider to prove the object's declared actions really
44+
are not pulled in (with a control proving the same wiring delivers them the
45+
moment a name asks).
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
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+
* `record:quick_actions` — the registered `actionNames` description describes
9+
* what the renderer does (objectui#4663).
10+
*
11+
* The registry `inputs` ARE the published authoring surface: `gen-manifest.ts`
12+
* serializes them into `sdui.manifest.json` and the JSX authoring types, and
13+
* Studio tooling teaches authors from these sentences. The `actionNames`
14+
* description promised a fallback that exists on no path —
15+
*
16+
* > 'Action names to expose, in order (else every action declared for the
17+
* > object at this location)'
18+
*
19+
* — and it was believed: objectstack#8744's dispatch prompt quoted it verbatim
20+
* as a declared input, and this is the clause that failed verification there.
21+
*
22+
* The measured chain, which the first two cases drive for real rather than
23+
* restate: no `actionNames` and no host `actions` → `namesToResolve` is empty →
24+
* `needsLookup` is false → the object metadata is NEVER queried → `actions` is
25+
* `[]` → the dashed placeholder renders. Per the triage ruling on #4663 the
26+
* fallback is a behaviour EXPANSION and is not being implemented here, so the
27+
* sentence is what changes; these cases are what stops it drifting back into a
28+
* promise.
29+
*
30+
* The metadata provider below is deliberately LOADED — it declares an action at
31+
* this very location — so "the bar stayed empty" cannot be an artefact of a
32+
* provider that had nothing to give. The control case proves the same wiring
33+
* delivers that action the moment a name asks for it.
34+
*/
35+
36+
import * as React from 'react';
37+
import { describe, it, expect, beforeEach, vi } from 'vitest';
38+
import { render, screen } from '@testing-library/react';
39+
import '@testing-library/jest-dom';
40+
import { ComponentRegistry } from '@object-ui/core';
41+
import { MetadataCtx, RecordContextProvider } from '@object-ui/react';
42+
import type { MetadataContextValue } from '@object-ui/react';
43+
import { RecordQuickActionsRenderer } from '../renderers/record-quick-actions';
44+
import '../index';
45+
46+
/** Declared on the object, at the location this bar renders by default. */
47+
const APPROVE = {
48+
name: 'approve',
49+
label: 'Approve',
50+
type: 'script',
51+
locations: ['record_header'],
52+
};
53+
54+
const OBJECT_META = { name: 'crm_account', actions: [APPROVE] };
55+
56+
const getItem = vi.fn(async (type: string, name: string) =>
57+
type === 'object' && name === 'crm_account' ? OBJECT_META : null,
58+
);
59+
60+
/**
61+
* A hand-rolled context value, held at MODULE level on purpose: `getItem` is an
62+
* effect dependency of `useMetadataItem`, so a value rebuilt per render spins
63+
* that hook forever (the loop `NO_METADATA_PROVIDER` was frozen to fix).
64+
*/
65+
const METADATA: MetadataContextValue = {
66+
apps: [],
67+
objects: [OBJECT_META] as any,
68+
dashboards: [],
69+
reports: [],
70+
pages: [],
71+
loading: false,
72+
error: null,
73+
refresh: async () => {},
74+
invalidate: () => {},
75+
ensureType: async () => [],
76+
getItem: getItem as unknown as MetadataContextValue['getItem'],
77+
getItemsByType: () => [],
78+
getTypeStatus: () => 'ready' as const,
79+
};
80+
81+
function mount(schema: Record<string, unknown>) {
82+
return render(
83+
<MetadataCtx.Provider value={METADATA}>
84+
<RecordContextProvider objectName="crm_account" recordId="rec-1" data={{ id: 'rec-1' }}>
85+
<RecordQuickActionsRenderer schema={schema as any} />
86+
</RecordContextProvider>
87+
</MetadataCtx.Provider>,
88+
);
89+
}
90+
91+
const actionNamesInput = () =>
92+
(ComponentRegistry.getConfig('record:quick_actions')?.inputs ?? []).find(
93+
(i) => i.name === 'actionNames',
94+
);
95+
96+
beforeEach(() => getItem.mockClear());
97+
98+
describe('record:quick_actions — `actionNames` description vs measured behaviour (objectui#4663)', () => {
99+
it('with no `actionNames` and no host `actions`, the object\'s declared actions are NOT pulled in', async () => {
100+
mount({});
101+
102+
// The bar's empty state, not the object's action.
103+
expect(await screen.findByText(/no actions configured/i)).toBeInTheDocument();
104+
expect(screen.queryByRole('button', { name: 'Approve' })).not.toBeInTheDocument();
105+
// …and the reason: nothing ever asked the metadata layer. This is the exact
106+
// step the promised "else every action declared for the object" fallback
107+
// would have had to take.
108+
expect(getItem).not.toHaveBeenCalled();
109+
});
110+
111+
it('names the actions and the same wiring resolves them (control)', async () => {
112+
mount({ actionNames: ['approve'] });
113+
114+
expect(await screen.findByRole('button', { name: 'Approve' })).toBeInTheDocument();
115+
expect(getItem).toHaveBeenCalledWith('object', 'crm_account');
116+
});
117+
118+
it('the published description does not promise the fallback the renderer lacks', () => {
119+
const description = actionNamesInput()?.description ?? '';
120+
expect(description).not.toBe('');
121+
// The removed claim, named exactly as it was published.
122+
expect(description).not.toMatch(/every action declared/i);
123+
// …replaced by the measured outcome of case 1, so an author reading the
124+
// manifest learns what actually happens when the input is left off.
125+
expect(description).toMatch(/empty placeholder/i);
126+
});
127+
});

packages/plugin-detail/src/index.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,17 @@ ComponentRegistry.register('quick_actions', RecordQuickActionsRenderer, {
537537
label: 'Quick Actions',
538538
icon: 'Zap',
539539
inputs: [
540-
{ name: 'actionNames', type: 'array', label: 'Actions', description: 'Action names to expose, in order (else every action declared for the object at this location)' },
540+
// Describes what the renderer does, not what it might do (objectui#4663).
541+
// This sentence used to end "(else every action declared for the object at
542+
// this location)" — a fallback that exists on no path: with no names and no
543+
// host-supplied `actions`, `needsLookup` is false, the object metadata is
544+
// never queried, and the bar renders its placeholder. The registry `inputs`
545+
// are the published surface (`gen-manifest.ts` serializes them into
546+
// `sdui.manifest.json` and the JSX authoring types), so the promise was
547+
// taught to authors and to tooling — objectstack#8744 quoted it verbatim.
548+
// Implementing the fallback would be a behaviour expansion and needs its own
549+
// card; pinned by `recordQuickActionsInputs.actionNamesFallback.test.tsx`.
550+
{ name: 'actionNames', type: 'array', label: 'Actions', description: 'Action names to expose, in order — resolved from the actions declared on the object. With no names (and no host-supplied actions) nothing is looked up and the bar renders its empty placeholder' },
541551
{ name: 'requiredPermissions', type: 'array', label: 'Required Permissions', description: 'Hide the whole bar unless the user holds these permissions' },
542552
// Derived from the spec's own vocabulary rather than restated — #3019.
543553
{ name: 'location', type: 'enum', label: 'Location', enum: [...ACTION_LOCATIONS], defaultValue: 'record_header', description: 'Which declared action location this bar renders' },
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
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+
* `record:quick_actions` — the toolbar's accessible name is read under the
9+
* spelling the platform ARIA contract actually accepts (objectui#4663).
10+
*
11+
* The bar used to read `schema.aria?.label` and nothing else. That is the ONE
12+
* spelling `@objectstack/spec`'s `AriaPropsSchema` refuses: `label` is an ALIAS
13+
* ENTRY on that closed shape, i.e. a rename prescription pointing at
14+
* `ariaLabel`, so it exists to produce a better rejection message — never to be
15+
* accepted. The two halves compounded into a dead read point:
16+
*
17+
* - the spec-valid `aria: { ariaLabel: '…' }` reached the renderer and was
18+
* read by nothing (the built-in "Quick actions" default won every time);
19+
* - the spelling the renderer honoured is the one an author cannot write
20+
* without the contract rejecting the document.
21+
*
22+
* `SchemaRenderer`'s generic ARIA channel is no escape hatch here: it reads the
23+
* FLAT `schema.ariaLabel` off the hoisted node and injects `aria-label` as a
24+
* component PROP, and this renderer drops every prop that is not a designer key
25+
* (`splitDesigner` keeps `data-obj-id` / `data-obj-type` / `style` and discards
26+
* the rest). The nested bag read below is the only live path for an authored
27+
* name on this surface.
28+
*
29+
* The spec half is measured, not asserted from memory — the last case parses
30+
* both spellings through the installed `AriaPropsSchema` so "the contract
31+
* refuses `label`" stays a fact this file checks rather than a claim it repeats.
32+
*/
33+
34+
import * as React from 'react';
35+
import { describe, it, expect } from 'vitest';
36+
import { render, screen, cleanup } from '@testing-library/react';
37+
import '@testing-library/jest-dom';
38+
import { RecordContextProvider } from '@object-ui/react';
39+
import { AriaPropsSchema } from '@objectstack/spec/ui';
40+
import { RecordQuickActionsRenderer } from '../record-quick-actions';
41+
42+
/**
43+
* One visible action, so the toolbar element (and its `aria-label`) renders at
44+
* all — with no visible action the bar short-circuits to its dashed placeholder,
45+
* which carries no toolbar role.
46+
*/
47+
const ACT = { name: 'act', label: 'Act', type: 'script', locations: ['record_header'] };
48+
49+
function mount(aria?: Record<string, unknown>) {
50+
return render(
51+
<RecordContextProvider objectName="crm_account" recordId="rec-1" data={{ id: 'rec-1' }}>
52+
<RecordQuickActionsRenderer schema={{ actions: [ACT], ...(aria ? { aria } : {}) } as any} />
53+
</RecordContextProvider>,
54+
);
55+
}
56+
57+
const toolbarName = () => screen.getByRole('toolbar').getAttribute('aria-label');
58+
59+
describe('record:quick_actions — toolbar accessible name (objectui#4663)', () => {
60+
it('reads the contract spelling `aria.ariaLabel`', () => {
61+
mount({ ariaLabel: 'Account actions' });
62+
expect(toolbarName()).toBe('Account actions');
63+
});
64+
65+
it('still honours the legacy `aria.label` stored documents carry', () => {
66+
// Back-compat only. `AriaPropsSchema` refuses this spelling (last case), so
67+
// nothing newly authored can reach here — it exists for documents written
68+
// before the contract closed, mirroring the fold `normalizeListViewSchema`
69+
// applies at the ListView boundary (`ARIA_KEY_ALIASES`, objectui#2890).
70+
mount({ label: 'Legacy name' });
71+
expect(toolbarName()).toBe('Legacy name');
72+
});
73+
74+
it('prefers the canonical spelling when a document carries both', () => {
75+
mount({ ariaLabel: 'Canonical', label: 'Legacy' });
76+
expect(toolbarName()).toBe('Canonical');
77+
});
78+
79+
it('falls back to the built-in name when neither spelling is authored', () => {
80+
mount();
81+
expect(toolbarName()).toBe('Quick actions');
82+
});
83+
84+
/**
85+
* Empty-string semantics, pinned because this is exactly where `??` and `||`
86+
* part company and the choice is deliberate (objectui#4663).
87+
*
88+
* Both halves follow the repo's existing handling of THIS key:
89+
*
90+
* - canonical-vs-legacy uses `??`, matching `normalizeListViewSchema`'s aria
91+
* fold, which copies the legacy key across only when the canonical one is
92+
* `undefined` — a declared `ariaLabel: ''` shadows the legacy key there,
93+
* and does here;
94+
* - the built-in default is truthiness-gated, matching `ListView`'s own read
95+
* point (`schema.aria?.ariaLabel ? {'aria-label': …} : {}`), which treats
96+
* an empty string as no accessible name at all. `role="toolbar"` needs a
97+
* name, so the equivalent here is the built-in default rather than
98+
* ListView's "omit the attribute".
99+
*/
100+
it('treats an authored empty string as no name — the built-in default wins', () => {
101+
mount({ ariaLabel: '' });
102+
expect(toolbarName()).toBe('Quick actions');
103+
cleanup();
104+
105+
// …and the empty canonical value still shadows the legacy key, rather than
106+
// letting a stale legacy name resurface under a document that has been
107+
// migrated to the contract spelling.
108+
mount({ ariaLabel: '', label: 'Legacy' });
109+
expect(toolbarName()).toBe('Quick actions');
110+
});
111+
112+
it('the platform contract really accepts `ariaLabel` and refuses `label`', () => {
113+
// The premise the read order above rests on, measured against the installed
114+
// spec instead of restated. If a future spec ever accepts `label`, this
115+
// fails and the legacy leg's justification (back-compat only, never an
116+
// authoring surface) has to be revisited.
117+
expect(AriaPropsSchema.safeParse({ ariaLabel: 'Account actions' }).success).toBe(true);
118+
119+
const legacy = AriaPropsSchema.safeParse({ label: 'Account actions' });
120+
expect(legacy.success).toBe(false);
121+
// Asserted as an envelope — the code AND the key it names — so a rejection
122+
// of something else could not satisfy it.
123+
expect(legacy.error?.issues.map((i) => i.code)).toContain('unrecognized_keys');
124+
expect(
125+
legacy.error?.issues.flatMap((i) => (i as unknown as { keys?: string[] }).keys ?? []),
126+
).toContain('label');
127+
});
128+
});

packages/plugin-detail/src/renderers/record-quick-actions.tsx

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,13 @@ export interface RecordQuickActionsRendererProps {
3535
align?: 'start' | 'center' | 'end';
3636
size?: 'sm' | 'default' | 'lg';
3737
variant?: 'default' | 'secondary' | 'outline' | 'ghost' | 'destructive' | 'link';
38-
aria?: { label?: string };
38+
/**
39+
* Accessible name for the toolbar. `ariaLabel` is the spelling
40+
* `@objectstack/spec`'s `AriaPropsSchema` accepts; `label` is that shape's
41+
* alias entry — refused on parse — and is kept here as a back-compat read
42+
* for documents written before the contract closed (objectui#4663).
43+
*/
44+
aria?: { ariaLabel?: string; label?: string };
3945
properties?: Record<string, any>;
4046
[k: string]: any;
4147
};
@@ -164,6 +170,36 @@ export const RecordQuickActionsRenderer: React.FC<RecordQuickActionsRendererProp
164170
// (the `inline` flag is set by PageHeader's first-class `actions` prop).
165171
const inlineWithHeader = location === 'record_header' && !schema.inline;
166172

173+
/**
174+
* The toolbar's accessible name, read under the spelling the platform ARIA
175+
* contract actually accepts (objectui#4663).
176+
*
177+
* This line used to read `aria.label` and nothing else — the ONE spelling
178+
* `@objectstack/spec`'s `AriaPropsSchema` refuses. `label` is that closed
179+
* shape's ALIAS ENTRY, a rename prescription pointing at `ariaLabel`, so it
180+
* exists to produce a better rejection message and is never accepted. The
181+
* result was a dead read point in both directions: a spec-valid
182+
* `aria: { ariaLabel: … }` was discarded, and the spelling honoured was one no
183+
* author can write without the contract rejecting the document.
184+
*
185+
* `SchemaRenderer`'s generic ARIA channel does not cover this: it reads the
186+
* FLAT `schema.ariaLabel` and injects `aria-label` as a component PROP, which
187+
* `splitDesigner` above drops with every other non-designer prop. The nested
188+
* bag is the live path here.
189+
*
190+
* `??` between the two spellings, `||` for the built-in default — both halves
191+
* follow how this repo already handles this key:
192+
*
193+
* - `normalizeListViewSchema`'s aria fold (`ARIA_KEY_ALIASES`, objectui#2890)
194+
* copies the legacy key onto the canonical one only when the canonical is
195+
* `undefined`, so a declared `ariaLabel: ''` shadows a stale `label` there
196+
* — and does here;
197+
* - `ListView`'s own read point treats an empty string as no accessible name
198+
* at all. `role="toolbar"` needs a name, so "no name" resolves to the
199+
* built-in default here rather than to ListView's omitted attribute.
200+
*/
201+
const ariaLabel = (schema.aria?.ariaLabel ?? schema.aria?.label) || 'Quick actions';
202+
167203
return (
168204
<div
169205
className={cn(
@@ -173,7 +209,7 @@ export const RecordQuickActionsRenderer: React.FC<RecordQuickActionsRendererProp
173209
className,
174210
)}
175211
role="toolbar"
176-
aria-label={schema.aria?.label || 'Quick actions'}
212+
aria-label={ariaLabel}
177213
{...designer}
178214
>
179215
{visibleActions.map((action, idx) => (

0 commit comments

Comments
 (0)