EnxDev commented on PR #41194:
URL: https://github.com/apache/superset/pull/41194#issuecomment-4809517366
## EnxDev's Review Agent — apache/superset#41194 · HEAD 68e9c45
**lgtm** — one-line config fix is correct and effective; `allowNewOptions:
true` is exactly the prop the core `Select` gates free-text entry on, mirroring
`d3NumberFormat`.
Verified end-to-end:
- `ControlFormItem` spreads the spec straight onto
`@superset-ui/core/components` `Select` (`<Control {...props} … />`), and that
Select reads `allowNewOptions` (`Select.tsx:93,393,665,700`) to enable option
creation. So the new flag flows through and works.
- The pre-existing `creatable: true` is **not** a prop of the core Select
(only `allowNewOptions` is) — the PR's claim that `creatable` is inert here is
accurate.
- CI is fully green; title follows Conventional Commits (`fix(explore): …`).
### 🟡 Should-fix
- **`ColumnConfigConstants.test.tsx`** — no regression guard for the new
behavior. Add a one-line
`expect(SHARED_COLUMN_CONFIG_PROPS.d3TimeFormat.allowNewOptions).toBe(true)`
next to the existing `d3NumberFormat` tests? Codecov marks the changed line
"covered" only because the module is imported, not because the behavior is
asserted — a silent revert of this flag would pass CI. (Copilot and Bito
flagged the same.)
### 🔵 Nits
- `constants.tsx:73` — `creatable: true` on `d3TimeFormat` (and `:58` on
`d3NumberFormat`) is dead config; the core Select has no `creatable` prop.
Worth dropping while you're here, though it's pre-existing so fine to leave for
a separate cleanup.
- `constants.tsx:65` — cosmetic: `d3NumberFormat` lists `allowNewOptions` as
the first key (`:49`); here it's third. Aligning the order would keep the two
specs visually parallel.
<!-- enxdev-review-agent:68e9c45 -->
_Reviewed by EnxDev's Review Agent — @EnxDev · HEAD 68e9c45._
--
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]
