diff --git a/.changeset/form-fullscreen-textarea-field-label.md b/.changeset/form-fullscreen-textarea-field-label.md new file mode 100644 index 0000000000..991ac3872f --- /dev/null +++ b/.changeset/form-fullscreen-textarea-field-label.md @@ -0,0 +1,18 @@ +--- +'@object-ui/components': patch +--- + +表单内置 `textarea` 的全屏编辑对话框现在能拿到字段自己的 label：对话框标题显示字段名而不是恒定的通用词「编辑文本」，同一张表单上多个长文本字段的展开按钮也终于有了互不相同的无障碍名（objectui#3393）。 + +`renderFormField` 在解构字段配置时把 `label` 单独取走了（它要渲染 ``），而下游 `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 时仍回落到被翻译的通用词。 diff --git a/packages/components/src/renderers/form/__tests__/form-fullscreen-textarea-i18n.test.tsx b/packages/components/src/renderers/form/__tests__/form-fullscreen-textarea-i18n.test.tsx index 6c80969fc9..b0a85f0739 100644 --- a/packages/components/src/renderers/form/__tests__/form-fullscreen-textarea-i18n.test.tsx +++ b/packages/components/src/renderers/form/__tests__/form-fullscreen-textarea-i18n.test.tsx @@ -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'; @@ -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( @@ -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 `` association — this case asserts wiring, not copy. + renderFormIn('zh', labelledFields); openFullscreen(); fireEvent.change(screen.getByTestId('form-textarea-fullscreen-input'), { diff --git a/packages/components/src/renderers/form/__tests__/form-fullscreen-textarea-label.test.tsx b/packages/components/src/renderers/form/__tests__/form-fullscreen-textarea-label.test.tsx new file mode 100644 index 0000000000..1ce6c0ffb0 --- /dev/null +++ b/packages/components/src/renderers/form/__tests__/form-fullscreen-textarea-label.test.tsx @@ -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: `` 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( + +
+ , + ); +} + +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 `` and its control association intact', () => { + // The strip must not be mistaken for "the label is gone": it is rendered by + // `` 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 | 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 ; +} + +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'); + }); +}); diff --git a/packages/components/src/renderers/form/form.tsx b/packages/components/src/renderers/form/form.tsx index 67a2f102ae..b736aaffa6 100644 --- a/packages/components/src/renderers/form/form.tsx +++ b/packages/components/src/renderers/form/form.tsx @@ -264,6 +264,16 @@ function stripRendererOnlyProps>(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: `` + // 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 `` / `