AmoghAtreya commented on code in PR #44959:
URL: https://github.com/apache/superset/pull/44959#discussion_r4178998769
##########
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:
Just looked over this and it happens to be a false positive with no changes
required.
The test spy only wraps `React.useState` (used for placement). Visibility
uses the named `useState` import, which is the real hook. That split is
intentional so placement assertions can mock one setter without freezing
open/close.
Uncontrolled click-to-open, scroll lock, and Escape already go through
`uncontrolledVisible`, and those tests still pass.
--
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]
