codeant-ai-for-open-source[bot] commented on code in PR #40297:
URL: https://github.com/apache/superset/pull/40297#discussion_r3277222852
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -595,12 +622,11 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
allowSelectAll={!searchAllOptions}
value={multiSelect ? filterState.value || [] : filterState.value}
disabled={isDisabled}
- getPopupContainer={
- showOverflow
- ? () => (parentRef?.current as HTMLElement) || document.body
- : (trigger: HTMLElement) =>
- (trigger?.parentNode as HTMLElement) || document.body
- }
+ getPopupContainer={getSelectPopupContainer(
+ appSection,
+ showOverflow,
+ parentRef,
+ )}
Review Comment:
**Suggestion:** The popup-container decision now depends on `showOverflow`,
but this prop is not the overflow flag used by the filter-bar pipeline
(`FilterControls` computes and passes `overflow`, not `showOverflow`). As a
result, overflowed filters are treated as non-overflowed and this code routes
their dropdowns to `document.body`, which can break positioning inside the
"More filters" popover. Wire the actual overflow state into this call (or use
the existing overflow signal passed through display settings) so overflowed
filters keep the popover-scoped container. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Overflowed dashboard filters misplace dropdowns outside popover.
- ⚠️ "More filters" popover UI appears visually broken.
- ⚠️ Users may see dropdown overlapping unrelated dashboard content.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx`
(around lines 158–176 and 184–195), note that `overflowedByIndex` is
computed and the
boolean `overflow` is passed into `filterControlFactory(index,
filterBarOrientation,
overflow)`, indicating which filters are actually overflowed into the "More
filters"
popover.
2. In
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterControlFactory.tsx`
(around lines 53–80), see that `filterControlFactory` passes `overflow` into
`<FilterControl overflow={overflow} ... />` but does not pass any
`showOverflow` prop, so
`showOverflow` in `FilterControlProps` is always `undefined` in real
dashboard usage.
3. In
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx`
(around lines 44–56 and 139–150), observe that `FilterControl` simply
forwards
`showOverflow` to `<FilterValue showOverflow={showOverflow} ...
overflow={overflow} />`,
meaning `FilterValue` receives the correct overflow flag via `overflow` and
an
always-undefined `showOverflow`.
4. In
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx`
(around lines 315–321 and 354–371), see that `displaySettings` is built with
`isOverflowingFilterBar: overflow`, and then `<SuperChart
showOverflow={showOverflow}
displaySettings={displaySettings} parentRef={parentRef} ... />` is rendered;
thus the
actual overflow state flows as `displaySettings.isOverflowingFilterBar`, not
via
`showOverflow`.
5. In `superset-frontend/src/filters/components/Select/transformProps.ts`
(around lines
64–83), confirm that the Select filter plugin's props are built from
`chartProps`: it
passes `displaySettings?.isOverflowingFilterBar` into
`PluginFilterSelectProps.isOverflowingFilterBar` but never maps
`chartProps.showOverflow`
to `PluginFilterSelectProps.showOverflow`, so `showOverflow` inside
`PluginFilterSelect`
stays `undefined` when used through `SuperChart`.
6. In
`superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx` (lines
103–121 and 146–168, 625–629), see that `getSelectPopupContainer(appSection,
showOverflow,
parentRef)` uses `showOverflow` to decide whether to mount the dropdown to
`parentRef`
(overflowed case) or `document.body` (non-overflowed dashboard/filter-bar
case). Because
`showOverflow` is never wired from the filter-bar pipeline, even overflowed
filters inside
the "More filters" popover will have `showOverflow === false`, causing their
dropdowns to
mount to `document.body` instead of the popover container and breaking the
intended
in-popover positioning.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=531456584a1148b89d310bfa60d34815&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=531456584a1148b89d310bfa60d34815&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/src/filters/components/Select/SelectFilterPlugin.tsx
**Line:** 625:629
**Comment:**
*Api Mismatch: The popup-container decision now depends on
`showOverflow`, but this prop is not the overflow flag used by the filter-bar
pipeline (`FilterControls` computes and passes `overflow`, not `showOverflow`).
As a result, overflowed filters are treated as non-overflowed and this code
routes their dropdowns to `document.body`, which can break positioning inside
the "More filters" popover. Wire the actual overflow state into this call (or
use the existing overflow signal passed through display settings) so overflowed
filters keep the popover-scoped container.
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%2F40297&comment_hash=a7c498c81a65dbca8389b8172525de3e6da6968475dd7f2c5f3d031b7e584deb&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40297&comment_hash=a7c498c81a65dbca8389b8172525de3e6da6968475dd7f2c5f3d031b7e584deb&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]
