rusackas commented on code in PR #45604:
URL: https://github.com/apache/superset/pull/45604#discussion_r4355858456
##########
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx:
##########
@@ -1003,7 +1003,7 @@ const Select = forwardRef(
getPopupContainer ||
((triggerNode: HTMLElement) =>
(triggerNode?.closest('.ant-modal-container') as HTMLElement) ||
- (triggerNode.parentNode as HTMLElement))
+ document.body)
Review Comment:
Good catch. The Select now asks the ConfigProvider (so the fullscreen-aware
container from ChartHolder) before falling back to document.body, same for
AsyncSelect. The modal check still wins first.
##########
superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.tsx:
##########
@@ -873,7 +873,7 @@ const AsyncSelect = forwardRef(
getPopupContainer ||
((triggerNode: HTMLElement) =>
(triggerNode?.closest('.ant-modal-container') as HTMLElement) ||
- (triggerNode.parentNode as HTMLElement))
+ document.body)
Review Comment:
Fair, added popupContainer.test.tsx, parameterized over Select and
AsyncSelect. One case renders in a scrollable non-modal wrapper and asserts the
popup lands under document.body outside the wrapper, the other checks a
ConfigProvider container is honored. I could not run jest locally (no deps on
this box), so CI will be the first real run.
--
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]
