fix(ui): remove getPopupContainer usage to prevent double scrollbar#38787
Closed
jaymasiwal wants to merge 3 commits intoapache:masterfrom
Closed
fix(ui): remove getPopupContainer usage to prevent double scrollbar#38787jaymasiwal wants to merge 3 commits intoapache:masterfrom
jaymasiwal wants to merge 3 commits intoapache:masterfrom
Conversation
jaymasiwal requested review from betodealmeida, craig-rueda, dpgaspar, eschutho, geido, hainenber, kgabryje, mistercrunch, nytai, rusackas, sadpandajoe and villebro as code owners 
github-actions bot added github_actions 
codeant-ai-for-open-source bot added the size:XS 
Contributor
Sequence DiagramThis PR removes the custom popup container override from the shared Select component. Dropdown menus now render in the default container, avoiding nested scroll areas and preventing the double scrollbar behavior. sequenceDiagram participant User participant SelectComponent participant DropdownLibrary participant Page User->>SelectComponent: Open select dropdown SelectComponent->>DropdownLibrary: Render dropdown without custom container DropdownLibrary->>Page: Attach popup to default container User->>Page: Scroll while dropdown is open Page-->>User: Single scrollbar and smooth dropdown behavior Generated by CodeAnt AI |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| getPopupContainer={ | ||
| getPopupContainer || (triggerNode => triggerNode.parentNode) | ||
| } | ||
| |
Contributor
There was a problem hiding this comment.
Suggestion: The component still accepts getPopupContainer in its public props but no longer forwards it to StyledSelect, which silently breaks caller expectations. Existing usages that rely on rendering the dropdown inside modals or scoped containers will now render in the default container and can cause clipping/z-index/positioning regressions. Forward the prop so explicit caller behavior is preserved. [logic error]
Severity Level: Critical 🚨
- ❌ Modal Select dropdown container targeting no longer honored. - ❌ User roles/groups modal dropdown placement can regress. - ⚠️ Explore color scheme dropdown may escape parent container. - ⚠️ Existing Select getPopupContainer contract silently broken. Suggested change
| getPopupContainer={getPopupContainer} |
Steps of Reproduction ✅
1. Open the user management modal flow rendered by `UserListModal` (`src/features/users/UserListModal.tsx`), where both role/group `<Select>` pass `getPopupContainer` at lines 206-207 and 225-226. 2. Interact with either select dropdown (`<Select ... />` at lines 198 and 217). This calls the shared core component `superset-ui-core/src/components/Select/Select.tsx`. 3. In `Select.tsx`, prop destructuring still accepts `getPopupContainer` (line 121), but the `<StyledSelect>` props block around lines 736-741 omits forwarding it (only `filterSort` then `headerPosition`). 4. Because the prop is consumed and not passed through, Ant Design falls back to default popup container behavior; dropdown renders outside caller-intended container (modal-scoped container no longer honored), causing positioning/clipping/z-index regressions in modal/scoped contexts. 5. This is verifiably an API regression: existing unit test `Select.test.tsx` line 604 (`triggers getPopupContainer if passed`) expects callback invocation at line 608, which no longer matches runtime behavior.Prompt for AI Agent 🤖
This is a comment left during a code review. **Path:** superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx **Line:** 739:739 **Comment:** *Logic Error: The component still accepts `getPopupContainer` in its public props but no longer forwards it to `StyledSelect`, which silently breaks caller expectations. Existing usages that rely on rendering the dropdown inside modals or scoped containers will now render in the default container and can cause clipping/z-index/positioning regressions. Forward the prop so explicit caller behavior is preserved. 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.![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Agent Run #cef156
Actionable Suggestions - 1
- superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx - 1
- Broken dropdown positioning · Line 738-738
Review Details
- Files reviewed - 14 · Commit Range:
f46c6f8..dd8781f- .github/CODEOWNERS
- .github/actions/cached-dependencies
- .github/actions/chart-releaser-action
- .github/actions/chart-testing-action
- .github/actions/comment-on-pr
- .github/actions/file-changes-action
- .github/actions/github-action-push-to-another-repository
- .github/actions/latest-tag
- .github/actions/pr-lint-action
- .github/issue_label_bot.yaml
- .github/workflows/bashlib.sh
- .github/workflows/caches.js
- .github/workflows/github-action-validator.sh
- superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx
- Files skipped - 60
- .github/ISSUE_TEMPLATE/bug-report.yml - Reason: Filter setting
- .github/ISSUE_TEMPLATE/config.yml - Reason: Filter setting
- .github/ISSUE_TEMPLATE/cosmetic.md - Reason: Filter setting
- .github/ISSUE_TEMPLATE/sip.md - Reason: Filter setting
- .github/PULL_REQUEST_TEMPLATE.md - Reason: Filter setting
- .github/SECURITY.md - Reason: Filter setting
- .github/actions/change-detector/action.yml - Reason: Filter setting
- .github/actions/change-detector/label-draft-pr.yml - Reason: Filter setting
- .github/actions/setup-backend/action.yml - Reason: Filter setting
- .github/actions/setup-docker/action.yml - Reason: Filter setting
- .github/actions/setup-supersetbot/action.yml - Reason: Filter setting
- .github/config.yml - Reason: Filter setting
- .github/copilot-instructions.md - Reason: Filter setting
- .github/dependabot.yml - Reason: Filter setting
- .github/labeler.yml - Reason: Filter setting
- .github/move.yml - Reason: Filter setting
- .github/stale.yml - Reason: Filter setting
- .github/workflows/bump-python-package.yml - Reason: Filter setting
- .github/workflows/cancel_duplicates.yml - Reason: Filter setting
- .github/workflows/check-python-deps.yml - Reason: Filter setting
- .github/workflows/check_db_migration_confict.yml - Reason: Filter setting
- .github/workflows/claude.yml - Reason: Filter setting
- .github/workflows/codeql-analysis.yml - Reason: Filter setting
- .github/workflows/dependency-review.yml - Reason: Filter setting
- .github/workflows/docker.yml - Reason: Filter setting
- .github/workflows/ecs-task-definition.json - Reason: Filter setting
- .github/workflows/embedded-sdk-release.yml - Reason: Filter setting
- .github/workflows/embedded-sdk-test.yml - Reason: Filter setting
- .github/workflows/ephemeral-env-pr-close.yml - Reason: Filter setting
- .github/workflows/ephemeral-env.yml - Reason: Filter setting
- .github/workflows/generate-FOSSA-report.yml - Reason: Filter setting
- .github/workflows/github-action-validator.yml - Reason: Filter setting
- .github/workflows/issue_creation.yml - Reason: Filter setting
- .github/workflows/labeler.yml - Reason: Filter setting
- .github/workflows/latest-release-tag.yml - Reason: Filter setting
- .github/workflows/license-check.yml - Reason: Filter setting
- .github/workflows/no-hold-label.yml - Reason: Filter setting
- .github/workflows/pr-lint.yml - Reason: Filter setting
- .github/workflows/pre-commit.yml - Reason: Filter setting
- .github/workflows/release.yml - Reason: Filter setting
- .github/workflows/showtime-cleanup.yml - Reason: Filter setting
- .github/workflows/showtime-trigger.yml - Reason: Filter setting
- .github/workflows/superset-app-cli.yml - Reason: Filter setting
- .github/workflows/superset-docs-deploy.yml - Reason: Filter setting
- .github/workflows/superset-docs-verify.yml - Reason: Filter setting
- .github/workflows/superset-e2e.yml - Reason: Filter setting
- .github/workflows/superset-extensions-cli.yml - Reason: Filter setting
- .github/workflows/superset-frontend.yml - Reason: Filter setting
- .github/workflows/superset-helm-lint.yml - Reason: Filter setting
- .github/workflows/superset-helm-release.yml - Reason: Filter setting
- .github/workflows/superset-playwright.yml - Reason: Filter setting
- .github/workflows/superset-python-integrationtest.yml - Reason: Filter setting
- .github/workflows/superset-python-presto-hive.yml - Reason: Filter setting
- .github/workflows/superset-python-unittest.yml - Reason: Filter setting
- .github/workflows/superset-translations.yml - Reason: Filter setting
- .github/workflows/superset-websocket.yml - Reason: Filter setting
- .github/workflows/supersetbot.yml - Reason: Filter setting
- .github/workflows/tag-release.yml - Reason: Filter setting
- .github/workflows/tech-debt.yml - Reason: Filter setting
- .github/workflows/welcome-new-users.yml - Reason: Filter setting
- 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 
| @@ -736,9 +736,7 @@ const Select = forwardRef( | |||
| popupRender={popupRender} | |||
| filterOption={handleFilterOption} | |||
| filterSort={sortComparatorWithSearch} | |||
Contributor
There was a problem hiding this comment.
Broken dropdown positioning
Removing getPopupContainer breaks the Select component's dropdown positioning, which is critical for rendering in modals. The test suite expects this prop to function, and widespread usage in the codebase relies on it. Revert this change to maintain compatibility.
Code suggestion
Check the AI-generated fix before applying
Suggested change
| filterSort={sortComparatorWithSearch} | |
| filterSort={sortComparatorWithSearch} | |
| getPopupContainer={getPopupContainer || (triggerNode => triggerNode.parentNode)} |
Code Review Run #cef156
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
Contributor
| Why are so many files got deleted? |
Contributor
| Closing this in favor of PR #38792 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Removes usage of getPopupContainer in Select component to prevent double scrollbar issue in dropdown rendering.
This avoids nested scroll containers and improves dropdown usability.
CodeAnt-AI Description
Stop the Select dropdown from showing a nested scrollbar
What Changed
Impact
✅ Cleaner dropdown scrolling✅ Fewer double-scrollbar issues✅ Easier option selection in long lists💡 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:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
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:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
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.