You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "BansonVuong (via GitHub)" <gi...@apache.org> on 2026/05/20 21:07:27 UTC

[PR] fix(dashboard): prevent native filter Select dropdown from obscuring input on small screens [superset]

BansonVuong opened a new pull request, #40297:
URL: https://github.com/apache/superset/pull/40297

   ### SUMMARY
   
   The Value filter dropdown in the dashboard native filter bar was mounting its popup to the trigger's parent node. On smaller viewports or with elevated browser zoom the constrained parent prevented antd's overflow logic from flipping the menu, so the dropdown rendered over the input field.
   
   This PR routes Dashboard/FilterBar Select filters through a new shared `getSelectPopupContainer` helper that mounts to `document.body` when the filter is not overflowed. That lets `DROPDOWN_ALIGN_BOTTOM`'s `overflow: { adjustY: 1 }` position the menu against the viewport β€” so the popup flips above the input when there is no space below. Overflowed filters (which render inside the filter-bar popover) and the filter-config modal keep their existing container behavior to avoid regressing those positioning contexts.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   <!-- TODO: capture short clip of small-viewport repro before/after -->
   
   ### TESTING INSTRUCTIONS
   
   1. Open the example Sales Dashboard.
   2. Reduce the viewport height (or increase browser zoom) until there isn't enough room below the Value filter for a normal dropdown.
   3. Click a Value filter.
   4. Confirm the dropdown flips above the input instead of obscuring it.
   5. Verify these still work as before:
      - Overflowed native filters (filter bar with many filters in horizontal mode).
      - Filter-config modal value selects.
      - Non-dashboard Select usage of this plugin.
   
   Unit tests: `npm test -- SelectFilterPlugin.test` (36/36 pass, including 3 new tests for the helper).
   
   ### ADDITIONAL INFORMATION
   
   - [x] Has associated issue: #34135
   - [ ] Required feature flags:
   - [x] Changes UI
   - [ ] Includes DB Migration
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(dashboard): prevent native filter Select dropdown from obscuring input on small screens [superset]

