fix(ui): remove getPopupContainer to prevent double scrollbar in drop…#38659
fix(ui): remove getPopupContainer to prevent double scrollbar in drop…#38659jaymasiwal wants to merge 1 commit into
Conversation
Code Review Agent Run #d6e16aActionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
codeant-ai-for-open-source
Bot
added
the
size:XS
Sequence DiagramThis PR removes custom popup container overrides so dropdown menus no longer render inside constrained modal parents. The select component now defaults to rendering in the document body, which avoids nested scrolling and eliminates double scrollbars. sequenceDiagram
participant User
participant DatabaseModal
participant Select
participant AntDesign
participant DocumentBody
User->>DatabaseModal: Open dropdown in database modal
DatabaseModal->>Select: Render select without popup container override
Select->>AntDesign: Resolve popup container to default
AntDesign->>DocumentBody: Render dropdown menu in document body
DocumentBody-->>User: Display dropdown with single scrollbar
Generated by CodeAnt AI |
jaymasiwal
force-pushed
the
fix-double-scrollbar
branch
from
8e741c3 to
c5f447a
Compare
codeant-ai-for-open-source
Bot
added
size:XS
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
jaymasiwal
force-pushed
the
fix-double-scrollbar
branch
2 times, most recently
from
f85096d to
d6ea0e6
Compare
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #e089f0
Actionable Suggestions - 1
-
superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx - 1
- Test logic error: wrong element clicked · Line 198-198
Additional Suggestions - 1
-
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx - 1
-
Inconsistent popup container default · Line 740-740The default getPopupContainer fallback changed from triggerNode.parentNode to document.body, which alters dropdown positioning and z-index behavior. This creates inconsistency with AsyncSelect, which still uses parentNode. Consider updating AsyncSelect for consistency, as this affects UI rendering in modals or containers.
-
Review Details
-
Files reviewed - 3 · Commit Range:
d6ea0e6..d6ea0e6- superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx
- superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx
- superset-frontend/src/features/databases/DatabaseModal/index.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
| await screen.findByText('Configure custom time range'), | ||
| ).toBeInTheDocument(); | ||
| userEvent.click(screen.getAllByTitle('Specific Date/Time')[1]); | ||
| await userEvent.click(await screen.findByTitle('Specific Date/Time')); |
There was a problem hiding this comment.
This change alters the test behavior by clicking the first 'Specific Date/Time' element (sinceMode) instead of the second one (untilMode). The CustomFrame component renders two Select components with identical options, including 'Specific Date/Time' for both start and end modes. The original code used getAllByTitle('Specific Date/Time')[1] to click the untilMode's option, but findByTitle('Specific Date/Time') clicks the sinceMode's. This may cause the test to verify incorrect behavior.
Code Review Run #e089f0
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
jaymasiwal
force-pushed
the
fix-double-scrollbar
branch
from
d6ea0e6 to
be007ca
Compare
codeant-ai-for-open-source
Bot
added
size:XS
There was a problem hiding this comment.
Code Review Agent Run #ada876
Actionable Suggestions - 1
-
superset-frontend/src/features/databases/DatabaseModal/index.tsx - 1
- Modal Select dropdown positioning risk · Line 1160-1160
Review Details
-
Files reviewed - 4 · Commit Range:
be007ca..be007ca- superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx
- superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx
- superset-frontend/src/explore/components/controls/SelectControl.test.tsx
- superset-frontend/src/features/databases/DatabaseModal/index.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
| getPopupContainer={triggerNode => | ||
| triggerNode.parentElement || document.body | ||
| } | ||
| dropdownStyle={{ maxHeight: 400, overflow: 'auto' }} |
There was a problem hiding this comment.
The removal of getPopupContainer from this Select component inside a Modal may cause the dropdown to be clipped or have z-index issues, as the dropdown will now render outside the modal by default. In modal contexts, getPopupContainer is typically used to ensure proper rendering. Consider adding it back, e.g., getPopupContainer={trigger => trigger.closest('.ant-modal-content')}, following patterns used in other modal Select components in the codebase like RoleFormItems.tsx and GroupListModal.tsx.
Code Review Run #ada876
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
There was a problem hiding this comment.
Pull request overview
This PR addresses the “double scrollbar” dropdown issue in modal contexts by removing/avoiding constrained popup containers and letting Ant Design dropdowns render into document.body (portal), improving scroll behavior for long option lists.
Changes:
- Removed a modal-specific
getPopupContaineroverride in the Database configuration modal’s supported-databases Select. - Updated the shared
@superset-ui/coreSelectcomponent default popup container todocument.body. - Adjusted RTL tests to find dropdown content in
document.bodyand to await async queries.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| superset-frontend/src/features/databases/DatabaseModal/index.tsx | Removes local popup container override so dropdowns are no longer constrained by modal layout. |
| superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx | Changes the shared Select default popup container to document.body to avoid nested scrolling containers. |
| superset-frontend/src/explore/components/controls/SelectControl.test.tsx | Updates a test to locate dropdown content in document.body after the popup container change. |
| superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx | Tweaks an interaction to await querying/clicking an option by title. |
You can also share your feedback on Copilot code review. Take the survey.
| userEvent.click(selectorInput); | ||
| expect(screen.getByText('Select all (3)')).toBeInTheDocument(); | ||
| expect( | ||
| await within(document.body).findByText('Select all (3)') | ||
| ).toBeInTheDocument(); |
| await screen.findByText('Configure custom time range'), | ||
| ).toBeInTheDocument(); | ||
| userEvent.click(screen.getAllByTitle('Specific Date/Time')[1]); | ||
| await userEvent.click(await screen.findByTitle('Specific Date/Time')); |
| getPopupContainer={ | ||
| getPopupContainer || (triggerNode => triggerNode.parentNode) | ||
| getPopupContainer || (() => document.body) | ||
| } |
| expect( | ||
| await within(document.body).findByText('Select all (3)') | ||
| ).toBeInTheDocument(); |
jaymasiwal
force-pushed
the
fix-double-scrollbar
branch
from
be007ca to
f910bc8
Compare
jaymasiwal
force-pushed
the
fix-double-scrollbar
branch
from
f910bc8 to
6516082
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (99.92%) is below the target coverage (100.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #38659 +/- ##
==========================================
+ Coverage 64.99% 65.50% +0.51%
==========================================
Files 1819 1828 +9
Lines 72515 73605 +1090
Branches 23149 23295 +146
==========================================
+ Hits 47128 48218 +1090
Misses 25387 25387
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jaymasiwal
force-pushed
the
fix-double-scrollbar
branch
from
6516082 to
4c33749
Compare
Code Review Agent Run #1a36a7Actionable Suggestions - 0Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
jaymasiwal
force-pushed
the
fix-double-scrollbar
branch
from
4c33749 to
0c669dc
Compare
codeant-ai-for-open-source
Bot
added
size:M
jaymasiwal
force-pushed
the
fix-double-scrollbar
branch
from
0c669dc to
c5f4bd4
Compare
There was a problem hiding this comment.
Code Review Agent Run #00fccc
Actionable Suggestions - 1
-
superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx - 1
- Incorrect async usage in test · Line 300-300
Additional Suggestions - 1
-
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx - 1
-
Dropdown positioning behavior change · Line 739-739The default fallback for getPopupContainer has changed from (triggerNode) => triggerNode.parentNode to () => document.body. This aligns with Ant Design's standard default but alters dropdown rendering behavior. In contexts like modals, this may cause dropdowns to render outside the modal container, potentially affecting visibility or z-index. Verify that any modals containing Select components have their getContainer prop configured if needed.
-
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx - 1
- Async element waiting · Line 335-335
Review Details
-
Files reviewed - 4 · Commit Range:
c5f4bd4..c5f4bd4- superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx
- superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx
- superset-frontend/src/explore/components/controls/SelectControl.test.tsx
- superset-frontend/src/features/databases/DatabaseModal/index.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
|
|
||
| const randomDate = screen.getByTitle('2021-03-28'); | ||
| userEvent.click(randomDate); | ||
| const randomDate = await screen.getByTitle('2021-03-28'); |
There was a problem hiding this comment.
The await on screen.getByTitle is incorrect since getByTitle is synchronous. Use findByTitle instead, as done in the START test, to ensure the date element is available before clicking.
Code Review Run #00fccc
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
jaymasiwal
force-pushed
the
fix-double-scrollbar
branch
from
c5f4bd4 to
6bec001
Compare
There was a problem hiding this comment.
Code Review Agent Run #04ec0e
Actionable Suggestions - 1
-
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx - 1
- Potential dropdown scrolling regression · Line 739-739
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx - 1
- Incorrect Async Query Usage · Line 336-336
Review Details
-
Files reviewed - 4 · Commit Range:
6bec001..6bec001- superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx
- superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx
- superset-frontend/src/explore/components/controls/SelectControl.test.tsx
- superset-frontend/src/features/databases/DatabaseModal/index.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
| getPopupContainer={ | ||
| getPopupContainer || (triggerNode => triggerNode.parentNode) | ||
| } | ||
| getPopupContainer={getPopupContainer || (() => document.body)} |
There was a problem hiding this comment.
This change switches the default getPopupContainer from the recommended triggerNode.parentNode to document.body, which Ant Design docs note causes dropdowns to scroll out of view in certain scenarios. Consider reverting to maintain proper dropdown positioning.
Code suggestion
Check the AI-generated fix before applying
| getPopupContainer={getPopupContainer || (() => document.body)} | |
| getPopupContainer={ | |
| getPopupContainer || (triggerNode => triggerNode.parentNode) | |
| } |
Code Review Run #04ec0e
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
User description
SUMMARY
Fixes the dual scrollbar issue in dropdown menus by removing custom
getPopupContaineroverrides that forced dropdowns to render inside constrained parent containers.Allowing Ant Design to render dropdowns in
document.body(default behavior) prevents nested scroll containers and resolves the double scrollbar problem.TESTING INSTRUCTIONS
ADDITIONAL INFORMATION
Fixes #35833
CodeAnt-AI Description
Render dropdowns into page body to fix double scrollbar in modals
What Changed
Impact
✅ Fewer double scrollbars in dropdowns✅ Clearer dropdown lists in modals with long option sets✅ More reliable UI tests💡 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.