Re: [PR] fix: remove getPopupContainer causing double scrollbar (#35833) [superset]

["7487 (via GitHub)" <gi...@apache.org>]

7487 commented on code in PR #39657:
URL: https://github.com/apache/superset/pull/39657#discussion_r3901041154


##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/GroupByFilterCard.tsx:
##########
@@ -608,7 +608,7 @@ const GroupByFilterCard: FC<GroupByFilterCardProps> = ({
                   .includes(input.toLowerCase())
               }
               sortComparator={sortComparator}
-              getPopupContainer={triggerNode => triggerNode.parentNode}
+              getPopupContainer={() => document.body}

Review Comment:
   Confirmed — with the popup portaled to `document.body`, the `.ant-select-dropdown` rules scoped to `HorizontalFormItem` no longer matched, so the horizontal dropdown lost its 200–400px clamp. Fixed in d7c71832: the constraints now travel with the popup via `styles.popup.root` (inline on the popup root, so they still win over rc-trigger's measured `min-width`, as the old `!important` rules did), and the dead scoped block is removed. Added a jest test that opens the horizontal-layout dropdown and asserts the clamp on the body-mounted popup.



##########
superset-frontend/src/explore/components/controls/ColorSchemeControl/index.tsx:
##########
@@ -321,7 +321,7 @@ const ColorSchemeControl = ({
         placeholder={t('Select scheme')}
         value={currentScheme}
         showSearch
-        getPopupContainer={triggerNode => triggerNode.parentNode}
+        getPopupContainer={() => document.body}

Review Comment:
   Confirmed. In production builds the `css` prop on `Select` compiles to a class on the Select's container div, and the old `triggerNode.parentNode` popup container was exactly that div — so these rules did match before and stopped matching once the popup portals to `document.body`. Fixed in d7c71832: the grouped-option rules now attach to the popup root via `classNames.popup.root` (emotion `ClassNames`, the same pattern `DateFunctionTooltip` already uses for its portaled overlay), so they follow the popup wherever it mounts. Also applied the same fix to the word-cloud plugin's copy of this control. Added a jest test asserting the grouped-option rules are wired to the body-mounted popup.



-- 
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