bito-code-review[bot] commented on code in PR #40280:
URL: https://github.com/apache/superset/pull/40280#discussion_r3279535875
##########
superset-frontend/src/features/semanticLayers/jsonFormsHelpers.tsx:
##########
@@ -255,14 +255,98 @@ const EnumNamesRenderer =
withJsonFormsControlProps(EnumNamesControl);
const enumNamesEntry = {
// Rank 5: higher than the default string renderer (2–3) so this fires
// whenever x-enumNames is present, regardless of the underlying type.
+ // Array-of-enum schemas are handled by ``multiEnumEntry`` below — this
+ // renderer only targets scalar string/number controls.
tester: rankWith(
5,
+ and(
+ schemaMatches(s => {
+ const names = (s as Record<string, unknown>)['x-enumNames'];
+ return Array.isArray(names) && (names as unknown[]).length > 0;
+ }),
+ schemaMatches(
+ s => (s as Record<string, unknown>)?.type !== 'array',
+ ),
+ ),
+ ),
+ renderer: EnumNamesRenderer,
+};
+
+/**
+ * Renderer for ``{type: 'array', items: {enum: [...]}}`` schemas. Renders
+ * a single Antd Select with ``mode="multiple"`` (tag-style multi-select),
+ * matching the natural expectation of a "pick several from a list" control.
+ *
+ * Without this, the default ``PrimitiveArrayControl`` from the upstream
+ * library renders an "Add …" button that creates one single-select per
+ * element — visually wrong for an enum multi-select and unable to display
+ * ``items.x-enumNames`` labels.
+ *
+ * The renderer is dynamic-aware: when the host form is refreshing the
+ * schema (e.g. compatible options narrowing as the user picks), the Select
+ * shows a loading indicator without becoming disabled, so the user can
+ * continue editing while options refresh.
+ */
+function MultiEnumControl(props: ControlProps) {
+ const { refreshingSchema } = props.config ?? {};
+ const arraySchema = props.schema as Record<string, unknown>;
+ const itemsSchema =
+ (arraySchema.items as Record<string, unknown>) ??
+ ({} as Record<string, unknown>);
+
+ const enumValues = (itemsSchema.enum as unknown[]) ?? [];
+ const enumNames =
+ (itemsSchema['x-enumNames'] as string[]) ?? enumValues.map(String);
+
+ const options = enumValues.map((value, index) => ({
+ value: value as string | number,
+ label: enumNames[index] ?? String(value),
+ }));
+
+ const value = Array.isArray(props.data) ? (props.data as unknown[]) : [];
+
+ const tooltip = (props.uischema?.options as Record<string, unknown>)
+ ?.tooltip as string | undefined;
+
+ return (
+ <Form.Item label={props.label} tooltip={tooltip}>
+ <Select
+ mode="multiple"
+ value={value as (string | number)[]}
+ onChange={next => props.handleChange(props.path, next)}
+ options={options}
+ style={{ width: '100%' }}
+ disabled={!props.enabled}
+ loading={!!refreshingSchema}
+ allowClear
+ optionFilterProp="label"
+ placeholder={
+ (props.uischema?.options as Record<string, unknown>)
+ ?.placeholderText as string | undefined
+ }
+ />
+ </Form.Item>
+ );
+}
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing unit tests for MultiEnumControl</b></div>
<div id="fix">
The new MultiEnumControl renderer and multiEnumEntry tester lack test
coverage. Per BITO.md rule [6262], tests should verify actual behavior logic.
Add tests to validate the multi-select rendering, enum label mapping, and
schema matching logic.
</div>
</div>
<small><i>Code Review Run #9c60cb</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
