diff --git a/.changeset/address-geo-unique-ids.md b/.changeset/address-geo-unique-ids.md new file mode 100644 index 000000000..e0e4d3b59 --- /dev/null +++ b/.changeset/address-geo-unique-ids.md @@ -0,0 +1,5 @@ +--- +'@object-ui/fields': patch +--- + +AddressField / GeolocationField sub-inputs now derive their DOM ids from a `useId()` prefix + sub-field name (the RadioField / CheckboxesField `groupId` paradigm) instead of hardcoded literals ("street", "city", "state", "zipCode", "country", "latitude", "longitude"). Two address or geolocation fields in one form no longer produce duplicate DOM ids, and each sub-label's `htmlFor` resolves to and focuses its own field's input instead of the first match in the document (#3343). diff --git a/packages/fields/src/widgets/AddressField.tsx b/packages/fields/src/widgets/AddressField.tsx index 830ee3db3..6df896be7 100644 --- a/packages/fields/src/widgets/AddressField.tsx +++ b/packages/fields/src/widgets/AddressField.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useId } from 'react'; import { Input, Label, EmptyValue } from '@object-ui/components'; import { FieldWidgetComponentProps } from './types'; import { toDomProps } from './toDomProps'; @@ -26,6 +26,12 @@ export function AddressField({ value, onChange, field, readonly, error, ...props // `aria-invalid={!!error}` (a form-level failure means the whole address is // missing/invalid, and whichever sub-input the user reaches must announce it). const domProps = toDomProps(props); + // Sub-input ids (objectui#3343): `useId()` prefix + sub-field name — the + // `groupId` paradigm of RadioField / CheckboxesField. Hardcoded literals + // ("street", "city", …) collide as soon as a form renders two address + // fields, and every label's htmlFor then resolves to the FIRST match. + const groupId = useId(); + const subId = (name: keyof AddressValue) => `${groupId}-${name}`; const handleFieldChange = (fieldName: keyof AddressValue, fieldValue: string) => { onChange({ @@ -52,10 +58,10 @@ export function AddressField({ value, onChange, field, readonly, error, ...props return (
- + handleFieldChange('street', e.target.value)} @@ -68,9 +74,9 @@ export function AddressField({ value, onChange, field, readonly, error, ...props
- + handleFieldChange('city', e.target.value)} @@ -81,9 +87,9 @@ export function AddressField({ value, onChange, field, readonly, error, ...props
- + handleFieldChange('state', e.target.value)} @@ -96,9 +102,9 @@ export function AddressField({ value, onChange, field, readonly, error, ...props
- + handleFieldChange('zipCode', e.target.value)} @@ -109,9 +115,9 @@ export function AddressField({ value, onChange, field, readonly, error, ...props
- + handleFieldChange('country', e.target.value)} diff --git a/packages/fields/src/widgets/AddressField.uniqueIds.test.tsx b/packages/fields/src/widgets/AddressField.uniqueIds.test.tsx new file mode 100644 index 000000000..be55fed4f --- /dev/null +++ b/packages/fields/src/widgets/AddressField.uniqueIds.test.tsx @@ -0,0 +1,92 @@ +/** + * 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. + */ + +/** + * Unique sub-input ids for the composite `address` widget (objectui#3343). + * + * The widget used to hardcode literal ids ("street", "city", "state", + * "zipCode", "country") on its sub-inputs. Two address fields in one form — + * e.g. billing + shipping — then produced duplicate DOM ids, and every + * `Label htmlFor` resolved to the FIRST match in the document: each sub-label + * of the second field clicked/announced the first field's input. + * + * The fix follows the `groupId` paradigm of RadioField / CheckboxesField: + * a `useId()` prefix + the sub-field name. These tests pin it by rendering + * TWO instances in one form: + * + * 1. every `[id]` in the document is globally unique (red again if anyone + * reverts to hardcoded literals — both instances would emit id="street"); + * 2. each sub-label of the SECOND instance resolves — via the exact + * mechanism browsers use for label activation, `for` → getElementById — + * to an input inside its OWN instance, and focusing it lands there. + */ +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, within } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { AddressField } from './AddressField'; + +const field = { name: 'address', type: 'address' } as any; + +const SUB_LABELS = [ + 'Street Address', + 'City', + 'State / Province', + 'ZIP / Postal Code', + 'Country', +]; + +function renderTwoInOneForm() { + return render( +
+
+ +
+
+ +
+
, + ); +} + +describe('AddressField — unique sub-input ids (objectui#3343)', () => { + it('two address fields in one form produce globally unique DOM ids', () => { + const { baseElement } = renderTwoInOneForm(); + const ids = Array.from(baseElement.querySelectorAll('[id]')).map((el) => el.id); + // 5 sub-inputs per instance — both instances must actually render ids. + expect(ids.length).toBeGreaterThanOrEqual(10); + const duplicates = ids.filter((id, i) => ids.indexOf(id) !== i); + expect(duplicates).toEqual([]); + }); + + it("each sub-label of the SECOND field focuses that field's own input", () => { + const { getByTestId } = renderTwoInOneForm(); + const first = getByTestId('address-a'); + const second = getByTestId('address-b'); + + for (const text of SUB_LABELS) { + const label = within(second).getByText(text); + const forId = label.getAttribute('for'); + expect(forId, `label "${text}" must carry htmlFor`).toBeTruthy(); + + // Exactly what the browser does on label click / SR announcement: + // resolve `for` against the document. + const control = document.getElementById(forId!); + expect(control, `htmlFor of "${text}" must resolve`).not.toBeNull(); + expect(control!.tagName).toBe('INPUT'); + expect( + second.contains(control), + `"${text}" must resolve into its OWN field, not the first one`, + ).toBe(true); + expect(first.contains(control)).toBe(false); + + (control as HTMLInputElement).focus(); + expect(control).toHaveFocus(); + } + }); +}); diff --git a/packages/fields/src/widgets/GeolocationField.tsx b/packages/fields/src/widgets/GeolocationField.tsx index c17eab00a..1e778b23a 100644 --- a/packages/fields/src/widgets/GeolocationField.tsx +++ b/packages/fields/src/widgets/GeolocationField.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useId } from 'react'; import { Input, Button, Label, EmptyValue } from '@object-ui/components'; import { MapPin, Crosshair } from 'lucide-react'; import { FieldWidgetComponentProps } from './types'; @@ -24,6 +24,13 @@ export function GeolocationField({ value, onChange, field, readonly, error, ...p // sub-input (latitude); the composite's validation state goes onto BOTH // focusable sub-inputs via `aria-invalid={!!error}`. const domProps = toDomProps(props); + // Sub-input ids (objectui#3343): `useId()` prefix + sub-field name — the + // `groupId` paradigm of RadioField / CheckboxesField. Hardcoded literals + // ("latitude" / "longitude") collide as soon as a form renders two + // geolocation fields, and every label's htmlFor then resolves to the + // FIRST match. + const groupId = useId(); + const subId = (name: keyof GeolocationValue) => `${groupId}-${name}`; const handleFieldChange = (fieldName: keyof GeolocationValue, fieldValue: string) => { onChange({ @@ -120,10 +127,10 @@ export function GeolocationField({ value, onChange, field, readonly, error, ...p
- + handleFieldChange('latitude', e.target.value)} @@ -136,9 +143,9 @@ export function GeolocationField({ value, onChange, field, readonly, error, ...p
- + handleFieldChange('longitude', e.target.value)} diff --git a/packages/fields/src/widgets/GeolocationField.uniqueIds.test.tsx b/packages/fields/src/widgets/GeolocationField.uniqueIds.test.tsx new file mode 100644 index 000000000..44b90e753 --- /dev/null +++ b/packages/fields/src/widgets/GeolocationField.uniqueIds.test.tsx @@ -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. + */ + +/** + * Unique sub-input ids for the composite `geolocation` widget + * (objectui#3343). + * + * The widget used to hardcode literal ids ("latitude" / "longitude") on its + * sub-inputs. Two geolocation fields in one form — e.g. origin + destination + * — then produced duplicate DOM ids, and every `Label htmlFor` resolved to + * the FIRST match in the document: each sub-label of the second field + * clicked/announced the first field's input. + * + * The fix follows the `groupId` paradigm of RadioField / CheckboxesField: + * a `useId()` prefix + the sub-field name. These tests pin it by rendering + * TWO instances in one form: + * + * 1. every `[id]` in the document is globally unique (red again if anyone + * reverts to hardcoded literals — both instances would emit + * id="latitude"); + * 2. each sub-label of the SECOND instance resolves — via the exact + * mechanism browsers use for label activation, `for` → getElementById — + * to an input inside its OWN instance, and focusing it lands there. + */ +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, within } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { GeolocationField } from './GeolocationField'; + +const field = { name: 'location', type: 'geolocation' } as any; + +const SUB_LABELS = ['Latitude', 'Longitude']; + +function renderTwoInOneForm() { + return render( +
+
+ +
+
+ +
+
, + ); +} + +describe('GeolocationField — unique sub-input ids (objectui#3343)', () => { + it('two geolocation fields in one form produce globally unique DOM ids', () => { + const { baseElement } = renderTwoInOneForm(); + const ids = Array.from(baseElement.querySelectorAll('[id]')).map((el) => el.id); + // 2 sub-inputs per instance — both instances must actually render ids. + expect(ids.length).toBeGreaterThanOrEqual(4); + const duplicates = ids.filter((id, i) => ids.indexOf(id) !== i); + expect(duplicates).toEqual([]); + }); + + it("each sub-label of the SECOND field focuses that field's own input", () => { + const { getByTestId } = renderTwoInOneForm(); + const first = getByTestId('geo-a'); + const second = getByTestId('geo-b'); + + for (const text of SUB_LABELS) { + const label = within(second).getByText(text); + const forId = label.getAttribute('for'); + expect(forId, `label "${text}" must carry htmlFor`).toBeTruthy(); + + // Exactly what the browser does on label click / SR announcement: + // resolve `for` against the document. + const control = document.getElementById(forId!); + expect(control, `htmlFor of "${text}" must resolve`).not.toBeNull(); + expect(control!.tagName).toBe('INPUT'); + expect( + second.contains(control), + `"${text}" must resolve into its OWN field, not the first one`, + ).toBe(true); + expect(first.contains(control)).toBe(false); + + (control as HTMLInputElement).focus(); + expect(control).toHaveFocus(); + } + }); +});