Fix popover behavior when popover is open and the trigger is clicked (#211)

* Fix popover behavior when open and clicking on trigger

* Remove console log

* Resolve merge conflicts

* Remove destructuring in favor of property access

* Extract usePopover return type into an interface

* Fix merge

packages/playground/src/shared/hello-world-extension-using-sheet-object/index.tsx

@@ -1,7 +1,7 @@
 import React from 'react'
 import ReactDOM from 'react-dom'
 import App from './App'
-import type {ISheetObject} from '@theatre/core';
+import type {ISheetObject} from '@theatre/core'
 import {onChange, types, val} from '@theatre/core'
 import studio from '@theatre/studio'
 import extension from '@theatre/r3f/dist/extension'

theatre/studio/src/panels/DetailPanel/ProjectDetails.tsx

@@ -62,7 +62,7 @@ const ProjectDetails: React.FC<{
     }, 40000)
   }, [])
 
-  const [tooltip, openExportTooltip] = usePopover(
+  const exportTooltip = usePopover(
     {debugName: 'ProjectDetails', pointerDistanceThreshold: 50},
     () => (
       <ExportTooltip>
@@ -81,13 +81,13 @@ const ProjectDetails: React.FC<{
 
   return (
     <>
-      {tooltip}
+      {exportTooltip.node}
       <Container>
         <StateConflictRow projectId={projectId} />
         <TheExportRow>
           <DetailPanelButton
             onMouseEnter={(e) =>
-              openExportTooltip(e, e.target as unknown as HTMLButtonElement)
+              exportTooltip.open(e, e.target as unknown as HTMLButtonElement)
             }
             onClick={!downloaded ? exportProject : undefined}
             disabled={downloaded}

theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/AggregatedKeyframeTrack/AggregateKeyframeEditor/AggregateKeyframeConnector.tsx

@@ -55,7 +55,11 @@ export const AggregateKeyframeConnector: React.VFC<IAggregateKeyframeConnectorPr
     const [contextMenu] = useConnectorContextMenu(props, node)
     const [isDragging] = useDragKeyframe(node, props.editorProps)
 
-    const [popoverNode, openPopover, closePopover] = usePopover(
+    const {
+      node: popoverNode,
+      toggle: togglePopover,
+      close: closePopover,
+    } = usePopover(
       () => {
         const rightDims = val(editorProps.layoutP.rightDims)
 
@@ -89,7 +93,7 @@ export const AggregateKeyframeConnector: React.VFC<IAggregateKeyframeConnectorPr
           isSelected={connected ? connected.selected : false}
           isPopoverOpen={isAggregateEditingInCurvePopover}
           openPopover={(e) => {
-            if (node) openPopover(e, node)
+            if (node) togglePopover(e, node)
           }}
         />
         {popoverNode}

theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/AggregatedKeyframeTrack/AggregateKeyframeEditor/AggregateKeyframeDot.tsx

@@ -99,8 +99,7 @@ export function AggregateKeyframeDot(
   const logger = useLogger('AggregateKeyframeDot')
   const {cur} = props.utils
 
-  const [inlineEditorPopover, openEditor, _, isInlineEditorPopoverOpen] =
+  const inlineEditorPopover = useKeyframeInlineEditorPopover(
-    useKeyframeInlineEditorPopover(
     props.editorProps.viewModel.type === 'sheetObject'
       ? sheetObjectBuild(props.editorProps.viewModel, cur.keyframes)
           ?.children ?? null
@@ -136,7 +135,7 @@ export function AggregateKeyframeDot(
         // Need this for the dragging logic to be able to get the keyframe props
         // based on the position.
         {...DopeSnap.includePositionSnapAttrs(cur.position)}
-        onClick={(e) => openEditor(e, ref.current!)}
+        onClick={(e) => inlineEditorPopover.open(e, ref.current!)}
       />
       <AggregateKeyframeVisualDot
         flag={presence.flag}
@@ -144,7 +143,7 @@ export function AggregateKeyframeDot(
         isSelected={cur.selected}
       />
       {contextMenu}
-      {inlineEditorPopover}
+      {inlineEditorPopover.node}
     </>
   )
 }

theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/BasicKeyframedTrack/KeyframeEditor/BasicKeyframeConnector.tsx

@@ -41,7 +41,11 @@ const BasicKeyframeConnector: React.VFC<IBasicKeyframeConnectorProps> = (
 
   const [nodeRef, node] = useRefAndState<HTMLDivElement | null>(null)
 
-  const [popoverNode, openPopover, closePopover, isPopoverOpen] = usePopover(
+  const {
+    node: popoverNode,
+    toggle: togglePopover,
+    close: closePopover,
+  } = usePopover(
     () => {
       const rightDims = val(props.layoutP.rightDims)
       return {
@@ -83,7 +87,7 @@ const BasicKeyframeConnector: React.VFC<IBasicKeyframeConnectorProps> = (
         connectorLengthInUnitSpace={connectorLengthInUnitSpace}
         {...themeValues}
         openPopover={(e) => {
-          if (node) openPopover(e, node)
+          if (node) togglePopover(e, node)
         }}
       >
         {popoverNode}

theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/BasicKeyframedTrack/KeyframeEditor/SingleKeyframeDot.tsx

@@ -101,8 +101,11 @@ const SingleKeyframeDot: React.VFC<ISingleKeyframeDotProps> = (props) => {
   const [ref, node] = useRefAndState<HTMLDivElement | null>(null)
 
   const [contextMenu] = useSingleKeyframeContextMenu(node, logger, props)
-  const [inlineEditorPopover, openEditor, _, isInlineEditorPopoverOpen] =
+  const {
-    useKeyframeInlineEditorPopover([
+    node: inlineEditorPopover,
+    toggle: toggleEditor,
+    isOpen: isInlineEditorPopoverOpen,
+  } = useKeyframeInlineEditorPopover([
     {
       type: 'primitiveProp',
       keyframe: props.keyframe,
@@ -114,7 +117,7 @@ const SingleKeyframeDot: React.VFC<ISingleKeyframeDotProps> = (props) => {
   ])
   const [isDragging] = useDragForSingleKeyframeDot(node, props, {
     onClickFromDrag(dragStartEvent) {
-      openEditor(dragStartEvent, ref.current!)
+      toggleEditor(dragStartEvent, ref.current!)
     },
   })
 
@@ -213,6 +216,8 @@ function useDragForSingleKeyframeDot(
   const propsRef = useRef(props)
   propsRef.current = props
 
+  const {onClickFromDrag} = options
+
   const useDragOpts = useMemo<UseDragOpts>(() => {
     return {
       debugName: 'KeyframeDot/useDragKeyframe',
@@ -268,7 +273,7 @@ function useDragForSingleKeyframeDot(
           return (
             handlers && {
               ...handlers,
-              onClick: options.onClickFromDrag,
+              onClick: onClickFromDrag,
               onDragEnd: (...args) => {
                 handlers.onDragEnd?.(...args)
                 snapToNone()
@@ -324,12 +329,12 @@ function useDragForSingleKeyframeDot(
             snapToNone()
           },
           onClick(ev) {
-            options.onClickFromDrag(ev)
+            onClickFromDrag(ev)
           },
         }
       },
     }
-  }, [])
+  }, [onClickFromDrag])
 
   const [isDragging] = useDrag(node, useDragOpts)
 

theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/LengthIndicator/LengthEditorPopover.tsx

@@ -32,7 +32,7 @@ const LengthEditorPopover: React.FC<{
    * Called when user hits enter/escape
    */
   onRequestClose: (reason: string) => void
-}> = ({layoutP, onRequestClose}) => {
+}> = ({layoutP}) => {
   const sheet = useVal(layoutP.sheet)
 
   const fns = useMemo(() => {
@@ -89,7 +89,6 @@ const LengthEditorPopover: React.FC<{
           {...fns}
           isValid={greaterThanZero}
           inputRef={inputRef}
-          onBlur={onRequestClose.bind(null, 'length editor number input blur')}
           nudge={nudge}
         />
       </Container>

theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/LengthIndicator/LengthIndicator.tsx

@@ -138,19 +138,17 @@ const RENDER_OUT_OF_VIEW_X = -10000
 const LengthIndicator: React.FC<IProps> = ({layoutP}) => {
   const [nodeRef, node] = useRefAndState<HTMLDivElement | null>(null)
   const [isDragging] = useDragBulge(node, {layoutP})
-  const [popoverNode, openPopover, closePopover, isPopoverOpen] = usePopover(
+  const {
-    {debugName: 'LengthIndicator'},
+    node: popoverNode,
-    () => {
+    toggle: togglePopover,
+    close: closePopover,
+  } = usePopover({debugName: 'LengthIndicator'}, () => {
     return (
       <BasicPopover>
-          <LengthEditorPopover
+        <LengthEditorPopover layoutP={layoutP} onRequestClose={closePopover} />
-            layoutP={layoutP}
-            onRequestClose={closePopover}
-          />
       </BasicPopover>
     )
-    },
+  })
-  )
 
   return usePrism(() => {
     const sheet = val(layoutP.sheet)
@@ -191,7 +189,7 @@ const LengthIndicator: React.FC<IProps> = ({layoutP}) => {
               ref={nodeRef}
               // title="Length of the sequence. Drag or click to change."
               onClick={(e) => {
-                openPopover(e, node!)
+                togglePopover(e, node!)
               }}
               {...includeLockFrameStampAttrs('hide')}
             >

theatre/studio/src/panels/SequenceEditorPanel/GraphEditor/BasicKeyframedTrack/KeyframeEditor/GraphEditorDotNonScalar.tsx

@@ -70,8 +70,7 @@ const GraphEditorDotNonScalar: React.VFC<IProps> = (props) => {
 
   const curValue = props.which === 'left' ? 0 : 1
 
-  const [inlineEditorPopover, openEditor, _, _isInlineEditorPopoverOpen] =
+  const inlineEditorPopover = useKeyframeInlineEditorPopover([
-    useKeyframeInlineEditorPopover([
     {
       type: 'primitiveProp',
       keyframe: props.keyframe,
@@ -87,7 +86,10 @@ const GraphEditorDotNonScalar: React.VFC<IProps> = (props) => {
     props,
     // dragging does not work with also having a click listener
     onDetectedClick: (event) =>
-      openEditor(event, event.target instanceof Element ? event.target : node!),
+      inlineEditorPopover.toggle(
+        event,
+        event.target instanceof Element ? event.target : node!,
+      ),
   })
 
   const cyInExtremumSpace = props.extremumSpace.fromValueSpace(curValue)
@@ -114,7 +116,7 @@ const GraphEditorDotNonScalar: React.VFC<IProps> = (props) => {
           fill: presence.flag === PresenceFlag.Primary ? 'white' : undefined,
         }}
       />
-      {inlineEditorPopover}
+      {inlineEditorPopover.node}
       {contextMenu}
     </>
   )

theatre/studio/src/panels/SequenceEditorPanel/GraphEditor/BasicKeyframedTrack/KeyframeEditor/GraphEditorDotScalar.tsx

@@ -70,8 +70,7 @@ const GraphEditorDotScalar: React.VFC<IProps> = (props) => {
   const curValue = cur.value as number
 
   const cyInExtremumSpace = props.extremumSpace.fromValueSpace(curValue)
-  const [inlineEditorPopover, openEditor, _, _isInlineEditorPopoverOpen] =
+  const inlineEditorPopover = useKeyframeInlineEditorPopover([
-    useKeyframeInlineEditorPopover([
     {
       type: 'primitiveProp',
       keyframe: props.keyframe,
@@ -87,7 +86,10 @@ const GraphEditorDotScalar: React.VFC<IProps> = (props) => {
     props,
     // dragging does not work with also having a click listener
     onDetectedClick: (event) =>
-      openEditor(event, event.target instanceof Element ? event.target : node!),
+      inlineEditorPopover.toggle(
+        event,
+        event.target instanceof Element ? event.target : node!,
+      ),
   })
 
   return (
@@ -112,7 +114,7 @@ const GraphEditorDotScalar: React.VFC<IProps> = (props) => {
           fill: presence.flag === PresenceFlag.Primary ? 'white' : undefined,
         }}
       />
-      {inlineEditorPopover}
+      {inlineEditorPopover.node}
       {contextMenu}
     </>
   )

theatre/studio/src/panels/SequenceEditorPanel/RightOverlay/Playhead.tsx

@@ -193,9 +193,11 @@ const Playhead: React.FC<{layoutP: Pointer<SequenceEditorPanelLayout>}> = ({
 }) => {
   const [thumbRef, thumbNode] = useRefAndState<HTMLElement | null>(null)
 
-  const [popoverNode, openPopover, closePopover, isPopoverOpen] = usePopover(
+  const {
-    {debugName: 'Playhead'},
+    node: popoverNode,
-    () => {
+    toggle: togglePopover,
+    close: closePopover,
+  } = usePopover({debugName: 'Playhead'}, () => {
     return (
       <BasicPopover>
         <PlayheadPositionPopover
@@ -204,8 +206,7 @@ const Playhead: React.FC<{layoutP: Pointer<SequenceEditorPanelLayout>}> = ({
         />
       </BasicPopover>
     )
-    },
+  })
-  )
 
   const gestureHandlers = useMemo((): Parameters<typeof useDrag>[1] => {
     return {
@@ -236,7 +237,7 @@ const Playhead: React.FC<{layoutP: Pointer<SequenceEditorPanelLayout>}> = ({
             snapToNone()
           },
           onClick(e) {
-            openPopover(e, thumbRef.current!)
+            togglePopover(e, thumbRef.current!)
           },
         }
       },

theatre/studio/src/panels/SequenceEditorPanel/RightOverlay/PlayheadPositionPopover.tsx

@@ -33,7 +33,7 @@ const PlayheadPositionPopover: React.FC<{
    * Called when user hits enter/escape
    */
   onRequestClose: (reason: string) => void
-}> = ({layoutP, onRequestClose}) => {
+}> = ({layoutP}) => {
   const sheet = val(layoutP.sheet)
   const sequence = sheet.getSequence()
 
@@ -80,7 +80,6 @@ const PlayheadPositionPopover: React.FC<{
           {...fns}
           isValid={greaterThanOrEqualToZero}
           inputRef={inputRef}
-          onBlur={onRequestClose.bind(null, 'number input blur')}
           nudge={nudge}
         />
       </Container>

theatre/studio/src/propEditors/simpleEditors/RgbaPropEditor.tsx

@@ -75,9 +75,7 @@ function RgbaPropEditor({
     [editingTools],
   )
 
-  const [popoverNode, openPopover] = usePopover(
+  const popover = usePopover({debugName: 'RgbaPropEditor'}, () => (
-    {debugName: 'RgbaPropEditor'},
-    () => (
     <RgbaPopover>
       <RgbaColorPicker
         color={{
@@ -98,8 +96,7 @@ function RgbaPropEditor({
         discardTemporaryValue={editingTools.discardTemporaryValue}
       />
     </RgbaPopover>
-    ),
+  ))
-  )
 
   return (
     <>
@@ -108,7 +105,7 @@ function RgbaPropEditor({
           rgbaColor={value}
           ref={containerRef}
           onClick={(e) => {
-            openPopover(e, containerRef.current)
+            popover.toggle(e, containerRef.current)
           }}
         />
         <HexInput
@@ -120,7 +117,7 @@ function RgbaPropEditor({
           autoFocus={autoFocus}
         />
       </RowContainer>
-      {popoverNode}
+      {popover.node}
     </>
   )
 }

theatre/studio/src/toolbars/GlobalToolbar.tsx

@@ -58,7 +58,7 @@ const HasUpdatesBadge = styled.div`
 `
 
 const GroupDivider = styled.div`
-  position: abolute;
+  position: absolute;
   height: 32px;
   width: 1px;
   background: #373b40;
@@ -98,7 +98,7 @@ const GlobalToolbar: React.FC = () => {
   const hasUpdates =
     useVal(getStudio().atomP.ahistoric.updateChecker.result.hasUpdates) === true
 
-  const [moreMenu, openMoreMenu] = usePopover(
+  const moreMenu = usePopover(
     () => {
       const triggerBounds = moreMenuTriggerRef.current!.getBoundingClientRect()
       return {
@@ -166,11 +166,11 @@ const GlobalToolbar: React.FC = () => {
         <ExtensionToolbar showLeftDivider toolbarId="global" />
       </SubContainer>
       <SubContainer>
-        {moreMenu}
+        {moreMenu.node}
         <ToolbarIconButton
           ref={moreMenuTriggerRef}
           onClick={(e) => {
-            openMoreMenu(e, moreMenuTriggerRef.current!)
+            moreMenu.toggle(e, moreMenuTriggerRef.current!)
           }}
         >
           <Ellipsis />

theatre/studio/src/uiComponents/Popover/TooltipWrapper.tsx

@@ -138,7 +138,10 @@ const TooltipWrapper: React.FC<{
     props.onPointerOutside,
   ])
 
-  useOnClickOutside(container, props.onClickOutside ?? noop)
+  useOnClickOutside(
+    [container, props.target ?? null],
+    props.onClickOutside ?? noop,
+  )
 
   return (
     <ArrowContext.Provider value={arrowContextValue}>

theatre/studio/src/uiComponents/Popover/usePopover.tsx

@@ -48,10 +48,22 @@ type Opts = {
   verticalGap?: number
 }
 
+export interface IPopover {
+  /**
+   * The React node of the popover. Insert into your JSX using \{node\}. Its state
+   * will be managed automatically.
+   */
+  node: React.ReactNode
+  open: OpenFn
+  close: CloseFn
+  toggle: OpenFn
+  isOpen: boolean
+}
+
 export default function usePopover(
   opts: Opts | (() => Opts),
   render: () => React.ReactElement,
-): [node: React.ReactNode, open: OpenFn, close: CloseFn, isOpen: boolean] {
+): IPopover {
   const _debug = (...args: any) => {}
 
   // want to make sure that we don't close a popover when dragging something (like a curve editor handle)
@@ -104,6 +116,14 @@ export default function usePopover(
     }
   }, [])
 
+  const toggle = useCallback<OpenFn>((...args) => {
+    if (stateRef.current.isOpen) {
+      close('toggled')
+    } else {
+      open(...args)
+    }
+  }, [])
+
   /**
    * See doc comment on {@link useAutoCloseLockState}.
    * Used to ensure that moving far away from a parent popover doesn't
@@ -146,7 +166,7 @@ export default function usePopover(
     <></>
   )
 
-  return [node, open, close, state.isOpen]
+  return {node, open, close, toggle, isOpen: state.isOpen}
 }
 
 /**

theatre/studio/src/uiComponents/useOnClickOutside.ts

@@ -2,15 +2,23 @@ import type {$IntentionalAny} from '@theatre/shared/utils/types'
 import {useEffect} from 'react'
 
 export default function useOnClickOutside(
-  container: Element | null,
+  container: Element | null | (Element | null)[],
   onOutside: (e: MouseEvent) => void,
   enabled?: boolean,
+  // Can be used e.g. to prevent unexpected closing-reopening when clicking on a
+  // popover's trigger.
 ) {
   useEffect(() => {
     if (!container || enabled === false) return
 
+    const containers = Array.isArray(container)
+      ? (container.filter((container) => container) as Element[])
+      : [container]
+
     const onMouseDown = (e: MouseEvent) => {
-      if (!e.composedPath().includes(container)) {
+      if (
+        containers.every((container) => !e.composedPath().includes(container))
+      ) {
         onOutside(e)
       }
     }