Skip to content

fix(ui): improve getPopupContainer to avoid double scrollbar without breaking modal behavior#38792

Open
jaymasiwal wants to merge 3 commits intoapache:masterfrom
jaymasiwal:fix-scroll-correct
Open

fix(ui): improve getPopupContainer to avoid double scrollbar without breaking modal behavior#38792
jaymasiwal wants to merge 3 commits intoapache:masterfrom
jaymasiwal:fix-scroll-correct

Conversation

@jaymasiwal
Copy link
Contributor

@jaymasiwal jaymasiwal commented Mar 22, 2026

User description

Fixes double scrollbar issue in Select dropdown without breaking existing behavior.

  • Preserves getPopupContainer if provided
  • Falls back to modal container when inside .ant-modal
  • Defaults to document.body otherwise

Prevents nested scroll containers while maintaining correct dropdown positioning.


CodeAnt-AI Description

Keep Select dropdowns inside modals without adding an extra scrollbar

What Changed

  • Select dropdowns now open inside the current modal when used in one, which prevents the double-scrollbar problem
  • Outside a modal, dropdowns open on the page body as before
  • Any custom dropdown container setting is still respected
  • Test coverage was updated to match dropdowns rendered outside the local component tree

Impact

✅ Fewer double-scrollbar issues in modal dropdowns
✅ Cleaner Select behavior inside dialogs
✅ Stable dropdown placement outside modals

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here 

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret? 

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here 

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports. 

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review 

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 22, 2026

Code Review Agent Run #e8bdc8

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 080fff9..080fff9
    • superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@dosubot dosubot bot added the change:frontend Requires changing the frontend label Mar 22, 2026
@codeant-ai-for-open-source codeant-ai-for-open-source bot added the size:XS This PR changes 0-9 lines, ignoring generated files label Mar 22, 2026
@codeant-ai-for-open-source
Copy link
Contributor

Sequence Diagram

This PR updates how the Select component chooses its popup container to prevent double scrollbars. It now keeps a custom container when provided, otherwise uses the nearest modal container, and falls back to the page body.

sequenceDiagram participant User participant Select participant DOM participant Popup User->>Select: Open dropdown Select->>Select: Check custom popup container prop alt Custom container provided Select->>Popup: Render in provided container else No custom container Select->>DOM: Find nearest modal container alt Modal container found Select->>Popup: Render inside modal container else No modal container Select->>Popup: Render in document body end end 
Loading

Generated by CodeAnt AI

@codeant-ai-for-open-source codeant-ai-for-open-source bot added size:XS This PR changes 0-9 lines, ignoring generated files and removed size:XS This PR changes 0-9 lines, ignoring generated files labels Mar 23, 2026
@codeant-ai-for-open-source codeant-ai-for-open-source bot added size:XS This PR changes 0-9 lines, ignoring generated files and removed size:XS This PR changes 0-9 lines, ignoring generated files labels Mar 23, 2026
@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 23, 2026

Code Review Agent Run #e104ae

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 1bcce78..1bcce78
    • superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@codeant-ai-for-open-source codeant-ai-for-open-source bot added size:M This PR changes 30-99 lines, ignoring generated files and removed size:XS This PR changes 0-9 lines, ignoring generated files labels Mar 25, 2026
@netlify
Copy link

netlify bot commented Mar 25, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit d3a371c
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/69c3fdbc1dca620008bf70c7
😎 Deploy Preview https://deploy-preview-38792--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Comment on lines +208 to +216
await waitFor(async () => {
const tabs = screen.getAllByText('Specific Date/Time');
if (tabs.length < 2) {
const nows = screen.getAllByText('Now');
userEvent.click(nows[nows.length - 1]);
throw new Error('Waiting for second tab...');
}
userEvent.click(tabs[1]);
}, { timeout: 3000 });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The waitFor retry callback performs clicks and throws intentionally, which means every retry triggers extra UI side effects and can over-increment onChange unpredictably. waitFor should only poll for a condition, then perform the click once after the condition is met. [logic error]

