sadpandajoe commented on code in PR #44959:
URL: https://github.com/apache/superset/pull/44959#discussion_r4188054794
##########
superset-frontend/src/explore/components/controls/ControlPopover/ControlPopover.tsx:
##########
@@ -84,15 +84,17 @@ const ControlPopover: FC<PopoverProps> = ({
getPopupContainer,
getVisibilityRatio = getElementVisibilityRatio,
open: visibleProp,
+ defaultOpen,
+ onOpenChange,
destroyOnHidden = false,
placement: initialPlacement = 'right',
autoAdjustOverflow,
...props
}) => {
+ const isControlled = visibleProp !== undefined;
const triggerElementRef = useRef<HTMLElement>();
- const [visible, setVisible] = useState(
- visibleProp === undefined ? props.defaultOpen : visibleProp,
- );
+ const [uncontrolledVisible, setUncontrolledVisible] =
useState(!!defaultOpen);
Review Comment:
Clicking Apply in Add annotation layer now mounts a closed annotation-row
popover whose initial `false` visibility sets the shared Explore container to
`overflow-y: auto`, releasing the still-open editor’s scroll lock; previously
the row started at `undefined` and skipped that effect. Could we preserve the
active lock when an unopened sibling mounts, with a regression test asserting
that the container stays locked?
--
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]
