mikebridge commented on code in PR #44274:
URL: https://github.com/apache/superset/pull/44274#discussion_r4039680475


##########
superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopoverTrigger.tsx:
##########
@@ -201,9 +201,13 @@ const ColumnSelectPopoverTriggerInner = ({
         title={popoverTitle}
         destroyOnHidden
       >
-        {/* Wrap in span so the Popover can attach a ref without relying
-            on findDOMNode (deprecated in React 18+). */}
-        <span>{children}</span>
+        {/* Wrap in a span so the Popover can attach a ref without relying
+            on findDOMNode (deprecated in React 18+). It must be block-level:
+            antd anchors the popup to this element's box, and a bare inline
+            span around block content (or the empty placeholder the "add new"
+            popovers use) measures 0×0 at the control's left edge, which
+            renders the popover detached (sc-120502). */}
+        <span style={{ display: 'block' }}>{children}</span>

Review Comment:
   Linking Amin’s measured explanation directly to this thread: 
https://github.com/apache/superset/pull/44274#pullrequestreview-5212776087 . 
The placeholder can remain zero-height, but the block wrapper supplies the full 
control width; with ControlPopover’s right placement, that width is what moves 
the popup off the panel. Artificial height is not required for this fix. The 
follow-up at 80456cdc2c6b281b5fb5ce47240904ecf959c3c5 also fixes the two 
sibling triggers and qualifies all five wrapper comments around the zero-height 
case: https://github.com/apache/superset/pull/44274#discussion_r4039004826 and 
https://github.com/apache/superset/pull/44274#discussion_r4039005115 . Could we 
consider the zero-height objection addressed on that basis?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to