codeant-ai-for-open-source[bot] commented on code in PR #39068:
URL: https://github.com/apache/superset/pull/39068#discussion_r3531273494
##########
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx:
##########
@@ -719,15 +784,8 @@ const Select = forwardRef(
setSelectValue(value);
}
} else {
- const token = tokenSeparators.find(token =>
pastedText.includes(token));
- const array = token
- ? uniq(
- pastedText
- .split(token)
- .map(item => item.trim())
- .filter(Boolean),
- )
- : [pastedText.trim()].filter(Boolean);
+ e.preventDefault();
+ const array = uniq(splitWithQuoteEscaping(pastedText,
tokenSeparators));
Review Comment:
**Suggestion:** Always calling `preventDefault()` on multi-select paste
suppresses normal paste behavior even when no valid token is produced, which
can silently drop user input (for example unknown value with `allowNewOptions`
disabled). Only prevent default when the paste is actually consumed by
tokenization/selection, otherwise let the input receive the pasted text. [logic
error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Pasted text lost when no token accepted.
- ⚠️ Multi-select filter UX degraded for non-creatable configs.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Use the existing `SelectControl` wrapper (`SelectControl.tsx:320-45`)
which renders the
core `Select` with `mode` resolved to `'multiple'` when `multi` or `isMulti`
are set, and
forwards `tokenSeparators` to the underlying Select via
`tokenSeparators={tokenSeparators}` (`SelectControl.tsx:334-42`).
2. Configure a scenario where new pasted values are not allowed: for
example, in the
native filter plugin (`SelectFilterPlugin.tsx:600-46`), set `multiSelect` to
true,
`searchAllOptions` to false, and `creatable` to `false` so that
`allowNewOptions` and
`allowNewOptionsOnPaste` resolve to `false` and `keepUnknownValues` is false
in
`Select.tsx:89-100`.
3. Focus the multi-select input and paste text containing values that are
not present in
`fullSelectOptions` (e.g. `UnknownValue1,UnknownValue2`). The paste is
handled by
`onPaste` at `Select.tsx:779-806`; since `isSingleMode` is false, execution
enters the
multi-select branch that immediately calls `e.preventDefault();` at
`Select.tsx:787`,
preventing the browser from inserting the clipboard text into the input, and
then computes
`array = uniq(splitWithQuoteEscaping(pastedText, tokenSeparators));` at
`Select.tsx:788`.
4. For each token, `getOption(item, fullSelectOptions, true)` fails and
`keepUnknownValues` is false, so `getPastedTextValue(item)` (defined at
`Select.tsx:50-69`) returns `undefined` and the `values` array is filtered
to empty in
`Select.tsx:91-105`. No options are added, no selections change,
`fireOnChange()` still
runs at `Select.tsx:107`, but because default paste was prevented, the
user’s pasted text
never appears in the input, effectively discarding valid user input whenever
tokenization
yields no accepted values.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a768b2868fa74edfb670b3549ff75dc1&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a768b2868fa74edfb670b3549ff75dc1&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx
**Line:** 787:788
**Comment:**
*Logic Error: Always calling `preventDefault()` on multi-select paste
suppresses normal paste behavior even when no valid token is produced, which
can silently drop user input (for example unknown value with `allowNewOptions`
disabled). Only prevent default when the paste is actually consumed by
tokenization/selection, otherwise let the input receive the pasted text.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39068&comment_hash=e23e63476c9d2b09bc5fe7028b03c89d76201a40590407a6e5eb60d7741af8f2&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39068&comment_hash=e23e63476c9d2b09bc5fe7028b03c89d76201a40590407a6e5eb60d7741af8f2&reaction=dislike'>👎</a>
##########
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx:
##########
@@ -354,6 +364,9 @@ const Select = forwardRef(
}
return previousState;
});
+ setInputValue('');
+ setIsSearching(false);
+ setVisibleOptions(fullSelectOptions);
fireOnChange();
Review Comment:
**Suggestion:** The search text is now always cleared after multi-select,
even when `autoClearSearchValue` is false. This breaks the component contract
for callers that rely on keeping the typed search term after selecting an item.
Gate these resets behind the existing `autoClearSearchValue` behavior (or a
dedicated condition for tokenization-only flows). [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Multi-select filters cannot preserve search term post-selection.
- ⚠️ `autoClearSearchValue` flag no longer controls clearing.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Render the core Select component from `Select.tsx:93-133` with
`mode="multiple"`
(multi-select) and explicitly set `autoClearSearchValue={false}` so callers
expect the
search term to be preserved after selection.
2. Type a search term into the Select’s input (wired via
`onSearch={shouldShowSearch ?
onSearchChange : undefined}` and `searchValue={inputValue}` at
`Select.tsx:68-77`),
causing `onSearchChange` and `runSearchLogic` (`Select.tsx:285-35`) to set
`inputValue`
and filter `visibleOptions`.
3. Click a matching option; antd Select invokes the `onSelect` prop, calling
`handleOnSelect` at `Select.tsx:337-44`. Because `isSingleMode` is false,
the multi-select
branch executes, updating `selectValue`.
4. After updating `selectValue`, the multi-select branch unconditionally
executes
`setInputValue(''); setIsSearching(false);
setVisibleOptions(fullSelectOptions);
fireOnChange();` at `Select.tsx:367-370`, clearing the search text and
search state even
though `autoClearSearchValue` is `false`. The subsequent guarded block `if
(autoClearSearchValue) { ... }` at `Select.tsx:372-376` is no longer the
sole authority
over search clearing, breaking the component contract for callers relying on
the flag.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=fbe81aab5d6b4f4f98a43e3425e06249&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=fbe81aab5d6b4f4f98a43e3425e06249&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx
**Line:** 367:370
**Comment:**
*Api Mismatch: The search text is now always cleared after
multi-select, even when `autoClearSearchValue` is false. This breaks the
component contract for callers that rely on keeping the typed search term after
selecting an item. Gate these resets behind the existing `autoClearSearchValue`
behavior (or a dedicated condition for tokenization-only flows).
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39068&comment_hash=0121e3746a01e6c1bb6018a5f193f2aefcd1b89278f988eb73eb4ca3a1ec0280&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39068&comment_hash=0121e3746a01e6c1bb6018a5f193f2aefcd1b89278f988eb73eb4ca3a1ec0280&reaction=dislike'>👎</a>
--
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]
