Kunal8954 commented on code in PR #43978:
URL: https://github.com/apache/superset/pull/43978#discussion_r3958696977
##########
superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx:
##########
@@ -368,11 +368,7 @@ export default function DateFilterLabel(props:
DateFilterControlProps) {
onOpenChange={toggleOverlay}
overlayStyle={{ width: 'min(600px, calc(100vw - 32px))' }}
destroyOnHidden
- getPopupContainer={nodeTrigger =>
- isOverflowingFilterBar
- ? (nodeTrigger.parentNode as HTMLElement)
- : document.body
- }
+ getPopupContainer={() => document.body}
Review Comment:
Thanks for the review. Mounting to `document.body` is intentional: when a
Date Range filter is pushed into the overflow dropdown, that dropdown caps its
panel at `MAX_HEIGHT = 500px` with `overflow: auto`
(DropdownContainer.tsx:366). The outer popover was previously attached to the
trigger parent inside that dropdown, so the calendar (>500px tall) got clipped.
Attaching to `document.body` escapes the cap and is the established convention
across the codebase (SemanticLayerModal, TagModal, DatabaseModal, SqlLab
SaveDatasetModal, FilterCard), and `ControlPopover` already falls back to
`document.body` (ControlPopover.tsx:141). The `min(600px, calc(100vw - 32px))`
width is vw-relative, so it still resolves against the iframe viewport. The
now-redundant inner `getPopupContainer` conditionals in `CustomFrame` are
removed in a follow-up commit.
##########
superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx:
##########
@@ -368,11 +368,7 @@ export default function DateFilterLabel(props:
DateFilterControlProps) {
onOpenChange={toggleOverlay}
overlayStyle={{ width: 'min(600px, calc(100vw - 32px))' }}
destroyOnHidden
- getPopupContainer={nodeTrigger =>
- isOverflowingFilterBar
- ? (nodeTrigger.parentNode as HTMLElement)
- : document.body
- }
+ getPopupContainer={() => document.body}
Review Comment:
The overflow tests were updated in commit `5749465a36` to assert the new
behavior (popup attaches to `document.body` even when overflowing), and both
`DateFilterLabel.test.tsx` and `CustomFrame.test.tsx` pass (28/28). The
suggested conditional would reintroduce the clipping this PR fixes: when
`isOverflowingFilterBar` is true, `trigger.parentNode` resolves inside the
500px overflow dropdown, clipping the calendar. Since the outer popover now
always mounts to `document.body`, that branch is unreachable and has been
removed from `CustomFrame`.
--
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]
