codeant-ai-for-open-source[bot] commented on code in PR #37479:
URL: https://github.com/apache/superset/pull/37479#discussion_r2737647384
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -513,7 +513,9 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
name={formData.nativeFilterId}
allowClear
allowNewOptions={!searchAllOptions && creatable !== false}
- allowSelectAll={!searchAllOptions}
+ allowSelectAll={
+ multiSelect && !(searchAllOptions && data.length >= 1000)
+ }
Review Comment:
**Suggestion:** Logic/regression: this condition reintroduces disabling
"Select All" for large dynamic-search datasets (when `searchAllOptions` is true
and `data.length >= 1000`), which contradicts the PR intent to enable Select
All for dynamic search; if the component already handles selecting only
visible/filtered results, prefer enabling Select All for all multi-selects by
removing the arbitrary size cutoff. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ "Select All" hidden for large dynamic-search filters.
- ⚠️ Contradicts PR feature enabling Select All.
- ⚠️ Affects multi-select dashboard filter UX.
```
</details>
```suggestion
allowSelectAll={multiSelect}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a dashboard filter with `Dynamically search all filter values`
enabled and
`multiSelect = true` (this corresponds to `formData.searchAllOptions = true`
and
`formData.multiSelect = true` passed into `PluginFilterSelect`).
2. Ensure the underlying dataset produces >= 1000 option rows for the filter
(so
`data.length >= 1000`). The component's JSX evaluates the `allowSelectAll`
expression at
lines 516-518 of
`superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx`.
3. Open the filter dropdown in the running UI: because the expression
short-circuits when
`searchAllOptions && data.length >= 1000` is true, the Select component is
rendered with
`allowSelectAll={false}` and the "Select All"/"Deselect All" buttons are
hidden, contrary
to the PR description.
4. Verify behavior change by replacing the expression with
`allowSelectAll={multiSelect}`:
with that change the "Select All" button appears for dynamic-search
multi-select filters
and selecting it follows the Select component's internal logic (selects
visible results
when filtered).
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
**Line:** 516:518
**Comment:**
*Logic Error: Logic/regression: this condition reintroduces disabling
"Select All" for large dynamic-search datasets (when `searchAllOptions` is true
and `data.length >= 1000`), which contradicts the PR intent to enable Select
All for dynamic search; if the component already handles selecting only
visible/filtered results, prefer enabling Select All for all multi-selects by
removing the arbitrary size cutoff.
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.
```
</details>
--
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]
