From 935ac36467924976402eacef15f5fa51c573175f Mon Sep 17 00:00:00 2001 From: Andrew Prifer Date: Fri, 10 Jun 2022 21:26:09 +0200 Subject: [PATCH] Fix popover behavior when open and clicking on trigger --- .../src/panels/DetailPanel/ProjectDetails.tsx | 2 +- .../AggregateKeyframeConnector.tsx | 8 +++-- .../KeyframeEditor/BasicKeyframeConnector.tsx | 8 +++-- .../KeyframeEditor/SingleKeyframeDot.tsx | 17 +++++++---- .../LengthIndicator/LengthEditorPopover.tsx | 3 +- .../Right/LengthIndicator/LengthIndicator.tsx | 26 ++++++++--------- .../RightOverlay/Playhead.tsx | 29 ++++++++++--------- .../RightOverlay/PlayheadPositionPopover.tsx | 3 +- .../simpleEditors/RgbaPropEditor.tsx | 4 +-- theatre/studio/src/toolbars/GlobalToolbar.tsx | 4 +-- .../uiComponents/Popover/TooltipWrapper.tsx | 5 +++- .../src/uiComponents/Popover/usePopover.tsx | 18 ++++++++++-- .../src/uiComponents/useOnClickOutside.ts | 13 +++++++-- 13 files changed, 88 insertions(+), 52 deletions(-) diff --git a/theatre/studio/src/panels/DetailPanel/ProjectDetails.tsx b/theatre/studio/src/panels/DetailPanel/ProjectDetails.tsx index aeb347c..aba7d8a 100644 --- a/theatre/studio/src/panels/DetailPanel/ProjectDetails.tsx +++ b/theatre/studio/src/panels/DetailPanel/ProjectDetails.tsx @@ -55,7 +55,7 @@ const ProjectDetails: React.FC<{ }, 40000) }, []) - const [tooltip, openExportTooltip] = usePopover( + const {node: tooltip, open: openExportTooltip} = usePopover( {debugName: 'ProjectDetails', pointerDistanceThreshold: 50}, () => ( diff --git a/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/AggregatedKeyframeTrack/AggregateKeyframeEditor/AggregateKeyframeConnector.tsx b/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/AggregatedKeyframeTrack/AggregateKeyframeEditor/AggregateKeyframeConnector.tsx index 99cebe7..218602d 100644 --- a/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/AggregatedKeyframeTrack/AggregateKeyframeEditor/AggregateKeyframeConnector.tsx +++ b/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/AggregatedKeyframeTrack/AggregateKeyframeEditor/AggregateKeyframeConnector.tsx @@ -55,7 +55,11 @@ export const AggregateKeyframeConnector: React.VFC { const rightDims = val(editorProps.layoutP.rightDims) @@ -89,7 +93,7 @@ export const AggregateKeyframeConnector: React.VFC { - if (node) openPopover(e, node) + if (node) togglePopover(e, node) }} /> {popoverNode} diff --git a/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/BasicKeyframedTrack/KeyframeEditor/BasicKeyframeConnector.tsx b/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/BasicKeyframedTrack/KeyframeEditor/BasicKeyframeConnector.tsx index 451b7a4..7038963 100644 --- a/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/BasicKeyframedTrack/KeyframeEditor/BasicKeyframeConnector.tsx +++ b/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/BasicKeyframedTrack/KeyframeEditor/BasicKeyframeConnector.tsx @@ -41,7 +41,11 @@ const BasicKeyframeConnector: React.VFC = ( const [nodeRef, node] = useRefAndState(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 = ( connectorLengthInUnitSpace={connectorLengthInUnitSpace} {...themeValues} openPopover={(e) => { - if (node) openPopover(e, node) + if (node) togglePopover(e, node) }} > {popoverNode} diff --git a/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/BasicKeyframedTrack/KeyframeEditor/SingleKeyframeDot.tsx b/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/BasicKeyframedTrack/KeyframeEditor/SingleKeyframeDot.tsx index 0cc379f..3f6087d 100644 --- a/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/BasicKeyframedTrack/KeyframeEditor/SingleKeyframeDot.tsx +++ b/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/BasicKeyframedTrack/KeyframeEditor/SingleKeyframeDot.tsx @@ -94,11 +94,14 @@ const SingleKeyframeDot: React.VFC = (props) => { const [ref, node] = useRefAndState(null) const [contextMenu] = useSingleKeyframeContextMenu(node, logger, props) - const [inlineEditorPopover, openEditor, _, isInlineEditorPopoverOpen] = - useSingleKeyframeInlineEditorPopover(props) + const { + node: inlineEditorPopover, + toggle: toggleEditor, + isOpen: isInlineEditorPopoverOpen, + } = useSingleKeyframeInlineEditorPopover(props) const [isDragging] = useDragForSingleKeyframeDot(node, props, { onClickFromDrag(dragStartEvent) { - openEditor(dragStartEvent, ref.current!) + toggleEditor(dragStartEvent, ref.current!) }, }) @@ -227,6 +230,8 @@ function useDragForSingleKeyframeDot( const propsRef = useRef(props) propsRef.current = props + const {onClickFromDrag} = options + const useDragOpts = useMemo(() => { return { debugName: 'KeyframeDot/useDragKeyframe', @@ -282,7 +287,7 @@ function useDragForSingleKeyframeDot( return ( handlers && { ...handlers, - onClick: options.onClickFromDrag, + onClick: onClickFromDrag, onDragEnd: (...args) => { handlers.onDragEnd?.(...args) snapToNone() @@ -338,12 +343,12 @@ function useDragForSingleKeyframeDot( snapToNone() }, onClick(ev) { - options.onClickFromDrag(ev) + onClickFromDrag(ev) }, } }, } - }, []) + }, [onClickFromDrag]) const [isDragging] = useDrag(node, useDragOpts) diff --git a/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/LengthIndicator/LengthEditorPopover.tsx b/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/LengthIndicator/LengthEditorPopover.tsx index 006f5de..82dbb5d 100644 --- a/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/LengthIndicator/LengthEditorPopover.tsx +++ b/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} /> diff --git a/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/LengthIndicator/LengthIndicator.tsx b/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/LengthIndicator/LengthIndicator.tsx index fb81d37..461aebf 100644 --- a/theatre/studio/src/panels/SequenceEditorPanel/DopeSheet/Right/LengthIndicator/LengthIndicator.tsx +++ b/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 = ({layoutP}) => { const [nodeRef, node] = useRefAndState(null) const [isDragging] = useDragBulge(node, {layoutP}) - const [popoverNode, openPopover, closePopover, isPopoverOpen] = usePopover( - {debugName: 'LengthIndicator'}, - () => { - return ( - - - - ) - }, - ) + const { + node: popoverNode, + toggle: togglePopover, + close: closePopover, + } = usePopover({debugName: 'LengthIndicator'}, () => { + return ( + + + + ) + }) return usePrism(() => { const sheet = val(layoutP.sheet) @@ -191,7 +189,7 @@ const LengthIndicator: React.FC = ({layoutP}) => { ref={nodeRef} // title="Length of the sequence. Drag or click to change." onClick={(e) => { - openPopover(e, node!) + togglePopover(e, node!) }} {...includeLockFrameStampAttrs('hide')} > diff --git a/theatre/studio/src/panels/SequenceEditorPanel/RightOverlay/Playhead.tsx b/theatre/studio/src/panels/SequenceEditorPanel/RightOverlay/Playhead.tsx index 6af70b5..b19ac57 100644 --- a/theatre/studio/src/panels/SequenceEditorPanel/RightOverlay/Playhead.tsx +++ b/theatre/studio/src/panels/SequenceEditorPanel/RightOverlay/Playhead.tsx @@ -193,19 +193,20 @@ const Playhead: React.FC<{layoutP: Pointer}> = ({ }) => { const [thumbRef, thumbNode] = useRefAndState(null) - const [popoverNode, openPopover, closePopover, isPopoverOpen] = usePopover( - {debugName: 'Playhead'}, - () => { - return ( - - - - ) - }, - ) + const { + node: popoverNode, + toggle: togglePopover, + close: closePopover, + } = usePopover({debugName: 'Playhead'}, () => { + return ( + + + + ) + }) const gestureHandlers = useMemo((): Parameters[1] => { return { @@ -236,7 +237,7 @@ const Playhead: React.FC<{layoutP: Pointer}> = ({ snapToNone() }, onClick(e) { - openPopover(e, thumbRef.current!) + togglePopover(e, thumbRef.current!) }, } }, diff --git a/theatre/studio/src/panels/SequenceEditorPanel/RightOverlay/PlayheadPositionPopover.tsx b/theatre/studio/src/panels/SequenceEditorPanel/RightOverlay/PlayheadPositionPopover.tsx index 492d673..27b4f73 100644 --- a/theatre/studio/src/panels/SequenceEditorPanel/RightOverlay/PlayheadPositionPopover.tsx +++ b/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={greaterThanZero} inputRef={inputRef} - onBlur={onRequestClose.bind(null, 'number input blur')} nudge={nudge} /> diff --git a/theatre/studio/src/propEditors/simpleEditors/RgbaPropEditor.tsx b/theatre/studio/src/propEditors/simpleEditors/RgbaPropEditor.tsx index 39dbab2..91fa27b 100644 --- a/theatre/studio/src/propEditors/simpleEditors/RgbaPropEditor.tsx +++ b/theatre/studio/src/propEditors/simpleEditors/RgbaPropEditor.tsx @@ -74,7 +74,7 @@ function RgbaPropEditor({ [editingTools], ) - const [popoverNode, openPopover] = usePopover( + const {node: popoverNode, toggle: togglePopover} = usePopover( {debugName: 'RgbaPropEditor'}, () => ( @@ -107,7 +107,7 @@ function RgbaPropEditor({ rgbaColor={value} ref={containerRef} onClick={(e) => { - openPopover(e, containerRef.current) + togglePopover(e, containerRef.current) }} /> { const hasUpdates = useVal(getStudio().atomP.ahistoric.updateChecker.result.hasUpdates) === true - const [moreMenu, openMoreMenu] = usePopover( + const {node: moreMenu, toggle: toggleMenu} = usePopover( () => { const triggerBounds = moreMenuTriggerRef.current!.getBoundingClientRect() return { @@ -138,7 +138,7 @@ const GlobalToolbar: React.FC = () => { { - openMoreMenu(e, moreMenuTriggerRef.current!) + toggleMenu(e, moreMenuTriggerRef.current!) }} > diff --git a/theatre/studio/src/uiComponents/Popover/TooltipWrapper.tsx b/theatre/studio/src/uiComponents/Popover/TooltipWrapper.tsx index 234afbc..57b31d6 100644 --- a/theatre/studio/src/uiComponents/Popover/TooltipWrapper.tsx +++ b/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 ( diff --git a/theatre/studio/src/uiComponents/Popover/usePopover.tsx b/theatre/studio/src/uiComponents/Popover/usePopover.tsx index f284069..f2567a7 100644 --- a/theatre/studio/src/uiComponents/Popover/usePopover.tsx +++ b/theatre/studio/src/uiComponents/Popover/usePopover.tsx @@ -51,7 +51,13 @@ type Opts = { export default function usePopover( opts: Opts | (() => Opts), render: () => React.ReactElement, -): [node: React.ReactNode, open: OpenFn, close: CloseFn, isOpen: boolean] { +): { + node: React.ReactNode + open: OpenFn + close: CloseFn + toggle: OpenFn + isOpen: boolean +} { 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 +110,14 @@ export default function usePopover( } }, []) + const toggle = useCallback((...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 +160,7 @@ export default function usePopover( <> ) - return [node, open, close, state.isOpen] + return {node, open, close, toggle, isOpen: state.isOpen} } /** diff --git a/theatre/studio/src/uiComponents/useOnClickOutside.ts b/theatre/studio/src/uiComponents/useOnClickOutside.ts index 96d533f..ac729a5 100644 --- a/theatre/studio/src/uiComponents/useOnClickOutside.ts +++ b/theatre/studio/src/uiComponents/useOnClickOutside.ts @@ -2,15 +2,24 @@ 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)) + ) { + console.log('outside') onOutside(e) } }