Skip to content

Commit eee4ded

Browse files
os-zhuangclaude
andauthored
feat(fields): select+multiple → multi-value chip picker; restore fields/core lint gates (#2709)
* feat(fields): render select+multiple through the multi-value chip picker A `select` field/param declared `multiple: true` is a multi-value selector (spec allows `multiple` on select), but it rendered a single-value dropdown that could hold only one value. SelectField now delegates to the multi-value chip picker (the multiselect widget) when `multiple` is set, storing a string[]. Delegating inside SelectField — rather than at a type-resolution layer — means every surface that renders the `select` widget inherits multi-select identically: the object form, the inline grid editor, and the app-shell ActionParamDialog. No per-surface drift, no divergence from the form (which was the risk of special-casing it only in the param adapter). Single selects keep the cascading dropdown; multi + per-option visibleWhen cascading is not a combination in use today. Tests: SelectField renders chip toggles (not a combobox) when multiple, toggles values as an array, and renders badges in readonly mode. Docs: fields README "Multi-value selects". Changeset: @object-ui/fields minor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg7oavKSUdb2KzYEEYrCHX * fix(fields,core): clear baseline lint errors to restore package lint gates The @object-ui/fields and @object-ui/core package lints were red at baseline, so the gate could not catch NEW violations. Cleared all errors (behavior unchanged): fields (react-hooks/rules-of-hooks — hooks were called after early returns): - ImageField: hoist the two useCallback handlers (+ their `images` dep) above the readonly early return. - TextAreaField: hoist the two useState calls above the readonly early return. - index.tsx: useFieldTranslate no longer wraps useObjectTranslation in try/catch (the hook is provider-safe and never throws; try/catch around a hook is the violation). Mirrors useFieldLabel. core: - ExpressionEvaluator: preserve-caught-error — the package targets ES2020 whose lib types only the 1-arg Error constructor, so `{ cause }` won't compile; the original message is inlined and the rule is disabled with that justification. - SafeExpressionParser: drop the useless `op` null-initializer (every branch assigns or breaks); drop the useless `\-` escape in a char class. - export-filename: drop the useless `\/` escape; the intentional control-char range keeps a justified no-control-regex disable. - record-title: `let display` -> `const` (never reassigned). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg7oavKSUdb2KzYEEYrCHX --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3b2e4d9 commit eee4ded

11 files changed

Lines changed: 170 additions & 54 deletions
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
"@object-ui/fields": minor
3+
"@object-ui/core": patch
4+
---
5+
6+
feat(fields): render `select` + `multiple` through the multi-value chip picker; restore fields/core lint gates
7+
8+
- **Multi-value select** — a `select` field/param declared `multiple: true`
9+
now renders the multi-value chip picker (the `multiselect` widget) and stores
10+
a `string[]`, instead of collapsing to a single-value dropdown that could
11+
hold only one value. The delegation lives inside `SelectField`, so the object
12+
form, the inline grid editor, and the app-shell `ActionParamDialog` all
13+
inherit it from the one `select` widget with no per-surface drift. Single
14+
selects keep the cascading dropdown (multi + per-option `visibleWhen`
15+
cascading is not a combination in use today).
16+
- **`autonumber` mapping is unchanged** here; this change is orthogonal.
17+
- **Lint gates restored** — fixed the pre-existing baseline lint errors that
18+
had left the `@object-ui/fields` and `@object-ui/core` package lints red (so
19+
the gate could not catch new violations): `react-hooks/rules-of-hooks` in
20+
`ImageField` / `TextAreaField` / `index.tsx` (hooks hoisted above early
21+
returns; the `useFieldTranslate` hook no longer wrapped in try/catch), plus
22+
`no-useless-assignment` / `no-useless-escape` / `no-control-regex` /
23+
`prefer-const` / `preserve-caught-error` in the core evaluator and utils. No
24+
behavior change from the lint fixes.

packages/core/src/evaluator/ExpressionEvaluator.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,10 @@ export class ExpressionEvaluator {
169169
// Execute with context values
170170
return compiled.fn(...varValues);
171171
} catch (error) {
172+
// The original error's message is inlined below. We can't pass it as the
173+
// `Error` `cause` option because this package targets ES2020, whose lib
174+
// types the 1-arg `Error` constructor only; hence the scoped disable.
175+
// eslint-disable-next-line preserve-caught-error
172176
throw new Error(`Failed to evaluate expression "${expression}": ${(error as Error).message}`);
173177
}
174178
}

