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
18 changes: 18 additions & 0 deletions .changeset/form-fullscreen-textarea-field-label.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
'@object-ui/components': patch
---

表单内置 `textarea` 的全屏编辑对话框现在能拿到字段自己的 label:对话框标题显示字段名而不是恒定的通用词「编辑文本」,同一张表单上多个长文本字段的展开按钮也终于有了互不相同的无障碍名(objectui#3393)。

`renderFormField` 在解构字段配置时把 `label` 单独取走了(它要渲染 `<FormLabel>`),而下游 `renderFieldComponent` 唯一调用点重建 props 对象时显式补回了 `field` / `inputType` / `options` / `placeholder` / `emptyHint` / `dependsOnLabels` 等等,唯独漏了 `label`。于是内置 `textarea` 分支里的 `label` 恒为 `undefined`,`FullscreenTextarea` 中两条依赖它的分支从写下那天起就没走到过:

- 对话框标题 `label ?? t('form.fullscreen.title')` 永远落到通用词——一个叫「备注」的字段点开全屏编辑,标题不会说自己是「备注」;
- 展开按钮的无障碍名永远插值通用名词,一张有三个长文本字段的表单上三个按钮读屏完全一样。读屏用户无法判断自己要展开的是哪个字段,这是可达性缺陷而不是观感问题。

## 改了什么

- 调用点显式转发 `label`(与 `placeholder` / `emptyHint` 同法),这是唯一的行为改动。
- `label` 属于 renderer-only:`stripRendererOnlyProps` 与 `stripRegisteredFieldProps` 各加一条丢弃项,所以它既不会变成 DOM 上的 `label="备注"` 杂属性(每个内置分支都会把剩余 props 直接摊到 DOM 节点上),也不会成为注册型 widget 新收到的 prop——自 v17 起 `field` 是它们唯一的元数据载体(objectui#3233),label 一直在那里读。
- 内置 `textarea` 分支里那句 `const { label: _label, ...rest }` 随之删除。它本想拦住 label 落到 DOM,但既然从来没有 label 送进来,它拦的是不存在的东西(ESLint 一直报着 `'_label' is assigned a value but never used`),而且只护住了这一个分支。现在这件事由 strip 统一负责,所有分支同等受护。

十个语言包零改动:#3272 把 `form.fullscreen.toggle` 做成了带 `{{label}}` 插值的整句(zh 插在句尾、ja 插在句首),label 一通,十个语言的句子直接就对。字段没有 label 时仍回落到被翻译的通用词。
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@
* Every locale case asserts the English literal is GONE as well as that the
* translation is present: a re-inlined literal alongside a translated sibling
* would still satisfy a positive-only assertion.
*
* The copy fixtures are deliberately LABEL-LESS since objectui#3393. The
* subject of this suite is the two GENERIC strings — `form.fullscreen.title`
* ("Edit text") and `form.fullscreen.textFallback` ("text", interpolated into
* `form.fullscreen.toggle`) — which are reachable only when the field has no
* label. When these cases were written the field was labelled `Notes` and the
* generic copy still appeared, because the renderer never forwarded `label`;
* that was the bug #3393 fixed, and re-pointing the fixtures at the stack the
* fallback actually serves is what keeps these ten packs covered rather than
* re-asserting the fixed defect. The labelled stack is pinned next door, in
* `form-fullscreen-textarea-label.test.tsx`.
*/
import { describe, it, expect } from 'vitest';
import { render, screen, fireEvent } from '@testing-library/react';
Expand All @@ -31,11 +42,16 @@ import { I18nProvider } from '@object-ui/i18n';
// is billed to `hookTimeout` (objectui#3010/#3021).
import '../../../renderers';

const fields = [
/** No `label` — the one authoring stack the generic copy is written for. */
const unlabelledFields = [
{ name: 'notes', type: 'textarea', mobile_fullscreen: true },
];

const labelledFields = [
{ name: 'notes', label: 'Notes', type: 'textarea', mobile_fullscreen: true },
];

function renderFormIn(language: string) {
function renderFormIn(language: string, fields: any[] = unlabelledFields) {
const Form = ComponentRegistry.get('form')!;
return render(
<I18nProvider config={{ defaultLanguage: language, detectBrowserLanguage: false }}>
Expand Down Expand Up @@ -111,7 +127,9 @@ describe('form renderer — fullscreen textarea dialog is translated (objectui#3
// The `Done` button lost its literal child; this pins that the click
// handler still rides on the translated button rather than on some other
// node that happened to carry the old text.
renderFormIn('zh');
// Labelled here, so the committed value can be found through the field's
// own `<FormLabel>` association — this case asserts wiring, not copy.
renderFormIn('zh', labelledFields);
openFullscreen();

fireEvent.change(screen.getByTestId('form-textarea-fullscreen-input'), {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,230 @@
/**
* 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.
*/

/**
* The fullscreen long-text editor knows WHICH field it is editing — objectui#3393.
*
* `renderFormField` destructures `label` off the field config before the
* `...fieldProps` rest, and the single `renderFieldComponent` call site rebuilt
* its props object without putting it back (it explicitly re-adds `field`,
* `inputType`, `options`, `placeholder`, `emptyHint`, `dependsOnLabels` … but
* not `label`). So the built-in `textarea` branch's `label` was `undefined` on
* every render and both of `FullscreenTextarea`'s label-dependent expressions
* were dead:
*
* 1. the dialog title `label ?? t('form.fullscreen.title')` always resolved
* to the generic "Edit text" — a field named "Notes" opened a dialog that
* would not say so;
* 2. the expand button's accessible name always interpolated the generic
* noun, so a form with three long-text fields gave all three buttons the
* SAME name. A screen-reader user could not tell which field a button
* expanded — an accessibility defect, not a cosmetic one.
*
* What this suite pins, in the two directions the fix can regress:
*
* - the label REACHES the dialog (title + a distinct name per field), and
* - it reaches nothing else. `label` is renderer-only: `<FormLabel>` already
* renders it above the control, so every other built-in branch spreads its
* leftover props straight onto a DOM node where `label="Notes"` would be a
* stray attribute, and registered widgets read the label off `field`, their
* single metadata carrier since v17 (objectui#3233). Both strips therefore
* discard it. Dropping either strip entry turns the DOM/widget cases below
* red; dropping the call-site key turns the dialog cases red.
*
* The label-less fallback (generic copy, in ten languages) is the subject of
* `form-fullscreen-textarea-i18n.test.tsx`; one case is repeated here so the
* `??` fallback arm cannot be deleted while this file alone stays green.
*/

import { describe, it, expect, beforeAll, beforeEach, afterEach, vi } from 'vitest';
import { render, screen, fireEvent, cleanup } from '@testing-library/react';
import { ComponentRegistry } from '@object-ui/core';
import { I18nProvider } from '@object-ui/i18n';
// Module scope, not `beforeAll` — the cold transform must not be billed to
// `hookTimeout`. See object-ui/no-dynamic-import-in-test-hook (objectui#3010).
import '../../../renderers';

function renderForm(fields: any[], language = 'en') {
const Form = ComponentRegistry.get('form')!;
return render(
<I18nProvider config={{ defaultLanguage: language, detectBrowserLanguage: false }}>
<Form schema={{ type: 'form', showSubmit: false, showCancel: false, fields }} />
</I18nProvider>,
);
}

const longText = (name: string, label?: string) => ({
name,
...(label === undefined ? {} : { label }),
type: 'textarea',
mobile_fullscreen: true,
});

afterEach(() => {
cleanup();
vi.restoreAllMocks();
});

describe('form renderer — the fullscreen dialog names its field (objectui#3393)', () => {
it('titles the dialog with the field label instead of the generic word', () => {
renderForm([longText('notes', 'Notes')]);

fireEvent.click(screen.getByTestId('form-textarea-fullscreen-toggle'));

const dialog = screen.getByTestId('form-textarea-fullscreen-dialog');
expect(dialog).toHaveTextContent('Notes');
// The nail: before the fix this WAS the title, for every field on every
// form. Asserting the label is present is not enough — the generic word
// has to be gone, or a title rendering both would still pass.
expect(screen.queryByText('Edit text')).not.toBeInTheDocument();
});

it('interpolates the label into the expand button accessible name', () => {
renderForm([longText('notes', 'Notes')]);

expect(screen.getByTestId('form-textarea-fullscreen-toggle')).toHaveAttribute(
'aria-label',
'Edit Notes fullscreen',
);
});

it('interpolates into the translated sentence with no locale-pack change', () => {
// objectui#3272 shaped `form.fullscreen.toggle` as ONE interpolated
// sentence (zh puts `{{label}}` last, ja first) precisely so that feeding
// the real label through would make all ten packs correct without touching
// them. This is that promise, cashed.
renderForm([longText('notes', '备注')], 'zh');

expect(screen.getByTestId('form-textarea-fullscreen-toggle')).toHaveAttribute(
'aria-label',
'全屏编辑备注',
);

fireEvent.click(screen.getByTestId('form-textarea-fullscreen-toggle'));
expect(screen.getByTestId('form-textarea-fullscreen-dialog')).toHaveTextContent('备注');
expect(screen.queryByText('编辑文本')).not.toBeInTheDocument();
});

it('gives every long-text field on one form a DISTINCT expand button name', () => {
// The accessibility defect itself. Three fields, three buttons, one name.
renderForm([
longText('notes', 'Notes'),
longText('summary', 'Summary'),
longText('resolution', 'Resolution'),
]);

const names = screen
.getAllByTestId('form-textarea-fullscreen-toggle')
.map((el) => el.getAttribute('aria-label'));

expect(names).toEqual([
'Edit Notes fullscreen',
'Edit Summary fullscreen',
'Edit Resolution fullscreen',
]);
expect(new Set(names).size).toBe(3);
});

it('opens the dialog for the SECOND field with that field own label', () => {
// Distinct names are only useful if the button actually opens the field it
// names — a per-field name plus a shared dialog would be worse than before.
renderForm([longText('notes', 'Notes'), longText('summary', 'Summary')]);

fireEvent.click(screen.getAllByTestId('form-textarea-fullscreen-toggle')[1]);

const dialog = screen.getByTestId('form-textarea-fullscreen-dialog');
expect(dialog).toHaveTextContent('Summary');
expect(dialog).not.toHaveTextContent('Notes');
});

it('still falls back to the generic title when the field has no label', () => {
renderForm([longText('notes')]);

expect(screen.getByTestId('form-textarea-fullscreen-toggle')).toHaveAttribute(
'aria-label',
'Edit text fullscreen',
);
fireEvent.click(screen.getByTestId('form-textarea-fullscreen-toggle'));
expect(screen.getByText('Edit text')).toBeInTheDocument();
});
});

describe('form renderer — the forwarded label reaches no DOM attribute (objectui#3393)', () => {
it('puts no `label` attribute on any control the built-in branches render', () => {
// One field per built-in branch that spreads its leftover props onto a DOM
// node. Before the strip entry existed, forwarding `label` from the call
// site dropped `label="…"` on every one of them; the local
// `const { label: _label, ...rest }` in the `textarea` branch guarded that
// ONE branch only — and, since nothing was forwarded, guarded nothing.
const { container } = renderForm([
{ name: 'title', label: 'Title', type: 'input' },
{ name: 'notes', label: 'Notes', type: 'textarea' },
longText('summary', 'Summary'),
{ name: 'agree', label: 'Agree', type: 'checkbox' },
{ name: 'active', label: 'Active', type: 'switch' },
{ name: 'mystery', label: 'Mystery', type: 'not-a-registered-type' },
]);

expect(container.querySelectorAll('[label]')).toHaveLength(0);
// Named, so a failure says which control leaked rather than just a count.
expect(screen.getByLabelText('Title')).not.toHaveAttribute('label');
expect(screen.getByLabelText('Notes')).not.toHaveAttribute('label');
expect(screen.getByLabelText('Summary')).not.toHaveAttribute('label');
expect(screen.getByLabelText('Mystery')).not.toHaveAttribute('label');
});

it('leaves the visible `<FormLabel>` and its control association intact', () => {
// The strip must not be mistaken for "the label is gone": it is rendered by
// `<FormLabel>` and associated with the control, which is what makes
// `getByLabelText` work at all.
renderForm([longText('notes', 'Notes')]);

expect(screen.getByText('Notes')).toBeInTheDocument();
expect(screen.getByLabelText('Notes').tagName).toBe('TEXTAREA');
});
});

/**
* Props the probe was rendered with, captured per render.
*
* A holder object rather than a bare module variable: `react-hooks/globals`
* forbids REASSIGNING an outer binding from a component body, and mutating a
* property of a stable object is the repo's usual shape for a probe.
*/
const captured: { props: Record<string, any> | null } = { props: null };

/** Stands in for a registered field widget — `@object-ui/components` tests never load `@object-ui/fields`. */
function LabelProbe(props: any) {
captured.props = props;
return <input data-testid={`probe-${props.name}`} value={(props.value as string) ?? ''} readOnly />;
}

describe('form renderer — registered widgets get no new `label` prop (objectui#3393)', () => {
beforeAll(() => {
ComponentRegistry.register('labelprobe', LabelProbe, { namespace: 'field' });
}, 30000);

beforeEach(() => {
captured.props = null;
});

it('strips `label` on the registered path, leaving `field` the one carrier', () => {
// Deliberately NOT "widgets should receive `label`". The built-in branch is
// the only reader this issue is scoped to; whether the published
// `FieldWidgetPropsSchema` should grow a `label` prop is a contract
// decision of its own (objectui#3233 just converged which registry entry
// decides which contract). Until that decision is taken, a widget reads the
// label where it has always read it — off `field`.
renderForm([{ name: 'notes', label: 'Notes', type: 'labelprobe' }]);

const props = captured.props!;
expect(props).not.toBeNull();
expect(props.label).toBeUndefined();
expect(props.field.label).toBe('Notes');
});
});
46 changes: 45 additions & 1 deletion packages/components/src/renderers/form/form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,16 @@ function stripRendererOnlyProps<T extends Record<string, any>>(props: T): T {
// is a React warning, so it is stripped here exactly like `dependentValues`.
dependsOnLabels: _dependsOnLabels,
emptyHint: _emptyHint,
// The field's own label (objectui#3393). It is forwarded from the single
// call site for ONE reader — the built-in `textarea` branch's fullscreen
// dialog, which names the field in its title and in the expand button's
// accessible name — and is renderer-only everywhere else: `<FormLabel>`
// above already renders it, and every other built-in branch spreads its
// leftover props straight onto a DOM node, where `label` would show up as
// a stray `label="Notes"` attribute on an `<input>` / `<textarea>` /
// Radix control. The `textarea` branch therefore reads it off the
// PRE-strip props, exactly like `mobile_fullscreen`.
label: _label,
// The validation message (objectui#3222) is for REGISTERED widgets, which
// need it to put `aria-invalid` on the control they render. The builtin
// branch below renders its control directly inside `<FormControl>`, whose
Expand Down Expand Up @@ -320,6 +330,17 @@ function stripRegisteredFieldProps(type: string, props: RenderFieldProps): Rende
dependentValues,
dependsOnLabels,
emptyHint,
// Renderer-only, and deliberately NOT a new key in the widget contract
// (objectui#3393). The call site started forwarding `label` for the
// built-in fullscreen dialog; registered widgets must keep receiving
// exactly what they received before, because `field` is their single
// metadata carrier since v17 (objectui#3233) and it already carries the
// label. Adding a second spelling here would give every AI-authored widget
// two places to read one fact — and, since widgets spread their leftover
// props onto the control they render, would also drop a `label` attribute
// onto seven widgets' DOM overnight. Whether the contract SHOULD publish a
// `label` prop is a separate decision against `FieldWidgetPropsSchema`.
label: _label,
// Retired from the widget contract in v17 (objectui#3233): `field` is the
// single metadata carrier. The strip stays so an authored form field that
// happens to carry a `schema` key cannot resurrect the second carrier —
Expand Down Expand Up @@ -1448,6 +1469,15 @@ ComponentRegistry.register('form',
field: field.field || field,
...formField,
inputType: fieldProps.inputType,
// The field's label, forwarded explicitly because the
// destructure above takes it OFF `fieldProps` (objectui#3393).
// Without this key the built-in `textarea` branch's `label`
// was `undefined` on every render, so the fullscreen dialog's
// title fell to the generic "Edit text" and all three long-text
// fields on one form gave their expand buttons the SAME
// accessible name. Renderer-only: both strips drop it, so it
// reaches no DOM attribute and no registered widget.
label,
options: isOptionField ? effectiveOptions : fieldProps.options,
placeholder: fieldProps.placeholder ?? (resolvedType === 'select' ? t('common.selectOption') : undefined),
// `disabled` means "not interactive, muted"; `readonly` means
Expand Down Expand Up @@ -1869,6 +1899,12 @@ interface RenderFieldProps {
* `@objectstack/spec/ui`'s `FieldWidgetPropsSchema` gives it (objectui#3222).
*/
error?: string;
/**
* The field's authored label (objectui#3393). Renderer-only: consumed by the
* built-in `textarea` branch's fullscreen dialog, stripped before the DOM and
* before every registered widget (which reads it off `field`).
*/
label?: string;
[key: string]: any;
}

Expand Down Expand Up @@ -2020,8 +2056,16 @@ function renderFieldComponent(type: string, props: RenderFieldProps) {
// whoever reads this file next (AGENTS.md #0.1). `ObjectForm` is the sole
// producer and it stamps `mobile_fullscreen` (#3245/#3300), which is also
// the single spelling `TextAreaField` and `RichTextField` read.
//
// `label` rides the same way: read off the PRE-strip props, because
// `stripRendererOnlyProps` discards it for the DOM (objectui#3393). It
// used to be discarded by a local `const { label: _label, ...rest }` on
// the next line — which ESLint flagged as an unused binding, correctly:
// the call site never forwarded a `label`, so the guard was defending
// against something that never arrived, while every OTHER built-in
// branch had no guard at all. Now the strip owns it for all branches.
const { mobile_fullscreen, label } = fieldProps as any;
const { label: _label, ...rest } = stripRendererOnlyProps(fieldProps);
const rest = stripRendererOnlyProps(fieldProps);
if (mobile_fullscreen) {
return (
<FullscreenTextarea
Expand Down
Loading