You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "jaymasiwal (via GitHub)" <gi...@apache.org> on 2026/03/21 20:25:43 UTC
[PR] fix(ui): remove getPopupContainer usage to prevent double scrollbar [superset]
jaymasiwal opened a new pull request, #38787:
URL: https://github.com/apache/superset/pull/38787
Removes usage of getPopupContainer in Select component to prevent double scrollbar issue in dropdown rendering.
This avoids nested scroll containers and improves dropdown usability.
--
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(ui): remove getPopupContainer usage to prevent double scrollbar [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on PR #38787:
URL: https://github.com/apache/superset/pull/38787#issuecomment-4107287537
Closing this in favor of PR https://github.com/apache/superset/pull/38792
--
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(ui): remove getPopupContainer usage to prevent double scrollbar [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #38787:
URL: https://github.com/apache/superset/pull/38787#discussion_r2970226874
##########
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx:
##########
@@ -736,9 +736,7 @@ const Select = forwardRef(
popupRender={popupRender}
filterOption={handleFilterOption}
filterSort={sortComparatorWithSearch}
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Broken dropdown positioning</b></div>
<div id="fix">
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.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
filterSort={sortComparatorWithSearch}
getPopupContainer={getPopupContainer || (triggerNode => triggerNode.parentNode)}
````
</div>
</details>
</div>
<small><i>Code Review Run #cef156</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(ui): remove getPopupContainer usage to prevent double scrollbar [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #38787:
URL: https://github.com/apache/superset/pull/38787#issuecomment-4104308325
## **Sequence Diagram**
This 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.
```mermaid
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](https://codeant.ai)*
--
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(ui): remove getPopupContainer usage to prevent double scrollbar [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 #38787:
URL: https://github.com/apache/superset/pull/38787#discussion_r2970173896
##########
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx:
##########
@@ -736,9 +736,7 @@ const Select = forwardRef(
popupRender={popupRender}
filterOption={handleFilterOption}
filterSort={sortComparatorWithSearch}
- getPopupContainer={
- getPopupContainer || (triggerNode => triggerNode.parentNode)
- }
+
Review 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]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ 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.
```
</details>
```suggestion
getPopupContainer={getPopupContainer}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
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.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
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.
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38787&comment_hash=20f7425acba0f42a3587cb79b1a3dc5602144e1358f18204765d3261cc6ec333&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38787&comment_hash=20f7425acba0f42a3587cb79b1a3dc5602144e1358f18204765d3261cc6ec333&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(ui): remove getPopupContainer usage to prevent double scrollbar [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38787:
URL: https://github.com/apache/superset/pull/38787#issuecomment-4104305077
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(ui): remove getPopupContainer usage to prevent double scrollbar [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on PR #38787:
URL: https://github.com/apache/superset/pull/38787#issuecomment-4104997809
Why are so many files got deleted?
--
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(ui): remove getPopupContainer usage to prevent double scrollbar [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber closed pull request #38787: fix(ui): remove getPopupContainer usage to prevent double scrollbar
URL: https://github.com/apache/superset/pull/38787
--
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