packages/core/src/evaluator/SafeExpressionParser.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,10 @@ export class SafeExpressionParser {
313313

314314
// eslint-disable-next-line no-constant-condition
315315
while (true) {
316-
let op: string | null = null;
316+
// Every branch below either assigns `op` or `break`s out of the loop, so
317+
// `op` is always set by the time the switch reads it (no null initializer
318+
// needed).
319+
let op: string;
317320

318321
if (this.peek() === '=' && this.peek(1) === '=' && this.peek(2) === '=') {
319322
op = '==='; this.pos += 3;
@@ -659,7 +662,7 @@ export class SafeExpressionParser {
659662
// Optional exponent: e+5, E-3
660663
if (/[eE]/.test(this.source[this.pos] ?? '')) {
661664
this.pos++; // consume 'e' or 'E'
662-
if (/[+\-]/.test(this.source[this.pos] ?? '')) this.pos++; // optional sign
665+
if (/[+-]/.test(this.source[this.pos] ?? '')) this.pos++; // optional sign
663666

664667
let expDigits = 0;
665668
while (this.pos < this.source.length && /\d/.test(this.source[this.pos])) {

packages/core/src/utils/export-filename.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99
*/
1010

1111
/** Windows-reserved + control characters that cannot appear in a filename. */
12-
const ILLEGAL_FILENAME_CHARS = /[\\\/:*?"<>|\u0000-\u001f]+/g;
12+
// The control-char range in the class is matched deliberately (illegal in
13+
// filenames, must be stripped), so no-control-regex is intentionally disabled.
14+
// eslint-disable-next-line no-control-regex
15+
const ILLEGAL_FILENAME_CHARS = /[\\/:*?"<>|\u0000-\u001f]+/g;
1316

1417
/** Longest base we emit — keeps the full name comfortably under OS limits. */
1518
const MAX_BASE_LENGTH = 80;
2 Bytes
Binary file not shown.

packages/fields/README.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,18 @@ or photo per row without opening the row form (objectui#2360). Columns accept
8383
exported as `FileCell`). Auto-derived subform columns map `file`/`image`/
8484
`avatar` fields to file columns instead of dropping them.
8585

86+
### Multi-value selects
87+
88+
A `select` field declared `multiple: true` selects zero-or-more values (spec
89+
allows `multiple` on `select`). `SelectField` delegates to the multi-value chip
90+
picker (the same widget the `multiselect` type uses) and stores a `string[]`.
91+
Delegating inside `SelectField` — rather than at a type-resolution layer — means
92+
every surface that renders the `select` widget (the object form, the inline grid
93+
editor, and the app-shell `ActionParamDialog`) gets multi-select identically,
94+
with no per-surface drift. Single-value selects keep the cascading dropdown
95+
below. (A multi-value select forgoes per-option `visibleWhen` cascading, which
96+
the chip picker does not implement.)
97+
8698
### Cascading & role-gated select options
8799

88100
`select` options support a per-option `visibleWhen` CEL predicate (offered only

packages/fields/src/index.tsx

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -277,12 +277,11 @@ function useFieldLabel() {
277277
* fallback in that case.
278278
*/
279279
function useFieldTranslate(): ((key: string, params?: Record<string, unknown>) => string) | undefined {
280-
try {
281-
const { t } = useObjectTranslation();
282-
return t as (key: string, params?: Record<string, unknown>) => string;
283-
} catch {
284-
return undefined;
285-
}
280+
// useObjectTranslation is provider-safe (optional context read; react-i18next
281+
// falls back to the global instance), so it never throws — no try/catch, which
282+
// would wrap a hook call and violate rules-of-hooks. Mirrors useFieldLabel.
283+
const { t } = useObjectTranslation();
284+
return t as (key: string, params?: Record<string, unknown>) => string;
286285
}
287286

288287
import { TextField } from './widgets/TextField';

packages/fields/src/standard-widgets.test.tsx

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,50 @@ describe('Standard Field Widgets', () => {
213213
);
214214
expect(container.textContent).toBe('Option A');
215215
});
216+
217+
// A `select` declared `multiple` is a multi-value picker (spec allows
218+
// `multiple` on select). SelectField delegates to the chip picker so the
219+
// form, inline editor, and ActionParamDialog all get multi-select from the
220+
// one `select` widget — no single-select fallback, no per-surface drift.
221+
it('renders the multi-value chip picker when field.multiple is set', () => {
222+
render(
223+
<SelectField
224+
value={['a']}
225+
onChange={mockOnChange}
226+
field={{ ...fieldMock, multiple: true } as any}
227+
/>
228+
);
229+
// Chip picker uses toggle buttons (aria-pressed), not a Radix combobox.
230+
expect(screen.queryByRole('combobox')).not.toBeInTheDocument();
231+
const optionA = screen.getByRole('button', { name: 'Option A' });
232+
expect(optionA).toHaveAttribute('aria-pressed', 'true');
233+
expect(screen.getByRole('button', { name: 'Option B' })).toHaveAttribute('aria-pressed', 'false');
234+
});
235+
236+
it('multi-select toggles values as an array', () => {
237+
render(
238+
<SelectField
239+
value={['a']}
240+
onChange={mockOnChange}
241+
field={{ ...fieldMock, multiple: true } as any}
242+
/>
243+
);
244+
fireEvent.click(screen.getByRole('button', { name: 'Option B' }));
245+
expect(mockOnChange).toHaveBeenCalledWith(['a', 'b']);
246+
});
247+
248+
it('multi-select renders selected values as badges in readonly mode', () => {
249+
const { container } = render(
250+
<SelectField
251+
value={['a', 'b']}
252+
onChange={mockOnChange}
253+
field={{ ...fieldMock, multiple: true } as any}
254+
readonly={true}
255+
/>
256+
);
257+
expect(container.textContent).toContain('Option A');
258+
expect(container.textContent).toContain('Option B');
259+
});
216260
});
217261

218262
describe('BooleanField', () => {

packages/fields/src/widgets/ImageField.tsx

Lines changed: 42 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,50 @@ export function ImageField({ value, onChange, field, readonly, onUploadingChange
2929
const [uploading, setUploading] = useState(false);
3030
useUploadingSignal(uploading, onUploadingChange);
3131

32+
// Derived value + memoized handlers must run before the readonly early return
33+
// so hook order stays stable across renders.
34+
const images = value ? (Array.isArray(value) ? value : [value]) : [];
35+
36+
const handleCropConfirm = useCallback(
37+
async (blob: Blob, name: string) => {
38+
if (!cropTarget) return;
39+
setUploading(true);
40+
try {
41+
const result = await upload(blob);
42+
const next = {
43+
name: result.name || name,
44+
original_name: name,
45+
size: result.size,
46+
mime_type: result.mimeType,
47+
url: result.url,
48+
};
49+
if (multiple) {
50+
const updated = [...images];
51+
updated[cropTarget.index] = next;
52+
onChange(updated);
53+
} else {
54+
onChange(next);
55+
}
56+
} finally {
57+
setUploading(false);
58+
setCropTarget(null);
59+
}
60+
},
61+
[cropTarget, images, multiple, onChange, upload],
62+
);
63+
64+
const openCropper = useCallback(
65+
(index: number) => {
66+
const img = images[index];
67+
if (!img?.url) return;
68+
setCropTarget({ index, src: img.url, name: img.name || `image-${index}.png` });
69+
},
70+
[images],
71+
);
72+
3273
if (readonly) {
3374
if (!value) return <EmptyValue />;
34-
35-
const images = Array.isArray(value) ? value : [value];
75+
3676
return (
3777
<div className="flex flex-wrap gap-2">
3878
{images.map((img: any, idx: number) => (
@@ -47,8 +87,6 @@ export function ImageField({ value, onChange, field, readonly, onUploadingChange
4787
);
4888
}
4989

50-
const images = value ? (Array.isArray(value) ? value : [value]) : [];
51-
5290
const handleFileChange = async (e: React.ChangeEvent<HTMLInputElement>) => {
5391
const selectedFiles = Array.from(e.target.files || []);
5492
if (selectedFiles.length === 0) return;
@@ -89,43 +127,6 @@ export function ImageField({ value, onChange, field, readonly, onUploadingChange
89127
}
90128
};
91129

92-
const handleCropConfirm = useCallback(
93-
async (blob: Blob, name: string) => {
94-
if (!cropTarget) return;
95-
setUploading(true);
96-
try {
97-
const result = await upload(blob);
98-
const next = {
99-
name: result.name || name,
100-
original_name: name,
101-
size: result.size,
102-
mime_type: result.mimeType,
103-
url: result.url,
104-
};
105-
if (multiple) {
106-
const updated = [...images];
107-
updated[cropTarget.index] = next;
108-
onChange(updated);
109-
} else {
110-
onChange(next);
111-
}
112-
} finally {
113-
setUploading(false);
114-
setCropTarget(null);
115-
}
116-
},
117-
[cropTarget, images, multiple, onChange, upload],
118-
);
119-
120-
const openCropper = useCallback(
121-
(index: number) => {
122-
const img = images[index];
123-
if (!img?.url) return;
124-
setCropTarget({ index, src: img.url, name: img.name || `image-${index}.png` });
125-
},
126-
[images],
127-
);
128-
129130
return (
130131
<div className={props.className}>
131132
<input

packages/fields/src/widgets/SelectField.tsx

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,33 @@ import { SchemaRendererContext, usePredicateScope } from '@object-ui/react';
1717
import { SelectFieldMetadata } from '@object-ui/types';
1818
import { useFieldTranslation } from './useFieldTranslation';
1919
import { FieldWidgetProps } from './types';
20+
import { MultiSelectField } from './MultiSelectField';
2021

2122
/**
22-
* SelectField - Dropdown selection widget with configurable options.
23+
* SelectField - dropdown selection widget.
24+
*
25+
* A field declared `multiple: true` selects zero-or-more values (spec:
26+
* `multiple` is valid on `select`), so it renders the multi-value chip picker
27+
* — the same widget the `multiselect` type uses. Delegating here (rather than
28+
* only at a type-resolution layer) means every surface that renders the
29+
* `select` widget — the object form, the inline grid editor, and
30+
* `ActionParamDialog` — inherits multi-select identically, with no drift
31+
* between them. Single-value selects keep the cascading dropdown below.
32+
*
33+
* (A `multiple` select forgoes per-option `visibleWhen` cascading, which the
34+
* chip picker does not implement; single selects retain it. Multi-select +
35+
* cascading is not a combination in use today.)
36+
*/
37+
export function SelectField(props: FieldWidgetProps<any>) {
38+
const config = (props.field || (props as any).schema) as SelectFieldMetadata | undefined;
39+
if ((config as any)?.multiple) {
40+
return <MultiSelectField {...props} />;
41+
}
42+
return <SingleSelectField {...(props as FieldWidgetProps<string>)} />;
43+
}
44+
45+
/**
46+
* SingleSelectField - single-value dropdown with configurable options.
2347
*
2448
* Supports cascading / role-gated options (#2284): each option may carry a
2549
* `visibleWhen` CEL predicate, evaluated against the live form record +
@@ -29,7 +53,7 @@ import { FieldWidgetProps } from './types';
2953
* of those is empty the control is gated with a "select the parent first" hint,
3054
* mirroring the dependent-lookup UX.
3155
*/
32-
export function SelectField({
56+
function SingleSelectField({
3357
value,
3458
onChange,
3559
field,

0 commit comments

Comments
 (0)