Posted by "gabotorresruiz (via GitHub)" <gi...@apache.org>.
gabotorresruiz commented on PR #40297:
URL: https://github.com/apache/superset/pull/40297#issuecomment-5589926670

   Hi @BansonVuong, thanks for digging into this one, and sorry it sat in the queue so long. Your root cause analysis (the constrained parent container defeating antd's flip logic) is exactly right.
   
   In the meantime #40032 merged in June and fixed #34135 in this same spot: it mounts the dashboard filter dropdown under `document.body` by default and only keeps the parent container for the filter config modal and overflow cases. I ran the Select plugin suite at current master and its regression test asserts the dropdown is portaled to `document.body`, so the fix is covered there; that is also why this branch now conflicts in `SelectFilterPlugin.tsx`.
   
   One thing worth flagging since the review threads touched on it: the dashboard filter bar never passes `appSection` into the plugin (only the filter config modal sets it), so the `AppSection.Dashboard`/`AppSection.FilterBar` branch here would not have fired at runtime. The merged fix sidesteps that by defaulting to `document.body`, and its test renders the real component and asserts the portal target, which is the pattern that catches unwired props where pure helper tests cannot.
   
   Given all that I think the kindest path is to close this one as superseded. The investigation was solid and the conclusion matched where master landed, so please read this as timing, nothing more. If you spot the overlap still reproducing on current master, or want to pick up another native filters issue, happy to dig in with you.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(dashboard): prevent native filter Select dropdown from obscuring input on small screens [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #40297:
URL: https://github.com/apache/superset/pull/40297#issuecomment-4594869769

   Lots of bot comments to address/refute as you see fit. Approving CI in the meantime. Even failing tests would help if possible.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(dashboard): prevent native filter Select dropdown from obscuring input on small screens [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40297:
URL: https://github.com/apache/superset/pull/40297#issuecomment-4502680147

   <!-- Bito Reply -->
   The PR diff shows that the `getSelectPopupContainer` function was added to handle popup container logic based on `appSection` and `showOverflow`. The function ensures that for dashboard and filter bar sections, the popup is mounted to `document.body` when not overflowing, and to the parent node otherwise. The test cases added validate this behavior. To resolve the issue, the `appSection` should be propagated from the native filter bar into `SuperChart` with `appSection={AppSection.Dashboard}` or `AppSection.FilterBar` in `FilterValue.tsx`. This will ensure the correct container is used for the dropdowns.
   
   **superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx**
   ```
   export const getSelectPopupContainer = (
     appSection: AppSection,
     showOverflow: boolean,
     parentRef?: RefObject<any>,
   ) => {
     if (showOverflow) {
       return () => (parentRef?.current as HTMLElement) || document.body;
     }
   
     if (
       appSection === AppSection.Dashboard ||
       appSection === AppSection.FilterBar
     ) {
       return () => document.body;
     }
   
     return (trigger: HTMLElement) =>
       (trigger?.parentNode as HTMLElement) || document.body;
   };
   ```
   
   **superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx**
   ```
   test('uses document.body as popup container in dashboard filter bar', () => {
     const container = getSelectPopupContainer(AppSection.Dashboard, false);
     expect(container(document.createElement('div'))).toBe(document.body);
   });
   ```


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(dashboard): prevent native filter Select dropdown from obscuring input on small screens [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed pull request #40297: fix(dashboard): prevent native filter Select dropdown from obscuring input on small screens
URL: https://github.com/apache/superset/pull/40297


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(dashboard): prevent native filter Select dropdown from obscuring input on small screens [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40297:
URL: https://github.com/apache/superset/pull/40297#issuecomment-4502640547

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(dashboard): prevent native filter Select dropdown from obscuring input on small screens [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #40297:
URL: https://github.com/apache/superset/pull/40297#issuecomment-5593186272

   I think the underlying bug (#34135) was already fixed on master by #40032, which took a safer approach (defaults to document.body except for FilterConfigModal, rather than requiring an explicit AppSection.Dashboard/FilterBar match that the dashboard filter bar never actually sets)
   
   
   I think we can close this one out as superseded, but let me know if you see otherwise, and we can reopen it. Appreciate the PR!


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(dashboard): prevent native filter Select dropdown from obscuring input on small screens [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40297:
URL: https://github.com/apache/superset/pull/40297#discussion_r3277211813


##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -93,6 +100,26 @@ function reducer(draft: DataMask, action: DataMaskAction) {
   }
 }
 
+export const getSelectPopupContainer = (
+  appSection: AppSection,
+  showOverflow: boolean,
+  parentRef?: RefObject<any>,
+) => {
+  if (showOverflow) {
+    return () => (parentRef?.current as HTMLElement) || document.body;
+  }
+
+  if (
+    appSection === AppSection.Dashboard ||
+    appSection === AppSection.FilterBar
+  ) {
+    return () => document.body;
+  }

Review Comment:
   **🟠 Architect Review β€” HIGH**
   
   Dashboard native filter Selects never set ChartProps.appSection, so getSelectPopupContainer is called with appSection undefined in the FilterBar path; the Dashboard/FilterBar branch (lines 112–117) is never taken and non-overflowing dropdowns still mount to the trigger parent instead of document.body.
   
   **Suggestion:** Propagate the correct AppSection from the native filter bar into SuperChart (e.g., appSection={AppSection.Dashboard} or AppSection.FilterBar in FilterValue.tsx) so transformProps can pass it through to PluginFilterSelect, and add an integration test that renders a filter via FilterValue/SuperChart and asserts getSelectPopupContainer returns document.body for non-overflowing dashboard filters.
   
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2625a7bc1e6e4c1d91f649775b956338&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=2625a7bc1e6e4c1d91f649775b956338&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 an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings β€” not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
   
   **Path:** superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
   **Line:** 112:117
   **Comment:**
   	*HIGH: Dashboard native filter Selects never set ChartProps.appSection, so getSelectPopupContainer is called with appSection undefined in the FilterBar path; the Dashboard/FilterBar branch (lines 112–117) is never taken and non-overflowing dropdowns still mount to the trigger parent instead of document.body.
   
   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.
   If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix β€” do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
   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>



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(dashboard): prevent native filter Select dropdown from obscuring input on small screens [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40297:
URL: https://github.com/apache/superset/pull/40297#issuecomment-4595038511

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40297?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 64.19%. Comparing base ([`f860047`](https://app.codecov.io/gh/apache/superset/commit/f8600471fadd749f0cb36342a9b6f66d5b1f6ed9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`78c8816`](https://app.codecov.io/gh/apache/superset/commit/78c8816a25e90a5558efe579e2ec8929e371a675?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 169 commits behind head on master.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #40297   +/-   ##
   =======================================
     Coverage   64.18%   64.19%           
   =======================================
     Files        2591     2591           
     Lines      138446   138453    +7     
     Branches    32116    32119    +3     
   =======================================
   + Hits        88867    88875    +8     
   + Misses      48048    48047    -1     
     Partials     1531     1531           
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/40297/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ” | |
   |---|---|---|
   | [javascript](https://app.codecov.io/gh/apache/superset/pull/40297/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.15% <100.00%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/40297?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(dashboard): prevent native filter Select dropdown from obscuring input on small screens [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40297:
URL: https://github.com/apache/superset/pull/40297#discussion_r3277251235


##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx:
##########
@@ -203,6 +205,30 @@ describe('SelectFilterPlugin', () => {
     });
   });
 
+  test('uses document.body as popup container in dashboard filter bar', () => {
+    const container = getSelectPopupContainer(AppSection.Dashboard, false);
+    expect(container(document.createElement('div'))).toBe(document.body);
+  });
+
+  test('uses parent ref as popup container for overflowed filters', () => {
+    const parentNode = document.createElement('div');
+    const container = getSelectPopupContainer(AppSection.Dashboard, true, {
+      current: parentNode,
+    });
+    expect(container()).toBe(parentNode);
+  });
+
+  test('keeps trigger parent popup container outside dashboard filter bar', () => {
+    const triggerParent = document.createElement('div');
+    const trigger = document.createElement('div');
+    triggerParent.appendChild(trigger);
+    const container = getSelectPopupContainer(
+      AppSection.FilterConfigModal,
+      false,
+    );
+    expect(container(trigger)).toBe(triggerParent);
+  });

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Missing test for FilterBar</b></div>
   <div id="fix">
   
   The implementation at lines 113-114 handles `AppSection.FilterBar` but no test covers this case. The `AppSection` enum includes `FilterBar = 'FILTER_BAR'` (Base.ts:54) and the implementation explicitly checks for it. Add a test: `test('uses document.body as popup container in filter bar', () => { const container = getSelectPopupContainer(AppSection.FilterBar, false); expect(container(document.createElement('div'))).toBe(document.body); });`
   </div>
   
   
   <details>
   <summary>
   <b>Code suggestion</b>
   </summary>
   <blockquote>Check the AI-generated fix before applying</blockquote>
   <div id="code">
   
   
   ```
    --- superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx (lines 208-230) ---
    208:   test('uses document.body as popup container in dashboard filter bar', () => {
    209:     const container = getSelectPopupContainer(AppSection.Dashboard, false);
    210:     expect(container(document.createElement('div'))).toBe(document.body);
    211:   });
    212: +
    213: +  test('uses document.body as popup container in filter bar', () => {
    214: +    const container = getSelectPopupContainer(AppSection.FilterBar, false);
    215: +    expect(container(document.createElement('div'))).toBe(document.body);
    216: +  });
    217: +
    218:   test('uses parent ref as popup container for overflowed filters', () => {
    219:     const parentNode = document.createElement('div');
    220:     const container = getSelectPopupContainer(AppSection.Dashboard, true, {
    221:       current: parentNode,
    222:     });
    223:     expect(container()).toBe(parentNode);
    224:   });
    225: +
    226:   test('keeps trigger parent popup container outside dashboard filter bar', () => {
    227:     const triggerParent = document.createElement('div');
    228:     const trigger = document.createElement('div');
    229:     triggerParent.appendChild(trigger);
    230:     const container = getSelectPopupContainer(
    231:       AppSection.FilterConfigModal,
    232:       false,
    233:     );
    234:     expect(container(trigger)).toBe(triggerParent);
    235:   });
   ```
   
   </div>
   </details>
   
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #06c067</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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(dashboard): prevent native filter Select dropdown from obscuring input on small screens [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #40297:
URL: https://github.com/apache/superset/pull/40297#discussion_r3877738438


##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx:
##########
@@ -203,6 +205,30 @@ describe('SelectFilterPlugin', () => {
     });
   });
 
+  test('uses document.body as popup container in dashboard filter bar', () => {
+    const container = getSelectPopupContainer(AppSection.Dashboard, false);
+    expect(container(document.createElement('div'))).toBe(document.body);
+  });
+
+  test('uses parent ref as popup container for overflowed filters', () => {
+    const parentNode = document.createElement('div');
+    const container = getSelectPopupContainer(AppSection.Dashboard, true, {
+      current: parentNode,
+    });
+    expect(container()).toBe(parentNode);

Review Comment:
   This makes the test fail type checking: the helper's inferred return type is a union of callbacks, one requiring `trigger`, so `container()` is rejected at this call site. Could the helper expose one callback signature with an optional trigger?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(dashboard): prevent native filter Select dropdown from obscuring input on small screens [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(dashboard): prevent native filter Select dropdown from obscuring input on small screens [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #40297:
URL: https://github.com/apache/superset/pull/40297#discussion_r3828762183


##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -93,6 +100,26 @@ function reducer(draft: DataMask, action: DataMaskAction) {
   }
 }
 
+export const getSelectPopupContainer = (
+  appSection: AppSection,
+  showOverflow: boolean,
+  parentRef?: RefObject<any>,
+) => {
+  if (showOverflow) {
+    return () => (parentRef?.current as HTMLElement) || document.body;
+  }
+
+  if (
+    appSection === AppSection.Dashboard ||
+    appSection === AppSection.FilterBar
+  ) {
+    return () => document.body;
+  }

Review Comment:
   Agreedβ€”the dashboard path never supplies `appSection`, so this branch falls back to the trigger parent and the reported overlap remains. Could the dashboard entrypoint pass its section through to `SuperChart` and cover that integration path?



##########
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:
   Agreedβ€”the actual overflow state is only carried in `displaySettings`, while this helper reads an unset `showOverflow`; dropdowns in More filters will therefore be reparented to `document.body`. Could this use the propagated overflow state and add an overflowed-filter regression test?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org