Fix usePopover event handler cleanup

Author: mdodgelooker

CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [UNRELEASED]
+
+### Fixed
+
+- `usePopover` event handlers not getting properly cleaned up
+
 ## [0.7.19] - 2020-02-24
 
 ### Added

packages/components/src/Popover/Popover.tsx

@@ -32,7 +32,6 @@ import React, {
   RefObject,
   Ref,
   SyntheticEvent,
-  useRef,
 } from 'react'
 import { Popper } from 'react-popper'
 import { Box } from '../Layout'
@@ -236,7 +235,9 @@ function usePopoverToggle(
   triggerElement: HTMLElement | null
 ): [boolean, (value: boolean) => void] {
   const [uncontrolledIsOpen, uncontrolledSetOpen] = useState(controlledIsOpen)
-  const mouseDownTarget = useRef<EventTarget | null>()
+  const [mouseDownTarget, setMouseDownTarget] = useState<EventTarget | null>(
+    null
+  )
   const isControlled = useControlWarn({
     controllingProps: ['setOpen'],
     isControlledCheck: () => controlledSetOpen !== undefined,
@@ -255,9 +256,9 @@ function usePopoverToggle(
       // outside the popover as this is preferable to a bug where another
       // component triggers a scroll animation resulting in an
       // unintentional drag, which closes the popover
-      if (portalElement && mouseDownTarget.current) {
+      if (portalElement && mouseDownTarget) {
         const relationship = portalElement.compareDocumentPosition(
-          mouseDownTarget.current as Node
+          mouseDownTarget as Node
         )
         if (
           relationship === Node.DOCUMENT_POSITION_FOLLOWING ||
@@ -303,7 +304,7 @@ function usePopoverToggle(
     }
 
     function handleMouseDown(event: MouseEvent) {
-      mouseDownTarget.current = event.target
+      setMouseDownTarget(event.target)
       checkCloseAndStopEvent(event)
     }
 
@@ -312,20 +313,16 @@ function usePopoverToggle(
     }
 
     function handleMouseUp() {
-      window.requestAnimationFrame(() => {
-        mouseDownTarget.current = null
-        document.removeEventListener('click', handleClickOutside, true)
-        document.removeEventListener('mouseup', handleMouseUp, true)
-      })
+      setMouseDownTarget(null)
     }
 
     if (isOpen) {
       document.addEventListener('mousedown', handleMouseDown, true)
-      if (!mouseDownTarget.current) {
-        document.addEventListener('click', handleClickOutside, true)
-      }
-    } else if (mouseDownTarget.current) {
       document.addEventListener('click', handleClickOutside, true)
+    } else if (mouseDownTarget) {
+      // popover was closed via mousedown, but still need to cancel next click
+      document.addEventListener('click', handleClickOutside, true)
+      // and then cleanup mouseDownTarget
       document.addEventListener('mouseup', handleMouseUp, true)
     }
 
@@ -342,6 +339,7 @@ function usePopoverToggle(
     triggerElement,
     portalElement,
     triggerToggle,
+    mouseDownTarget,
   ])
 
   return [isOpen, setOpen]

packages/playground/src/index.tsx

@@ -20,30 +20,17 @@
 
 import React from 'react'
 import ReactDOM from 'react-dom'
-import { GlobalStyle, Field, FieldText } from '@looker/components'
+import { GlobalStyle } from '@looker/components'
 import { theme } from '@looker/design-tokens'
 import { ThemeProvider } from 'styled-components'
 
+import { SelectDemo } from './Select/SelectDemo'
+
 const App: React.FC = () => {
   return (
     <ThemeProvider theme={theme}>
       <GlobalStyle />
-      <Field required description="description" detail="0/50" label="First" />
-      <FieldText
-        required
-        description="description"
-        detail="0/50"
-        label="First"
-        name="firstName"
-      />
-      <FieldText
-        width="480px"
-        required
-        description="description"
-        detail="0/50"
-        label="First"
-        name="firstName"
-      />
+      <SelectDemo />
     </ThemeProvider>
   )
 }