diff --git a/.changeset/console-chrome-i18n-5407.md b/.changeset/console-chrome-i18n-5407.md new file mode 100644 index 0000000000..385369e552 --- /dev/null +++ b/.changeset/console-chrome-i18n-5407.md @@ -0,0 +1,29 @@ +--- +'@object-ui/components': minor +'@object-ui/fields': minor +'@object-ui/plugin-detail': patch +'@object-ui/i18n': patch +--- + +Console chrome i18n gaps (objectstack#5407). + +- A dependency-gated lookup now names its controlling field by its **label** + instead of its raw API name. The sentence was localized but the interpolated + name was not, so every locale — English included — read `Select crm_account + first`. The form renderer passes a new `dependsOnLabels` widget prop (the + lookup-side counterpart of `emptyHint`, which it already resolves to labels + for the fixed-option widgets); a name the host does not cover still falls + back to itself. +- The page-header overflow trigger's `More actions` accessible name now reads + `detail.moreActions`, the same key `action:menu`'s own overflow trigger uses, + so the two cannot diverge per locale. +- The activity-feed reaction button's `Add reaction` accessible name is now a + bundle key (`detail.addReaction`, added to all ten packs). +- The "check the highlighted fields" toast joins field names with a per-locale + separator (`validation.formInvalidJoiner`) instead of a hardcoded `、` + (U+3001) — right for zh/ja by accident, wrong in English and every Latin + locale. Latin packs use `, `, CJK `、`, Arabic `، `. +- The Spanish `validation.required` / `validation.unique` templates gained + their own masculine head noun (`El campo {{field}} es obligatorio`) so the + adjective agrees for feminine field labels too — `Cuenta es obligatorio` was + ungrammatical. diff --git a/packages/components/src/__tests__/page-header-more-actions-i18n.test.tsx b/packages/components/src/__tests__/page-header-more-actions-i18n.test.tsx new file mode 100644 index 0000000000..e09eb71142 --- /dev/null +++ b/packages/components/src/__tests__/page-header-more-actions-i18n.test.tsx @@ -0,0 +1,89 @@ +/** + * 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 page header's overflow trigger speaks the session locale — + * objectstack#5407. + * + * `page:header` collapses everything past `maxVisible` into a `⋯` button whose + * accessible name was the English literal "More actions". The button is + * icon-only, so that literal IS the button to a screen reader (and to a hover + * tooltip): under a zh/ja/es session it was the only English left in the + * header row. + * + * It now reads `detail.moreActions` — deliberately the SAME key `action:menu`'s + * own overflow trigger already used, not a new one. A record page can show both + * `⋯` buttons at once, and two keys would let them drift apart per locale. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import { ActionProvider } from '@object-ui/react'; +import { I18nProvider } from '@object-ui/i18n'; +// Registers `page:header` at module scope, NOT inside a `beforeAll` — there the +// cold transform is billed to `hookTimeout`. Importing it here also keeps this +// file out of the `heavyDomTests` list: it brings its own registration instead +// of depending on the full DOM setup to have run one +// (object-ui/no-dynamic-import-in-test-hook, objectui#3010/#3021). +import '../renderers'; + +function PageHeader({ schema }: { schema: any }) { + const Component = ComponentRegistry.get('page:header'); + if (!Component) throw new Error('page:header not registered'); + // eslint-disable-next-line react-hooks/static-components -- ComponentRegistry.get returns a registered component (stable), not one created during render + return ; +} + +/** Four header actions against the default `maxVisible` of 3 → one overflows. */ +const schema = { + type: 'page:header', + title: 'Acme Corp', + actions: [ + { name: 'convert', locations: ['record_header'], label: 'Convert', type: 'flow' }, + { name: 'clone', locations: ['record_header'], label: 'Clone', type: 'flow' }, + { name: 'share', locations: ['record_header'], label: 'Share', type: 'flow' }, + { name: 'archive', locations: ['record_header'], label: 'Archive', type: 'flow' }, + ], +}; + +function renderHeaderIn(language: string) { + return render( + + + + + , + ); +} + +afterEach(() => cleanup()); + +describe('page:header overflow trigger — accessible name (objectstack#5407)', () => { + it('reads the zh bundle value under a zh session', () => { + renderHeaderIn('zh'); + + expect(screen.getByRole('button', { name: '更多操作' })).toBeTruthy(); + // The literal this replaced. Asserted negatively too: a re-inlined English + // string would still let the positive assertion pass if the header ever + // rendered two overflow triggers. + expect(screen.queryByRole('button', { name: 'More actions' })).toBeNull(); + }); + + it('reads the ja bundle value under a ja session', () => { + renderHeaderIn('ja'); + + expect(screen.getByRole('button', { name: 'その他の操作' })).toBeTruthy(); + }); + + it('still reads English under an en session', () => { + renderHeaderIn('en'); + + expect(screen.getByRole('button', { name: 'More actions' })).toBeTruthy(); + }); +}); diff --git a/packages/components/src/renderers/form/__tests__/form-depends-on-labels.test.tsx b/packages/components/src/renderers/form/__tests__/form-depends-on-labels.test.tsx new file mode 100644 index 0000000000..75cb2e8d42 --- /dev/null +++ b/packages/components/src/renderers/form/__tests__/form-depends-on-labels.test.tsx @@ -0,0 +1,168 @@ +/** + * 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 form renderer hands data-source widgets the LABELS of the sibling fields + * whose VALUES it already hands them — objectstack#5407. + * + * A dependency-gated lookup has to name its controlling field in user-visible + * copy ("Select Account first"), but the widget only ever sees `depends_on`, + * which holds API names. Only the form knows the label. It already resolves + * exactly this map for the fixed-option widgets (`emptyHint`); the lookup side + * had no equivalent, so the gate sentence interpolated `crm_account` into every + * locale, English included. + * + * This pins the PLUMBING (form → widget prop). The widget's own use of the map + * is pinned in `packages/fields`' `LookupField.gateHintLabel.test.tsx`; the two + * halves are asserted separately because `@object-ui/components` cannot import + * `@object-ui/fields` (that is the dependency direction, not a test shortcut). + * + * The strip half matters as much as the pass half: `stripRegisteredFieldProps` + * allow-lists these props precisely so an unknown object prop cannot reach a + * DOM node through a widget's `...props` spread — a React warning, and the + * reason `emptyHint` is allow-listed rather than passed unconditionally. + */ + +import { describe, it, expect, beforeAll, afterEach, vi } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +// Registers the renderers at module scope, NOT inside a `beforeAll` — there the +// cold transform is billed to `hookTimeout`. See +// object-ui/no-dynamic-import-in-test-hook (objectui#3010/#3021). +import '../../../renderers'; + +/** Props each stub widget saw on its last render, keyed by registry type. */ +const seen: Record = {}; + +function makeStub(type: string) { + return function Stub(props: any) { + seen[type] = props; + return
; + }; +} + +beforeAll(() => { + // `field:lookup` is contributed by `@object-ui/fields`, which this package + // does not (and must not) depend on — so registering a stub here shadows + // nothing and leaks into no other suite. + ComponentRegistry.register('field:lookup', makeStub('lookup')); + ComponentRegistry.register('field:tags', makeStub('tags')); +}); + +afterEach(() => cleanup()); + +const fields = [ + { name: 'crm_account', label: 'Account', type: 'input' }, + { + name: 'contact', + label: 'Contact', + type: 'lookup', + field: { name: 'contact', reference_to: 'crm_contact', depends_on: ['crm_account'] }, + }, + // A widget outside the data-source family, to pin the strip half. + { name: 'topics', label: 'Topics', type: 'tags' }, +]; + +function renderForm() { + const Form = ComponentRegistry.get('form')!; + return render( +
, + ); +} + +describe('form renderer — dependsOnLabels plumbing (objectstack#5407)', () => { + it('passes a data-source widget the sibling field name → label map', () => { + renderForm(); + + expect(seen.lookup.dependsOnLabels).toEqual( + expect.objectContaining({ crm_account: 'Account', contact: 'Contact' }), + ); + }); + + it('keys the map by API name, so the widget can resolve its own depends_on', () => { + renderForm(); + + // The exact lookup the widget performs. Written as the resolution rather + // than as a shape assertion, because the shape is only useful if this + // reads back the label. + const depends = 'crm_account'; + expect(seen.lookup.dependsOnLabels[depends]).toBe('Account'); + }); + + it('falls back to the field name for a field that declared no label', () => { + const Form = ComponentRegistry.get('form')!; + render( + , + ); + + // No label authored → the map still answers, with the name. The widget's + // own `|| d.field` fallback therefore never has to fire for a field the + // form knows about. + expect(seen.lookup.dependsOnLabels.crm_account).toBe('crm_account'); + }); + + it('strips the map for widgets outside the data-source family', () => { + renderForm(); + + // `tags` spreads its leftover props onto a DOM node; an object-valued + // `dependsOnLabels` attribute there is a React warning. + expect(seen.tags).toBeDefined(); + expect(seen.tags.dependsOnLabels).toBeUndefined(); + }); + + it('strips the map before the BUILTIN branch reaches the DOM', () => { + // The builtin types (`input`/`textarea`/`checkbox`/`switch`/`select`) never + // pass through `stripRegisteredFieldProps` — they render their control + // directly and spread what is left onto it. `stripRendererOnlyProps` is the + // strip that covers them, and it was the one this prop was first missing: + // every plain text field on every form logged "React does not recognize the + // `dependsOnLabels` prop on a DOM element". + const errors: unknown[][] = []; + const spy = vi.spyOn(console, 'error').mockImplementation((...args) => { + errors.push(args); + }); + try { + const Form = ComponentRegistry.get('form')!; + const { container } = render( + , + ); + + const input = container.querySelector('input')!; + expect(input).not.toBeNull(); + expect(input.getAttributeNames().map((n) => n.toLowerCase())).not.toContain( + 'dependsonlabels', + ); + expect( + errors.filter((e) => e.some((a) => String(a).includes('dependsOnLabels'))), + ).toEqual([]); + } finally { + spy.mockRestore(); + } + }); +}); diff --git a/packages/components/src/renderers/form/__tests__/form-invalid-toast-joiner.test.tsx b/packages/components/src/renderers/form/__tests__/form-invalid-toast-joiner.test.tsx new file mode 100644 index 0000000000..14dcd8f5b9 --- /dev/null +++ b/packages/components/src/renderers/form/__tests__/form-invalid-toast-joiner.test.tsx @@ -0,0 +1,105 @@ +/** + * 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 invalid-submit toast joins the field names with a PER-LOCALE separator — + * objectstack#5407. + * + * `announceFieldErrors` used to build the list with a hardcoded `、` (U+3001). + * That is the CJK enumeration comma: correct for zh/ja by accident, wrong in + * every Latin locale, and most visibly wrong in English, where the toast read + * + * Please check the highlighted fields: Subject、Account、Status + * + * List punctuation is a property of the locale, not of the code, so the + * separator is now its own bundle entry (`validation.formInvalidJoiner`) that + * each pack declares. `Intl.ListFormat` was measured and rejected for this + * call site: its `unit` styles emit an EMPTY separator for zh and ru, and its + * `conjunction` styles splice in "and"/"和"/"y", which reads as a sentence + * rather than as the truncated list this is ("A, B, C…"). + * + * The two locales asserted here are the two SIDES of the bug: `en` is the one + * that was visibly wrong, `zh` the one that was right by accident and must + * stay right now that the value is declared rather than assumed. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import { I18nProvider } from '@object-ui/i18n'; +import { toast } from '../../../ui/sonner'; +// Registers the renderers at module scope, NOT inside a `beforeAll` — there the +// cold transform is billed to `hookTimeout`. See +// object-ui/no-dynamic-import-in-test-hook (objectui#3010/#3021). +import '../../../renderers'; + +let toastErrorSpy: ReturnType; + +beforeEach(() => { + toastErrorSpy = vi.spyOn(toast, 'error').mockImplementation(() => 'id' as any); + if (!(Element.prototype as any).scrollIntoView) { + (Element.prototype as any).scrollIntoView = () => {}; + } +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +/** Three required fields, so the joiner appears twice and truncation does not. */ +const fields = [ + { name: 'subject', label: 'Subject', type: 'input', required: true }, + { name: 'account', label: 'Account', type: 'input', required: true }, + { name: 'status', label: 'Status', type: 'input', required: true }, +]; + +function renderFormIn(language: string) { + const Form = ComponentRegistry.get('form')!; + return render( + + + , + ); +} + +async function toastTextAfterSubmit(language: string): Promise { + renderFormIn(language); + fireEvent.click(screen.getByRole('button', { name: /create/i })); + await waitFor(() => expect(toastErrorSpy).toHaveBeenCalled()); + return String(toastErrorSpy.mock.calls[0][0]); +} + +describe('form renderer — invalid-submit toast list joiner (objectstack#5407)', () => { + it('joins with a comma+space under an en session, never the CJK comma', async () => { + const text = await toastTextAfterSubmit('en'); + + expect(text).toContain('Subject, Account, Status'); + // The literal this replaced. Asserted negatively as well as positively so a + // re-hardcoded joiner cannot pass by rendering both forms somewhere. + expect(text).not.toContain('、'); + }); + + it('still joins with the CJK comma under a zh session', async () => { + const text = await toastTextAfterSubmit('zh'); + + expect(text).toContain('Subject、Account、Status'); + // zh's sentence, to prove the joiner is being read from the zh pack and not + // from an `en` fallback that happens to carry the same punctuation. + expect(text).toContain('请检查表单中标记的字段'); + }); +}); diff --git a/packages/components/src/renderers/form/form.tsx b/packages/components/src/renderers/form/form.tsx index c6a70a0021..59c1de8d33 100644 --- a/packages/components/src/renderers/form/form.tsx +++ b/packages/components/src/renderers/form/form.tsx @@ -128,6 +128,12 @@ const useSafeFormTranslation = createSafeTranslation( 'validation.email': 'Please enter a valid email address', 'validation.url': 'Please enter a valid URL', 'validation.formInvalid': 'Please check the highlighted fields: {{fields}}', + // objectstack#5407 — the joiner between the field names is a LOCALE + // property, not a code constant. It used to be a hardcoded `、` (U+3001), + // which is right for zh/ja and wrong everywhere else — including `en`, + // where the toast read "Subject、Account、Status". A locale that ships no + // entry falls back through i18next to `en`'s ", ". + 'validation.formInvalidJoiner': ', ', 'errors.forbidden': 'Access denied.', 'form.noPermissionToSave': "You don't have permission to save this record.", 'form.submitFailed': 'Could not save. Please try again.', @@ -226,6 +232,11 @@ function stripRendererOnlyProps>(props: T): T { fullscreen: _fullscreen, dependentValues: _dependentValues, dependsOn: _dependsOn, + // objectstack#5407 — the sibling name→label map is for widgets that NAME a + // controlling field in their own copy (the lookup gate hint). No builtin + // branch renders such copy, and an object-valued prop reaching a DOM node + // is a React warning, so it is stripped here exactly like `dependentValues`. + dependsOnLabels: _dependsOnLabels, emptyHint: _emptyHint, // The validation message (objectui#3222) is for REGISTERED widgets, which // need it to put `aria-invalid` on the control they render. The builtin @@ -282,6 +293,7 @@ function stripRegisteredFieldProps(type: string, props: RenderFieldProps): Rende mobile_fullscreen: _mobileFullscreen, fullscreen: _fullscreen, dependentValues, + dependsOnLabels, emptyHint, // Retired from the widget contract in v17 (objectui#3233): `field` is the // single metadata carrier. The strip stays so an authored form field that @@ -294,7 +306,12 @@ function stripRegisteredFieldProps(type: string, props: RenderFieldProps): Rende return { ...fieldProps, - ...(DATA_SOURCE_FIELD_TYPES.has(normalizedType) ? { dataSource, dependentValues } : {}), + // `dependsOnLabels` rides with `dependentValues`: the widgets that gate on + // a sibling field's VALUE are exactly the ones that have to NAME that field + // in the gate hint (objectstack#5407). Stripped for everything else for the + // same reason `emptyHint` is — an unknown object prop reaching a DOM node + // through a widget's `...props` spread is a React warning. + ...(DATA_SOURCE_FIELD_TYPES.has(normalizedType) ? { dataSource, dependentValues, dependsOnLabels } : {}), // The cascade option widgets own the gate hint's presentation, so they get // the computed `emptyHint` alongside the live record (objectui#3231). It is // stripped by default because every OTHER registered widget spreads its @@ -877,7 +894,9 @@ ComponentRegistry.register('form', setRejectedFieldNames(names); const labels = names.map((n) => fieldLabelByName[n] || n); const MAX = 3; - const fieldsText = labels.slice(0, MAX).join('、') + (labels.length > MAX ? '…' : ''); + const fieldsText = + labels.slice(0, MAX).join(t('validation.formInvalidJoiner')) + + (labels.length > MAX ? '…' : ''); toast.error(t('validation.formInvalid', { fields: fieldsText })); const errored = new Set(names); @@ -1410,6 +1429,14 @@ ComponentRegistry.register('form', // form, not read a stale record snapshot. Forwarded to // data-source widgets only (see stripRegisteredFieldProps). dependentValues: ruleRecord, + // Field name → label for the SAME sibling fields + // `dependentValues` carries the values of, so a gated lookup + // can say "Select Account first" instead of naming the raw + // API name (objectstack#5407). The whole form's map is passed + // (not a per-field slice) because the widget reads only the + // entries its own `depends_on` names, and one stable + // reference keeps the widget's memo deps from thrashing. + dependsOnLabels: fieldLabelByName, // The validation slot the widget props contract has declared // all along and nobody produced (objectui#3222). Spelled // `error` because that is what `@objectstack/spec/ui`'s diff --git a/packages/components/src/renderers/layout/containers.tsx b/packages/components/src/renderers/layout/containers.tsx index f179cb20ae..7f7ab37485 100644 --- a/packages/components/src/renderers/layout/containers.tsx +++ b/packages/components/src/renderers/layout/containers.tsx @@ -47,7 +47,7 @@ import { DropdownMenuItem, } from '../../ui'; import { RecordTitleChip } from '../../custom/RecordTitleChip'; -import { useObjectLabel, useSafeFieldLabel, useObjectTranslation, pickLocalized } from '@object-ui/i18n'; +import { useObjectLabel, useSafeFieldLabel, useObjectTranslation, useSafeTranslate, pickLocalized } from '@object-ui/i18n'; import { MoreHorizontal } from 'lucide-react'; /** @@ -875,6 +875,10 @@ const PageHeaderRenderer: React.FC = ({ schema, className, ...props }) => { const { objectLabel: tObjectLabel, actionLabel: tActionLabel } = useObjectLabel(); const { fieldOptionLabel } = useSafeFieldLabel(); const { language } = useObjectTranslation(); + // Console-supplied chrome copy (objectstack#5407). `detail.moreActions` is + // the SAME key `action:menu`'s overflow trigger already reads, so the two + // `⋯` buttons a record page can show cannot read differently per locale. + const tt = useSafeTranslate(); // Spec bridge may either inline `properties.*` onto the node or preserve // the raw bag (see record:quick_actions for the same pattern). Read from // both so a `{ properties: { title } }` schema is rendered correctly. @@ -1204,7 +1208,7 @@ const PageHeaderRenderer: React.FC = ({ schema, className, ...props }) => { variant="outline" size="sm" className="gap-1 px-2" - aria-label="More actions" + aria-label={tt('detail.moreActions', 'More actions')} > diff --git a/packages/fields/src/widgets/LookupField.gateHintLabel.test.tsx b/packages/fields/src/widgets/LookupField.gateHintLabel.test.tsx new file mode 100644 index 0000000000..38653c2292 --- /dev/null +++ b/packages/fields/src/widgets/LookupField.gateHintLabel.test.tsx @@ -0,0 +1,135 @@ +/** + * 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. + */ + +/** + * A gated lookup names its controlling field by LABEL — objectstack#5407. + * + * The gate sentence (`lookup.selectFirst`) is translated in all ten packs, but + * its `{{fields}}` interpolation came straight off `depends_on`, which holds + * API names. So the rendered hint was localized around a raw identifier in + * every locale, English included: + * + * en Select crm_account first + * zh-CN 请先选择crm_account + * + * The `en` row is why this is not merely an i18n bug: an English user was shown + * an internal identifier. The form renderer knows the label (it already + * resolves the same map for the fixed-option widgets' `emptyHint`) and now + * hands it over as `dependsOnLabels`. + * + * All three gated surfaces are asserted — the trigger's visible text, the + * trigger's `title`, and the "browse all" button's `title` — because they were + * three independent copies of the same expression and a fix applied to one of + * them looks identical from the outside. + */ + +import { render, screen, cleanup } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { LookupField } from './LookupField'; + +/** Mirrors the reported metadata: the parent field's API name IS an object-ish + * name (`crm_account`), which is exactly what made the leak so legible. */ +const gatedField = { + name: 'contact', + label: 'Contact', + reference_to: 'crm_contact', + reference_field: 'name', + depends_on: ['crm_account'], +} as any; + +const dataSource = { find: vi.fn(async () => ({ data: [], total: 0 })) } as any; + +function renderGated(extra: Record) { + return render( + , + ); +} + +beforeEach(() => { + Object.defineProperty(window, 'innerWidth', { writable: true, configurable: true, value: 1280 }); + window.matchMedia = ((query: string) => ({ + matches: false, + media: query, + onchange: null, + addEventListener: () => {}, + removeEventListener: () => {}, + addListener: () => {}, + removeListener: () => {}, + dispatchEvent: () => false, + })) as any; +}); + +afterEach(() => cleanup()); + +describe('LookupField gate hint names the controlling field (objectstack#5407)', () => { + it('interpolates the label the host supplied, not the API name', () => { + renderGated({ dependsOnLabels: { crm_account: 'Account' } }); + + const trigger = screen.getByTestId('lookup-trigger-gated'); + expect(trigger).toHaveTextContent('Select Account first'); + // The identifier must be gone from the copy entirely, not merely + // accompanied by the label. + expect(trigger).not.toHaveTextContent('crm_account'); + expect(trigger).toHaveAttribute('title', 'Select Account first'); + }); + + it('labels the gated "browse all" button the same way', () => { + renderGated({ dependsOnLabels: { crm_account: 'Account' } }); + + const browse = screen.getByTestId('browse-all-records'); + expect(browse).toBeDisabled(); + expect(browse).toHaveAttribute('title', 'Select Account first'); + }); + + it('joins several controlling fields by their labels', () => { + render( + , + ); + + expect(screen.getByTestId('lookup-trigger-gated')).toHaveTextContent( + 'Select Account, Lead Source first', + ); + }); + + it('falls back to the API name for a field the host did not map', () => { + // Standalone use (no host form) has to keep rendering what it rendered + // before — the map is an enrichment, not a new requirement. This is also + // the assertion that would go red if `dependsOnLabels` were consulted for + // its VALUES rather than keyed by field name. + renderGated({ dependsOnLabels: { some_other_field: 'Irrelevant' } }); + + expect(screen.getByTestId('lookup-trigger-gated')).toHaveTextContent( + 'Select crm_account first', + ); + }); + + it('falls back to the API name when the host passes no map at all', () => { + renderGated({}); + + expect(screen.getByTestId('lookup-trigger-gated')).toHaveTextContent( + 'Select crm_account first', + ); + }); +}); diff --git a/packages/fields/src/widgets/LookupField.tsx b/packages/fields/src/widgets/LookupField.tsx index ad3369c86a..1deadce319 100644 --- a/packages/fields/src/widgets/LookupField.tsx +++ b/packages/fields/src/widgets/LookupField.tsx @@ -280,6 +280,23 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro return []; }, [fieldMeta?.depends_on, fieldMeta?.dependsOn]); + /** + * The gate sentence's `{{fields}}` — the controlling fields named the way the + * user sees them on the form, not the way the metadata spells them. + * + * `depends_on` holds API names, and this used to interpolate them straight + * into the sentence, so every locale — `en` included — read "Select + * crm_account first" (objectstack#5407): an internal identifier in the UI, + * not merely an untranslated word. The host form supplies the name→label map + * (`dependsOnLabels`); a name it doesn't cover falls back to itself, so a + * standalone widget with no host renders exactly what it did before. + */ + const dependsOnLabelsProp = props.dependsOnLabels; + const dependsOnFieldsText = useMemo( + () => dependsOn.map((d) => dependsOnLabelsProp?.[d.field] || d.field).join(', '), + [dependsOn, dependsOnLabelsProp], + ); + // Resolve dependent field values from explicit prop or SchemaRendererContext.data const dependentValuesProp = props.dependentValues; @@ -931,7 +948,7 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro disabled={dependenciesMissing || props.disabled} data-testid={dependenciesMissing ? 'lookup-trigger-gated' : ((props.name || lookupField?.name) ? `lookup-trigger-${props.name || lookupField.name}` : 'lookup-trigger')} title={dependenciesMissing - ? t('lookup.selectFirst', { fields: dependsOn.map(d => d.field).join(', ') }) + ? t('lookup.selectFirst', { fields: dependsOnFieldsText }) : undefined} // AFTER the spread so this widget's own computation wins (#3222): // `fieldError` is the published validation slot — NOT the popover's @@ -948,7 +965,7 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro )} {dependenciesMissing - ? t('lookup.selectFirst', { fields: dependsOn.map(d => d.field).join(', ') }) + ? t('lookup.selectFirst', { fields: dependsOnFieldsText }) : hydrating // The value EXISTS but its labels are still loading — say so // instead of the empty placeholder (#3108). @@ -1230,7 +1247,7 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro onClick={() => setIsPickerOpen(true)} aria-label={t('lookup.browseAll')} title={dependenciesMissing - ? t('lookup.selectFirst', { fields: dependsOn.map(d => d.field).join(', ') }) + ? t('lookup.selectFirst', { fields: dependsOnFieldsText }) : t('lookup.browseAll')} data-testid="browse-all-records" > diff --git a/packages/fields/src/widgets/types.ts b/packages/fields/src/widgets/types.ts index cdfea8c77d..5c35d7d885 100644 --- a/packages/fields/src/widgets/types.ts +++ b/packages/fields/src/widgets/types.ts @@ -184,6 +184,20 @@ export type FieldWidgetComponentProps = { * a field it synthesised. Field metadata wins when both are present. */ dependsOn?: DependsOnInput; + /** + * Controlling-field name → its human LABEL, for widgets that have to NAME a + * sibling field in user-visible copy (the dependent-lookup gate hint). + * + * Only the form renderer knows a field's label — the widget sees its own + * metadata and a `depends_on` list of API names. Without this map the gate + * sentence interpolated the raw API name into every locale, `en` included + * ("Select crm_account first"), which is an internal identifier leaking into + * the UI, not merely an untranslated word (objectstack#5407). This is the + * lookup-side counterpart of `emptyHint`, which the form already resolves to + * labels for the fixed-option widgets. A name with no entry falls back to + * itself, so a host that passes nothing renders exactly what it did before. + */ + dependsOnLabels?: Record; /** * Hint shown when an option list cannot be filled — typically a * dependency-gated list still waiting on its controlling field (#2284). diff --git a/packages/i18n/src/__tests__/validation-list-joiner-locale-parity.test.ts b/packages/i18n/src/__tests__/validation-list-joiner-locale-parity.test.ts new file mode 100644 index 0000000000..747fcad392 --- /dev/null +++ b/packages/i18n/src/__tests__/validation-list-joiner-locale-parity.test.ts @@ -0,0 +1,88 @@ +/** + * 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. + */ + +/** + * `validation.formInvalidJoiner` — objectstack#5407. + * + * `all-locales-key-parity` guarantees every pack DEFINES the key. It cannot + * catch the defect this key exists to fix, which was about the VALUE: the + * renderer hardcoded `、` (U+3001) for every locale, so the English toast read + * "Subject、Account、Status". A pack that back-fills the entry by copying en's + * `", "` into zh would satisfy parity and re-break Chinese, so the script + * families are asserted here by hand. + * + * `Intl.ListFormat` would have needed no entries at all and was measured + * first — it is rejected because its `unit` styles emit an EMPTY separator for + * zh and ru (worse than the bug) and its `conjunction` styles splice in + * "and"/"和"/"y", which does not belong in a truncated "A, B, C…" list. + */ +import { describe, it, expect } from 'vitest'; +import { builtInLocales } from '../locales'; + +const JOINER = 'formInvalidJoiner'; +/** U+3001 IDEOGRAPHIC COMMA — the CJK list separator. */ +const CJK_COMMA = '、'; +/** U+060C ARABIC COMMA. */ +const ARABIC_COMMA = '،'; + +const validationOf = (lang: string) => + (builtInLocales as Record)[lang].validation as Record; + +describe('validation.formInvalidJoiner is declared per locale (objectstack#5407)', () => { + it('every built-in pack declares a non-empty joiner', () => { + const langs = Object.keys(builtInLocales); + expect(langs.length).toBeGreaterThanOrEqual(10); + for (const lang of langs) { + const joiner = validationOf(lang)[JOINER]; + expect(typeof joiner, `${lang} joiner type`).toBe('string'); + expect(joiner.length, `${lang} joiner is empty`).toBeGreaterThan(0); + } + }); + + it('CJK packs enumerate with U+3001', () => { + for (const lang of ['zh', 'ja']) { + expect(validationOf(lang)[JOINER], lang).toBe(CJK_COMMA); + } + }); + + it('Latin-script and Korean packs enumerate with a comma + space', () => { + // Korean sits here rather than with zh/ja: it uses the ASCII comma. + for (const lang of ['en', 'es', 'de', 'fr', 'pt', 'ru', 'ko']) { + expect(validationOf(lang)[JOINER], lang).toBe(', '); + } + }); + + it('the Arabic pack enumerates with U+060C', () => { + expect(validationOf('ar')[JOINER]).toBe(`${ARABIC_COMMA} `); + }); + + it('no pack outside zh/ja carries the CJK comma — the exact shipped defect', () => { + const offenders = Object.keys(builtInLocales).filter( + (lang) => !['zh', 'ja'].includes(lang) && validationOf(lang)[JOINER].includes(CJK_COMMA), + ); + expect(offenders).toEqual([]); + }); +}); + +describe('es validation templates agree in gender (objectstack#5407)', () => { + // "{{field}} es obligatorio" only agreed when the field label was masculine — + // "Cuenta es obligatorio" is wrong. The template now supplies its own + // masculine head noun so the adjective agrees with THAT, whatever the label. + it('required and unique carry their own head noun', () => { + const es = validationOf('es'); + expect(es.required).toBe('El campo {{field}} es obligatorio'); + expect(es.unique).toBe('El campo {{field}} debe ser único'); + }); + + it('neither template starts with the bare interpolation', () => { + const es = validationOf('es'); + for (const key of ['required', 'unique']) { + expect(es[key].startsWith('{{field}}'), `es.validation.${key}`).toBe(false); + } + }); +}); diff --git a/packages/i18n/src/locales/ar.ts b/packages/i18n/src/locales/ar.ts index 94f3f4cbd8..78017f8026 100644 --- a/packages/i18n/src/locales/ar.ts +++ b/packages/i18n/src/locales/ar.ts @@ -103,6 +103,7 @@ const ar = { url: "يرجى إدخال رابط صالح", pattern: "صيغة {{field}} غير صالحة", formInvalid: "يرجى التحقق من الحقول المميزة: {{fields}}", + formInvalidJoiner: "، ", unique: "{{field}} يجب أن يكون فريداً", type: "{{field}} يجب أن يكون {{type}} صالحاً", }, @@ -729,6 +730,7 @@ const ar = { viewHistory: "عرض السجل", delete: "حذف", moreActions: "المزيد من الإجراءات", + addReaction: "إضافة تفاعل", addToFavorites: "إضافة إلى المفضلة", removeFromFavorites: "إزالة من المفضلة", previousRecord: "السجل السابق", diff --git a/packages/i18n/src/locales/de.ts b/packages/i18n/src/locales/de.ts index ef6feb1bfe..01c1eb8cd9 100644 --- a/packages/i18n/src/locales/de.ts +++ b/packages/i18n/src/locales/de.ts @@ -103,6 +103,7 @@ const de = { url: "Bitte geben Sie eine gültige URL ein", pattern: "{{field}} hat ein ungültiges Format", formInvalid: "Bitte überprüfen Sie die markierten Felder: {{fields}}", + formInvalidJoiner: ", ", unique: "{{field}} muss eindeutig sein", type: "{{field}} muss ein gültiger {{type}} sein", }, @@ -727,6 +728,7 @@ const de = { viewHistory: "Verlauf anzeigen", delete: "Löschen", moreActions: "Weitere Aktionen", + addReaction: "Reaktion hinzufügen", addToFavorites: "Zu Favoriten hinzufügen", removeFromFavorites: "Aus Favoriten entfernen", previousRecord: "Vorheriger Datensatz", diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index cc3e48d531..2b16e5ce1e 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -109,6 +109,12 @@ const en = { url: 'Please enter a valid URL', pattern: '{{field}} format is invalid', formInvalid: 'Please check the highlighted fields: {{fields}}', + // Separator between the field names interpolated into `formInvalid`. + // Per-locale because list punctuation is a locale property, not a code + // constant: CJK enumerates with U+3001, Latin scripts with a comma+space, + // Arabic with U+060C. Hardcoding one of them in the renderer put the CJK + // comma into the English toast (objectstack#5407). + formInvalidJoiner: ', ', unique: '{{field}} must be unique', type: '{{field}} must be a valid {{type}}', }, @@ -771,6 +777,7 @@ const en = { viewHistory: 'View history', delete: 'Delete', moreActions: 'More actions', + addReaction: 'Add reaction', addToFavorites: 'Add to favorites', removeFromFavorites: 'Remove from favorites', previousRecord: 'Previous record', diff --git a/packages/i18n/src/locales/es.ts b/packages/i18n/src/locales/es.ts index 57e16f8e3e..232037ef00 100644 --- a/packages/i18n/src/locales/es.ts +++ b/packages/i18n/src/locales/es.ts @@ -94,7 +94,12 @@ const es = { }, }, validation: { - required: "{{field}} es obligatorio", + // "{{field}} es obligatorio" only agreed with masculine field labels — + // "Cuenta es obligatorio" is wrong (objectstack#5407). Spanish adjectives + // agree with the head noun, which a runtime label cannot declare, so the + // template supplies its own masculine head ("el campo") and the adjective + // agrees with THAT for every label. + required: "El campo {{field}} es obligatorio", minLength: "{{field}} debe tener al menos {{min}} caracteres", maxLength: "{{field}} debe tener como máximo {{max}} caracteres", min: "{{field}} debe ser al menos {{min}}", @@ -103,7 +108,8 @@ const es = { url: "Por favor, introduzca una URL válida", pattern: "El formato de {{field}} no es válido", formInvalid: "Por favor, revise los campos resaltados: {{fields}}", - unique: "{{field}} debe ser único", + formInvalidJoiner: ", ", + unique: "El campo {{field}} debe ser único", type: "{{field}} debe ser un {{type}} válido", }, form: { @@ -727,6 +733,7 @@ const es = { viewHistory: "Ver historial", delete: "Eliminar", moreActions: "Más acciones", + addReaction: "Añadir reacción", addToFavorites: "Añadir a favoritos", removeFromFavorites: "Quitar de favoritos", previousRecord: "Registro anterior", diff --git a/packages/i18n/src/locales/fr.ts b/packages/i18n/src/locales/fr.ts index 0bb66e6155..c4b80e027e 100644 --- a/packages/i18n/src/locales/fr.ts +++ b/packages/i18n/src/locales/fr.ts @@ -103,6 +103,7 @@ const fr = { url: "Veuillez saisir une URL valide", pattern: "Le format de {{field}} est invalide", formInvalid: "Veuillez vérifier les champs en surbrillance : {{fields}}", + formInvalidJoiner: ", ", unique: "{{field}} doit être unique", type: "{{field}} doit être un {{type}} valide", }, @@ -729,6 +730,7 @@ const fr = { viewHistory: "Voir l'historique", delete: "Supprimer", moreActions: "Plus d'actions", + addReaction: "Ajouter une réaction", addToFavorites: "Ajouter aux favoris", removeFromFavorites: "Retirer des favoris", previousRecord: "Enregistrement précédent", diff --git a/packages/i18n/src/locales/ja.ts b/packages/i18n/src/locales/ja.ts index f409225f0e..035d54ae22 100644 --- a/packages/i18n/src/locales/ja.ts +++ b/packages/i18n/src/locales/ja.ts @@ -103,6 +103,7 @@ const ja = { url: "有効なURLを入力してください", pattern: "{{field}}の形式が正しくありません", formInvalid: "ハイライトされた項目を確認してください: {{fields}}", + formInvalidJoiner: "、", unique: "{{field}}は一意である必要があります", type: "{{field}}は有効な{{type}}である必要があります", }, @@ -727,6 +728,7 @@ const ja = { viewHistory: "履歴を表示", delete: "削除", moreActions: "その他の操作", + addReaction: "リアクションを追加", addToFavorites: "お気に入りに追加", removeFromFavorites: "お気に入りから削除", previousRecord: "前のレコード", diff --git a/packages/i18n/src/locales/ko.ts b/packages/i18n/src/locales/ko.ts index 333ff3757b..e74aa2c4eb 100644 --- a/packages/i18n/src/locales/ko.ts +++ b/packages/i18n/src/locales/ko.ts @@ -103,6 +103,7 @@ const ko = { url: "유효한 URL을 입력해주세요", pattern: "{{field}} 형식이 올바르지 않습니다", formInvalid: "표시된 필드를 확인해 주세요: {{fields}}", + formInvalidJoiner: ", ", unique: "{{field}}은(는) 고유해야 합니다", type: "{{field}}은(는) 유효한 {{type}}이어야 합니다", }, @@ -727,6 +728,7 @@ const ko = { viewHistory: "기록 보기", delete: "삭제", moreActions: "더 많은 작업", + addReaction: "반응 추가", addToFavorites: "즐겨찾기에 추가", removeFromFavorites: "즐겨찾기에서 제거", previousRecord: "이전 레코드", diff --git a/packages/i18n/src/locales/pt.ts b/packages/i18n/src/locales/pt.ts index e20c4d4472..cf63fe8836 100644 --- a/packages/i18n/src/locales/pt.ts +++ b/packages/i18n/src/locales/pt.ts @@ -103,6 +103,7 @@ const pt = { url: "Por favor, insira uma URL válida", pattern: "O formato de {{field}} é inválido", formInvalid: "Verifique os campos destacados: {{fields}}", + formInvalidJoiner: ", ", unique: "{{field}} deve ser único", type: "{{field}} deve ser um {{type}} válido", }, @@ -729,6 +730,7 @@ const pt = { viewHistory: "Ver histórico", delete: "Excluir", moreActions: "Mais ações", + addReaction: "Adicionar reação", addToFavorites: "Adicionar aos favoritos", removeFromFavorites: "Remover dos favoritos", previousRecord: "Registro anterior", diff --git a/packages/i18n/src/locales/ru.ts b/packages/i18n/src/locales/ru.ts index 123de7970a..559fe913c7 100644 --- a/packages/i18n/src/locales/ru.ts +++ b/packages/i18n/src/locales/ru.ts @@ -103,6 +103,7 @@ const ru = { url: "Пожалуйста, введите корректный URL", pattern: "Неверный формат поля {{field}}", formInvalid: "Проверьте выделенные поля: {{fields}}", + formInvalidJoiner: ", ", unique: "{{field}} должно быть уникальным", type: "{{field}} должно быть допустимым {{type}}", }, @@ -729,6 +730,7 @@ const ru = { viewHistory: "Просмотр истории", delete: "Удалить", moreActions: "Другие действия", + addReaction: "Добавить реакцию", addToFavorites: "Добавить в избранное", removeFromFavorites: "Удалить из избранного", previousRecord: "Предыдущая запись", diff --git a/packages/i18n/src/locales/zh.ts b/packages/i18n/src/locales/zh.ts index 712b34c68e..c325a83ad2 100644 --- a/packages/i18n/src/locales/zh.ts +++ b/packages/i18n/src/locales/zh.ts @@ -108,6 +108,7 @@ const zh = { url: '请输入有效的URL', pattern: '{{field}}格式不正确', formInvalid: '请检查表单中标记的字段:{{fields}}', + formInvalidJoiner: '、', unique: '{{field}}必须唯一', type: '{{field}}必须是有效的{{type}}', }, @@ -777,6 +778,7 @@ const zh = { viewHistory: '查看历史', delete: '删除', moreActions: '更多操作', + addReaction: '添加表情回应', addToFavorites: '添加到收藏', removeFromFavorites: '从收藏中移除', previousRecord: '上一条记录', diff --git a/packages/plugin-detail/src/ReactionPicker.i18n.test.tsx b/packages/plugin-detail/src/ReactionPicker.i18n.test.tsx new file mode 100644 index 0000000000..9941a4bd74 --- /dev/null +++ b/packages/plugin-detail/src/ReactionPicker.i18n.test.tsx @@ -0,0 +1,65 @@ +/** + * 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 activity-feed reaction button speaks the session locale — + * objectstack#5407. + * + * `ReactionPicker`'s add button is icon-only (`SmilePlus`), so its + * `aria-label` IS the button as far as a screen reader or a hover tooltip is + * concerned. It was the English literal "Add reaction" in every locale; it now + * reads `detail.addReaction`, backfilled into all ten packs. + * + * The no-provider case is asserted alongside the translated one because + * `useDetailTranslation` is a `createSafeTranslation` hook: its whole point is + * that a component rendered without an `I18nProvider` gets the English default + * rather than the raw key, and "aria-label reads `detail.addReaction`" is a + * failure mode that no positive assertion under a provider would catch. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import { I18nProvider } from '@object-ui/i18n'; +import { ReactionPicker } from './ReactionPicker'; + +afterEach(() => cleanup()); + +function renderPickerIn(language: string) { + return render( + + + , + ); +} + +describe('ReactionPicker add button — accessible name (objectstack#5407)', () => { + it('reads the zh bundle value under a zh session', () => { + renderPickerIn('zh'); + + expect(screen.getByRole('button', { name: '添加表情回应' })).toBeTruthy(); + expect(screen.queryByRole('button', { name: 'Add reaction' })).toBeNull(); + }); + + it('reads the es bundle value under an es session', () => { + renderPickerIn('es'); + + expect(screen.getByRole('button', { name: 'Añadir reacción' })).toBeTruthy(); + }); + + it('still reads English under an en session', () => { + renderPickerIn('en'); + + expect(screen.getByRole('button', { name: 'Add reaction' })).toBeTruthy(); + }); + + it('never renders the raw key', () => { + renderPickerIn('en'); + + expect(screen.queryByRole('button', { name: 'detail.addReaction' })).toBeNull(); + }); +}); diff --git a/packages/plugin-detail/src/ReactionPicker.tsx b/packages/plugin-detail/src/ReactionPicker.tsx index 36da80c577..d089087ff4 100644 --- a/packages/plugin-detail/src/ReactionPicker.tsx +++ b/packages/plugin-detail/src/ReactionPicker.tsx @@ -10,6 +10,7 @@ import * as React from 'react'; import { cn, Button } from '@object-ui/components'; import { SmilePlus } from 'lucide-react'; import type { Reaction } from '@object-ui/types'; +import { useDetailTranslation } from './useDetailTranslation'; const DEFAULT_EMOJI_OPTIONS = ['👍', '❤️', '🎉', '😂', '😮', '😢']; @@ -35,6 +36,7 @@ export const ReactionPicker: React.FC = ({ className, }) => { const [showPicker, setShowPicker] = React.useState(false); + const { t } = useDetailTranslation(); const handleReaction = React.useCallback( (emoji: string) => { @@ -74,7 +76,7 @@ export const ReactionPicker: React.FC = ({ size="icon" className="h-6 w-6" onClick={() => setShowPicker(!showPicker)} - aria-label="Add reaction" + aria-label={t('detail.addReaction')} > diff --git a/packages/plugin-detail/src/useDetailTranslation.ts b/packages/plugin-detail/src/useDetailTranslation.ts index a7ae262736..7486a2daec 100644 --- a/packages/plugin-detail/src/useDetailTranslation.ts +++ b/packages/plugin-detail/src/useDetailTranslation.ts @@ -43,6 +43,10 @@ export const DETAIL_DEFAULT_TRANSLATIONS: Record = { 'detail.viewHistory': 'View history', 'detail.delete': 'Delete', 'detail.moreActions': 'More actions', + // objectstack#5407 — the activity-feed reaction button's accessible name. + // It is icon-only, so this label IS the button as far as a screen reader + // (or a hover tooltip) is concerned; it used to be an English literal. + 'detail.addReaction': 'Add reaction', 'detail.addToFavorites': 'Add to favorites', 'detail.removeFromFavorites': 'Remove from favorites', 'detail.previousRecord': 'Previous record',