Severity Level: Major ⚠️
- ⚠️ DateFilterControl mode-change test becomes flaky in CI. - ⚠️ Jest call-count assertion intermittently fails.
Suggested change
await waitFor(async () => {
const tabs = screen.getAllByText('Specific Date/Time');
if (tabs.length < 2) {
const nows = screen.getAllByText('Now');
userEvent.click(nows[nows.length - 1]);
throw new Error('Waiting for second tab...');
}
userEvent.click(tabs[1]);
}, { timeout: 3000 });
await waitFor(
() => expect(screen.getAllByText('Specific Date/Time')).toHaveLength(2),
{ timeout: 3000 },
);
const tabs = screen.getAllByText('Specific Date/Time');
await userEvent.click(tabs[1]);
Steps of Reproduction ✅
1. Run test `triggers onChange when the mode changes` in `superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx:188` with `onChange = jest.fn()`. 2. Execution enters `waitFor` block at `CustomFrame.test.tsx:208-216`; when `tabs.length < 2`, callback clicks a `Now` option (`line 212`) and deliberately throws (`line 213`) to force retry. 3. Each retry repeats the click side effect; in component `superset-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx:68-74`, `onChange(...)` always calls `props.onChange(...)`, incrementing the mock call count. 4. When retries occur before both tabs appear, extra `onChange` calls are accumulated, making final assertion `toHaveBeenCalledTimes(2)` at `CustomFrame.test.tsx:218` nondeterministic/flaky.
Prompt for AI Agent 🤖
This is a comment left during a code review. **Path:** superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx **Line:** 208:216 **Comment:** *Logic Error: The `waitFor` retry callback performs clicks and throws intentionally, which means every retry triggers extra UI side effects and can over-increment `onChange` unpredictably. `waitFor` should only poll for a condition, then perform the click once after the condition is met. 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.
👍 | 👎
@jaymasiwal
Copy link
Contributor Author

All CI checks are now passing. I have updated the test selectors in SelectControl.test.tsx and CustomFrame.test.tsx to use global screen and async findBy logic to support the transition to Portals. This PR is ready for final review and merge. Thanks!

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 25, 2026

Code Review Agent Run #23c825

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 1bcce78..da8536f
    • superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx
    • superset-frontend/src/explore/components/controls/SelectControl.test.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@jaymasiwal
Copy link
Contributor Author

Hi everyone,

I’ve been working on improving frontend UI behavior in Superset, particularly around dropdown rendering and container handling.

Recent contributions:

While working on these, I encountered recurring issues related to:

  • inconsistent container handling across components
  • portal-based rendering causing test and UI inconsistencies
  • difficulty maintaining predictable behavior in modal vs non-modal contexts

I’ve also worked through fixing related Jest test failures (especially around portal-based dropdowns and async rendering), which highlighted gaps in current testing patterns.

I’m planning to propose a GSoC (Google Summer of Code 26) project focused on:

  • standardizing container rendering logic
  • improving Explore view performance
  • refactoring control components for consistency and testability

Before finalizing the proposal, I wanted to ask:
Does this direction align with current frontend priorities in Superset?
Are there specific areas in Explore view or control components that would benefit most from this effort?

Would really appreciate any feedback.

Thanks!

@codeant-ai-for-open-source codeant-ai-for-open-source bot added size:M This PR changes 30-99 lines, ignoring generated files and removed size:M This PR changes 30-99 lines, ignoring generated files labels Mar 25, 2026
@sadpandajoe
Copy link
Member

@jaymasiwal can you add before and after screenshots?

@sadpandajoe
Copy link
Member

Also is this a duplicate of #38659? if so can you either close one or combine them into one PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:frontend Requires changing the frontend packages size/M size:M This PR changes 30-99 lines, ignoring generated files

2 participants